Demonstrate that we don't need pytest-openfiles any more#14040
Closed
Cadair wants to merge 1 commit intoastropy:mainfrom
Closed
Demonstrate that we don't need pytest-openfiles any more#14040Cadair wants to merge 1 commit intoastropy:mainfrom
Cadair wants to merge 1 commit intoastropy:mainfrom
Conversation
Cadair
changed the title
Member
Author
Contributor
|
This looks good! I noticed that |
Member
Author
|
Well pytest-openfiles would catch the fixture if it were closed or unclosed, so it's not a surprise it gets it, what is a surprise is that it doesn't raise a resource warning. |
Member
|
But we are also ignoring these. Does this mean we cannot ignore these no more? Lines 135 to 136 in b2b32a4 |
Member
|
I would also be interested to see if #14041 would fail on these cases. Is there already such a PR somewhere? |
Member
Author
|
Those ignores aren't excluding open file resource warnings, just ones for SSL and socket? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I ran into a pytest-openfiles bug astropy/pytest-openfiles#32 and it seems like we should just stop using it because pytest throws a
ResourceWarningnow which our default warnings filter converts into an exception.This PR adds some tests which leave open files hanging (and some which don't), my hypothesis is that all the CI jobs will fail and not just the one which uses
pytest-openfiles, if this is the case we can remove openfiles because it is redundant.Secondary hypothesis: More tests will fail on the pytest-openfiles build because of astropy/pytest-openfiles#32 that will fail on the other builds, but these fails are actually incorrect and shouldn't fail because the files are closed in the fixture teardown which happens after pytest-openfiles checks to see if the files are closed.