Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2 failing test fixes for Windows platform #672

Merged
merged 3 commits into from Aug 12, 2022

Conversation

surdouski
Copy link
Contributor

@surdouski surdouski commented Jul 2, 2022

This pull request fixes multiple failing tests within test_exceptions.py and test_utils.py when papermill is run on the windows platform.

test_exceptions.py : test_exceptions_are_unpickleable
The previous behavior of the code created a temporary file with the incorrect permissions assigned to it resulting in a PermissionDenied error.

test_utils.py : test_chdir
The previous behavior of the code used os.path to compare against strings, which had the issue of sometimes comparing minified path strings (such as 'C:\Users\USER~1\Path\To\My\Basement') to non-minified path strings. New code uses more commonly used pathlib library to get and compare Paths.

@surdouski surdouski changed the title 2 of 2 failing tests on Windows (test_utils.py) 2 failing test fixes for Windows platform Jul 2, 2022
@surdouski surdouski force-pushed the windows-compatability-test_utils branch from d9f779b to 0137c57 Compare July 2, 2022 17:43
Copy link
Member

@rohitsanj rohitsanj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hoping this review triggers CI 🤷

@rohitsanj rohitsanj self-requested a review August 12, 2022 22:52
@codecov
Copy link

codecov bot commented Aug 12, 2022

Codecov Report

Merging #672 (daa9419) into main (a3f530e) will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #672      +/-   ##
==========================================
- Coverage   91.90%   91.87%   -0.03%     
==========================================
  Files          17       17              
  Lines        1605     1600       -5     
==========================================
- Hits         1475     1470       -5     
  Misses        130      130              
Impacted Files Coverage Δ
papermill/parameterize.py 97.91% <0.00%> (-0.09%) ⬇️
papermill/execute.py 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3f530e...daa9419. Read the comment docs.

Copy link
Member

@rohitsanj rohitsanj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for improving the tests, @surdouski!

@rohitsanj rohitsanj merged commit 77ba9c4 into nteract:main Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants