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

Ignore errors in teardown process #56

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

GenevieveBuckley
Copy link
Contributor

Closes #55

I understand totally if you don't want to go with the simplest solution here, but I thought I'd open this in case your open to it.

With the current main branch, some users might run into failing CI jobs, like this.

With this PR, the worst case scenario is that some test files do not get removed completely from the pytest temp directories. That's not a problem on CI, since there is no persistance of files there. On a local machine, possibly this could become a problem if many hundreds or thousands of tiles built up over time on the disk (but presumably someone would clear the pytest cache at some stage if they needed more disk space). So, potentially annoying in some limited edge cases, but the advantage is that it won't cause the test suite to fail (and not because the tests fail, but because of an error in the teardown process).

Copy link

codecov bot commented Nov 1, 2023

Codecov Report

Merging #56 (04b1368) into main (c9f5e3f) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main      #56   +/-   ##
=======================================
  Coverage   98.59%   98.59%           
=======================================
  Files           2        2           
  Lines          71       71           
=======================================
  Hits           70       70           
  Misses          1        1           
Files Coverage Δ
pytest_copie/plugin.py 98.52% <100.00%> (ø)

Copy link
Owner

@12rambau 12rambau left a comment

Choose a reason for hiding this comment

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

I don't have a strong opinion on that part of the code, so I guess I'm happy with the suggested changes.
As you seems to have a solid use case, it's even better. In case someone face an issue with this modification we could revert it and find something more clever.

@12rambau 12rambau merged commit aebafc7 into 12rambau:main Nov 1, 2023
12 checks passed
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.

Graceful error handling in pytest-copie teardown process
2 participants