-
Notifications
You must be signed in to change notification settings - Fork 47
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
Add support for classes with pytest 7 #164
Conversation
This pull request introduces 2 alerts when merging 83359e6 into b8bcfe1 - view on LGTM.com new alerts:
|
Instead modifying the test function itself by wrapping it in a function which runs the tests, use pytest hooks to intercept the generated figure and then run the tests. This should be a more robust approach that doesn't need as many special cases to be hardcoded. I have also refactored the get_marker and get_compare functions/methods to simplify these.
83359e6
to
50251e5
Compare
To ensure - linux: py310-test-mpl35-pytestdev
- linux: py310-test-mpl35-pytest62
- linux: py38-test-mpl35-pytest54 Since pytest doesn't seem to release bug fixes for previous major/minor versions, I have just added tests for the latest release for each major version from the past ~2 years. (We should probably set up a weekly cron for the tests also?) I've also added a test using the development version of pytest. (This uncovered that pytest are going to start warning and eventually erroring if a test returns anything other than |
# "item.keywords.get" was deprecated in pytest 3.6 | ||
# See https://docs.pytest.org/en/latest/mark.html#updating-code | ||
return item.keywords.get(marker_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to support this branch anymore.
I've also updated the This is technically a bugfix as the following should be able to generate three different baselines, but currently without the class name they overwrite each other and only the final test will pass when tested: class TestA:
@pytest.mark.mpl_image_compare
def test_a(self): ...
class TestB:
@pytest.mark.mpl_image_compare
def test_a(self): ...
@pytest.mark.mpl_image_compare
def test_a(self): ... This is a breaking change for figure tests within classes only, and the following will need to be done:
Thought I would include this here while we are fixing the classes. |
@Cadair I ran the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests pass so it's probably fine 🤣
Since
pytest
7 was releasedpytest-mpl
has not been running tests inside classes. This means that figure tests inside classes are always being reported as passing. In this PR, I have added additional tests that ensure figure tests inside classes can fail correctly, and I have also added new testing jobs for older versions of pytest as well as the development version.pytest-mpl
Solution
Instead modifying the test function itself by wrapping it in a function which runs the tests, this PR uses pytest hooks to intercept the generated figure and then run the tests. This should be a more robust approach that doesn't need as many special cases to be hardcoded (such as classes and support for
setup_method
etc.).Previously we updated the test function (to a version of itself but wrapped in the testing code) inside the
pytest_runtest_setup
hook. However, now we work inside thepytest_runtest_call
hook and override pytest's defaultpytest_pyfunc_call
hook:pytest_runtest_call
hookyield
pytest_pyfunc_call
hook which runs the test function, intercepts the returned figure, and saves it as an attribute (self.result
)yield
inside ourpytest_runtest_call
hookself.result
I have also combined the
get_marker
andget_compare
functions/methods to simplify these.