Convert windows line ending and previous PR #276

pem70 · 2024-06-04T15:30:23Z

What It Does
Fixes [#104]
Address issues [#264] and [#265] by refactoring classes and tests

How to Test

Review Checklist
I certify that I have:

tested my changes
added/updated automated tests
updated the changelog
followed the contribution guidelines

Additional Comments

Signed-off-by: pem70 <[email protected]>

codecov · 2024-06-04T15:36:29Z

Codecov Report

Attention: Patch coverage is 93.06931% with 63 lines in your changes missing coverage. Please review.

Project coverage is 89.28%. Comparing base (5816aab) to head (95ac994).
Report is 230 commits behind head on main.

Files with missing lines	Patch %	Lines
.../zos_files/zowe/zos_files_for_zowe_sdk/datasets.py	88.42%	25 Missing ⚠️
src/zos_files/zowe/zos_files_for_zowe_sdk/uss.py	66.10%	20 Missing ⚠️
src/zos_files/zowe/zos_files_for_zowe_sdk/files.py	78.26%	10 Missing ⚠️
...s_files/zowe/zos_files_for_zowe_sdk/file_system.py	90.00%	5 Missing ⚠️
tests/unit/core/test_config.py	96.07%	2 Missing ⚠️
src/core/zowe/core_for_zowe_sdk/profile_manager.py	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #276      +/-   ##
==========================================
+ Coverage   86.21%   89.28%   +3.06%     
==========================================
  Files          35       53      +18     
  Lines        2292     2519     +227     
==========================================
+ Hits         1976     2249     +273     
+ Misses        316      270      -46

Flag	Coverage Δ
unittests	`89.28% <93.06%> (+3.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

t1m0thyj

Looks pretty good, thanks for restructuring code and tests @pem70! Left a few comments

CHANGELOG.md

src/core/zowe/core_for_zowe_sdk/logger.py

src/zos_files/zowe/zos_files_for_zowe_sdk/datasets.py

src/zos_files/zowe/zos_files_for_zowe_sdk/file_system.py

src/zos_files/zowe/zos_files_for_zowe_sdk/uss.py

tests/unit/files/datasets/test_delete.py

tests/unit/files/datasets/test_rename.py

Signed-off-by: pem70 <[email protected]>

zFernand0

Most comments from #274 and #275 seem to have been addressed in this PR.
Thank you! 🥳

src/zos_files/zowe/zos_files_for_zowe_sdk/files.py

CHANGELOG.md

tests/unit/core/test_request_handler.py

zFernand0

Looking good so far! 🥳
I'll approve once all PR comments are resolved.

Also, I noticed that you've been trying to keep the two PRs in sync (e.g. logger class changes or test-delete function names). To avoid extra work, you can base your other PR (#277) to this PR's target branch (convert_windows_line_ending)

src/core/zowe/core_for_zowe_sdk/logger.py

zFernand0 · 2024-06-05T19:17:55Z

src/core/zowe/core_for_zowe_sdk/profile_manager.py

-            logger.error(f"Unknown type is found in schema_json, exc")
-            raise jsonschema.exceptions.UnknownType(f"Unknown type is found in schema_json, exc")
+            logger.error(f"Unknown type is found in schema_json, {exc}")
+            raise jsonschema.exceptions.UnknownType(f"Unknown type is found in schema_json, {exc}")


Good catch!

Not sure if we should do a quick search across the board to make sure that we are interpolating/inserting the value of the error instead of the literal string of the variable name

zFernand0 · 2024-06-05T19:19:59Z

src/zos_files/zowe/zos_files_for_zowe_sdk/files.py

+from zowe.zos_files_for_zowe_sdk.constants import FileType
+from .datasets import Datasets
+from .uss import USSFiles
+from .file_system import FileSystems

 _ZOWE_FILES_DEFAULT_ENCODING = "utf-8"


I know all functions in this class are deprecated, but curious if we should also move this into a constansts.py file.

I'm not sure as I hardly think these imports would be used again in another file.

zFernand0 · 2024-06-05T19:21:58Z

src/zos_files/zowe/zos_files_for_zowe_sdk/uss.py

+    def list(self, path):
+        """Retrieve a list of USS files based on a given pattern.
+
+        Returns
+        -------
+        json
+            A JSON with a list of dataset names matching the given pattern
+        """


I know it may be a little redundant, and Timothy mentioned this offline not long ago...
Also, we don't have to do this as part of this PR.

Q: Should we make sure that the docs are complete and accurate?

For example, this is missing the parameters.

I think we definitely need to standardize the docs, but maybe not in this PR

Created #279 to track this

Co-authored-by: Fernando Rijo Cedeno <[email protected]> Signed-off-by: Peizhao Mei <[email protected]>

Signed-off-by: pem70 <[email protected]>

traeok

LGTM, thanks @pem70 for the fix and restructuring! I left a couple comments, but I don't have anything that must be addressed.

tests/unit/files/datasets/test_list.py

src/zos_files/zowe/zos_files_for_zowe_sdk/uss.py

Signed-off-by: pem70 <[email protected]>

pem70 · 2024-06-07T14:38:12Z

Revert changes from previous commit as it would break testing environments.

This reverts commit e491f73. Signed-off-by: pem70 <[email protected]>

Signed-off-by: pem70 <[email protected]>

t1m0thyj

LGTM, thanks @pem70! Will approve after conflicts have been resolved

src/core/zowe/core_for_zowe_sdk/profile_manager.py

Signed-off-by: pem70 <[email protected]>

t1m0thyj

LGTM, thanks @pem70!

Changes were addressed

pem70 added 7 commits May 30, 2024 09:56

Refactor files.py into separate classes

ddfca3f

Signed-off-by: pem70 <[email protected]>

Update CHANGELOG.md

58a0854

Signed-off-by: pem70 <[email protected]>

organize some tests

a4380d5

Signed-off-by: pem70 <[email protected]>

Refactor files tests and core tests

6a709b8

Signed-off-by: pem70 <[email protected]>

Add more unit tests

a01dc05

Signed-off-by: pem70 <[email protected]>

Update CHANGELOG.md

6a13b4d

Signed-off-by: pem70 <[email protected]>

fix bugs

dec0acd

Signed-off-by: pem70 <[email protected]>

github-actions bot assigned pem70 Jun 4, 2024

Fix unit tests

3258e0c

Signed-off-by: pem70 <[email protected]>

pem70 requested review from t1m0thyj, zFernand0 and traeok and removed request for zFernand0 June 4, 2024 15:37

pem70 marked this pull request as ready for review June 5, 2024 14:47

t1m0thyj requested changes Jun 5, 2024

View reviewed changes

pem70 added 2 commits June 5, 2024 13:20

Updated changelog and tsts_delete

7f8c2d8

Signed-off-by: pem70 <[email protected]>

Update logger.py

ece4724

Signed-off-by: pem70 <[email protected]>

zFernand0 reviewed Jun 5, 2024

View reviewed changes

src/zos_files/zowe/zos_files_for_zowe_sdk/files.py Show resolved Hide resolved

CHANGELOG.md Show resolved Hide resolved

CHANGELOG.md Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

tests/unit/core/test_request_handler.py Show resolved Hide resolved

zFernand0 previously requested changes Jun 5, 2024

View reviewed changes

pem70 and others added 3 commits June 5, 2024 15:31

Update CHANGELOG.md

1282174

Co-authored-by: Fernando Rijo Cedeno <[email protected]> Signed-off-by: Peizhao Mei <[email protected]>

Update test_rename.py

0dec3e9

Signed-off-by: pem70 <[email protected]>

Change _ZOWE_FILES_DEFAULT_ENCODING to a constant

b6be7e1

Signed-off-by: pem70 <[email protected]>

traeok approved these changes Jun 6, 2024

View reviewed changes

tests/unit/files/datasets/test_list.py Show resolved Hide resolved

src/zos_files/zowe/zos_files_for_zowe_sdk/uss.py Outdated Show resolved Hide resolved

pem70 added 2 commits June 6, 2024 16:15

Update test_copy.py

bc4069f

Signed-off-by: pem70 <[email protected]>

Remove unused imports and add test constants

e491f73

Signed-off-by: pem70 <[email protected]>

pem70 added 2 commits June 7, 2024 10:51

Revert "Remove unused imports and add test constants"

171b881

This reverts commit e491f73. Signed-off-by: pem70 <[email protected]>

Revert changes

736e714

Signed-off-by: pem70 <[email protected]>

pem70 force-pushed the convert_windows_line_ending branch from 34a3582 to 736e714 Compare June 7, 2024 14:51

t1m0thyj reviewed Jun 10, 2024

View reviewed changes

Merge branch 'main' into convert_windows_line_ending

935f20d

t1m0thyj requested review from t1m0thyj, traeok and zFernand0 June 10, 2024 17:50

t1m0thyj reviewed Jun 10, 2024

View reviewed changes

src/core/zowe/core_for_zowe_sdk/profile_manager.py Show resolved Hide resolved

Update profile_manager.py

95ac994

Signed-off-by: pem70 <[email protected]>

t1m0thyj approved these changes Jun 10, 2024

View reviewed changes

t1m0thyj merged commit cdf6265 into main Jun 10, 2024
21 checks passed

t1m0thyj deleted the convert_windows_line_ending branch June 10, 2024 18:57

adam-wolfe mentioned this pull request Jun 11, 2024

[zos-files] upload_file_to_dsn does not convert Windows line endings #104

Closed

t1m0thyj mentioned this pull request Sep 21, 2024

Update sample scripts in readmes to remove deprecated methods #336

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert windows line ending and previous PR #276

Convert windows line ending and previous PR #276

pem70 commented Jun 4, 2024 •

edited

Loading

codecov bot commented Jun 4, 2024 •

edited

Loading

t1m0thyj left a comment

zFernand0 left a comment

zFernand0 left a comment

zFernand0 Jun 5, 2024

zFernand0 Jun 5, 2024

pem70 Jun 6, 2024

zFernand0 Jun 5, 2024

pem70 Jun 5, 2024

t1m0thyj Jun 10, 2024

traeok left a comment •

edited

Loading

pem70 commented Jun 7, 2024

t1m0thyj left a comment

t1m0thyj left a comment

Convert windows line ending and previous PR #276

Convert windows line ending and previous PR #276

Conversation

pem70 commented Jun 4, 2024 • edited Loading

codecov bot commented Jun 4, 2024 • edited Loading

Codecov Report

t1m0thyj left a comment

Choose a reason for hiding this comment

zFernand0 left a comment

Choose a reason for hiding this comment

zFernand0 left a comment

Choose a reason for hiding this comment

zFernand0 Jun 5, 2024

Choose a reason for hiding this comment

zFernand0 Jun 5, 2024

Choose a reason for hiding this comment

pem70 Jun 6, 2024

Choose a reason for hiding this comment

zFernand0 Jun 5, 2024

Choose a reason for hiding this comment

pem70 Jun 5, 2024

Choose a reason for hiding this comment

t1m0thyj Jun 10, 2024

Choose a reason for hiding this comment

traeok left a comment • edited Loading

Choose a reason for hiding this comment

pem70 commented Jun 7, 2024

t1m0thyj left a comment

Choose a reason for hiding this comment

t1m0thyj left a comment

Choose a reason for hiding this comment

pem70 commented Jun 4, 2024 •

edited

Loading

codecov bot commented Jun 4, 2024 •

edited

Loading

traeok left a comment •

edited

Loading