Skip to content

Fix a few more setup -> setup_method tests#1142

Merged
dwhswenson merged 3 commits into
openpathsampling:masterfrom
dwhswenson:pytest-setup-in-exp
Apr 23, 2024
Merged

Fix a few more setup -> setup_method tests#1142
dwhswenson merged 3 commits into
openpathsampling:masterfrom
dwhswenson:pytest-setup-in-exp

Conversation

@dwhswenson

@dwhswenson dwhswenson commented Jan 28, 2024

Copy link
Copy Markdown
Member

In #1123, most of our setup/teardown methods were fixed to use pytest's preferred (and now required) setup_method/teardown_method nomenclature. This fixes a few more.


UPDATE: This fixes 3 problems caused by upstream breaking changes:

  1. As mentioned above, pytest now requires setup_method / teardown_method instead of setup and teardown.
  2. Newer versions of pytest only work with nbval 0.10.0 or later. nbval 0.10.0 introduced what I consider to be a bug, which is that return values that you hide in-notebook with a ; are seen by nbval as output. This required using NBVAL_IGNORE in some notebooks.
  3. pytest also broke XUnit-style tests that include staticmethods as tests (I consider this to also be a bug). The fix was to just make them instance methods.

@dwhswenson

Copy link
Copy Markdown
Member Author

Current failure here is because the newer pytest requires nbval 0.10.0, which we pin to avoid against because it broke notebook tests (outputs changed). This will take a little work to resolve.

For some reason, if you silence output with a ; in an ipynb, nbval still
gets the original output and expects that. I think this is a bug, but
apparently they haven't fixed this yet.
@dwhswenson

Copy link
Copy Markdown
Member Author

Finally getting CI to green again! See top post for updated contents of this PR. This is ready for review and comment. Without review, I will merge this after at least 18 hours, no sooner than Tue 23 Apr 19:00 GMT (14:00 my local).

@sroet sroet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I assume the ipynb changes outside of "# NBVAL_IGNORE_OUTPUT are just some other update diffs? LGTM otherwise

@dwhswenson

Copy link
Copy Markdown
Member Author

Yeah, the other notebook changes were just "rerun the notebook with updated software and maybe that will fix it?" before actually having to look in detail at the problem.

@dwhswenson dwhswenson merged commit 73a5565 into openpathsampling:master Apr 23, 2024
@dwhswenson dwhswenson deleted the pytest-setup-in-exp branch April 23, 2024 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants