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

Use Subprocess Runs for Example Tests #327

Merged
merged 2 commits into from
Jul 31, 2024
Merged

Conversation

Scienfitz
Copy link
Collaborator

@Scienfitz Scienfitz commented Jul 31, 2024

runpy did not run example files in isolated environments, causing problems with examples that had hook modifications via monkeypatching. subprocess.run calls a separate process hence not exhibiting the problem

this also required some coverage exceptions as some utils are only tested in examples

@Scienfitz Scienfitz added the bug Something isn't working label Jul 31, 2024
@Scienfitz Scienfitz self-assigned this Jul 31, 2024
Copy link
Collaborator

@AdrianSosic AdrianSosic left a comment

Choose a reason for hiding this comment

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

Thanks! But it would be good if you could add an explanation both to the PR description AND in the code, so that people (including future us) don't wonder why this is necessary in the first place.

@Scienfitz
Copy link
Collaborator Author

@AdrianSosic there is explanation in the code?

@AdrianSosic
Copy link
Collaborator

@AdrianSosic there is explanation in the code?

Example of a comment that mostly restates the obvious, i.e. of the form

# Do X
X()

You:

# This runs it in a separate process
subprocess(...)

Doesn't answer why the separate environment is needed in the first place. Would be good to mention e.g. monkeypatching that is applied in the examples that breaks subsequent code execution or similar

@Scienfitz
Copy link
Collaborator Author

@AdrianSosic ping

@Scienfitz Scienfitz merged commit 31c6804 into main Jul 31, 2024
9 of 10 checks passed
@Scienfitz Scienfitz deleted the fix/example_test_execution branch July 31, 2024 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants