Skip to content

Conversation

@edabor
Copy link
Contributor

@edabor edabor commented Aug 30, 2025

Closes #172

@github-actions github-actions bot added the bug Something isn't working label Aug 30, 2025
@codecov-commenter
Copy link

codecov-commenter commented Aug 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.34%. Comparing base (00f3018) to head (4010683).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #201      +/-   ##
==========================================
+ Coverage   95.31%   95.34%   +0.03%     
==========================================
  Files           2        2              
  Lines         256      258       +2     
  Branches       39       39              
==========================================
+ Hits          244      246       +2     
  Misses          6        6              
  Partials        6        6              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@user27182 user27182 left a comment

Choose a reason for hiding this comment

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

Thanks! Coverage is reporting 3 lines missing but it seems broken after #196 merged

@user27182
Copy link
Contributor

I tried this patch with --disallow_unused_cache in #199 and it resolved 16/17 callback errors. But one error still persists with test_screenshot_notebook
https://github.com/pyvista/pytest-pyvista/actions/runs/17352748316/job/49261538806?pr=199

============================= pytest-pyvista ERROR =============================
Unused cached image file(s) detected (1). The following images are
cached, but were not generated or skipped by any of the tests:
['screenshot_notebook.png']

This is the test:
https://github.com/pyvista/pyvista/blob/7e531e3dadd2d78e75a5790be884172ab87c7331/tests/plotting/test_plotting.py#L3116-L3126
It doesn't use auto_close, so not strictly related to #172, but the test uses a fixture specifically to allow unused images, yet an unused image error is being raised.

@edabor
Copy link
Contributor Author

edabor commented Aug 31, 2025

I tried this patch with --disallow_unused_cache in #199 and it resolved 16/17 callback errors. But one error still persists with test_screenshot_notebook https://github.com/pyvista/pytest-pyvista/actions/runs/17352748316/job/49261538806?pr=199

============================= pytest-pyvista ERROR =============================
Unused cached image file(s) detected (1). The following images are
cached, but were not generated or skipped by any of the tests:
['screenshot_notebook.png']

This is the test: https://github.com/pyvista/pyvista/blob/7e531e3dadd2d78e75a5790be884172ab87c7331/tests/plotting/test_plotting.py#L3116-L3126 It doesn't use auto_close, so not strictly related to #172, but the test uses a fixture specifically to allow unused images, yet an unused image error is being raised.

Something very tricky happened here.

No call to the fixture is expected due to verify_image_cache_wrapper.allow_useless_fixture = True, which is indeed occurring because the teardown of no_images_to_verify does not detect any call to the fixture (ie. n_calls == 0).

However, because the plotter is not closed (hard to tell but maybe because of the notebook = True), the check_gc fixture is closing the plotter at teardown.
Therefore, the callback is called and an image is generated, which was stored in the cache when writing the test originally.

https://github.com/pyvista/pyvista/blob/2208861be4edff692c328703f7c245e56e2084c1/tests/plotting/conftest.py#L60-L71

Two options:

  1. add the @pytest.mark.skip_check_gc mark to the test and remove the file from the cache
  2. remove the @pytest.mark.usefixtures('no_images_to_verify') mark and add a pl.close() to make sure the plot is closed by the test and not the check_gc fixture

I vote for option 2

The conclusion is also that the current PR is not responsible of this failure.

@akaszynski
Copy link
Member

remove the @pytest.mark.usefixtures('no_images_to_verify') mark and add a pl.close() to make sure the plot is closed by the test and not the check_gc fixture

Option 2 seems more explicit. Let's go with that.

Copy link
Member

@akaszynski akaszynski left a comment

Choose a reason for hiding this comment

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

LGTM aside from a nitpick.

@user27182
Copy link
Contributor

remove the @pytest.mark.usefixtures('no_images_to_verify') mark and add a pl.close() to make sure the plot is closed by the test and not the check_gc fixture

Option 2 seems more explicit. Let's go with that.

Confirmed that this works. CI is green with the --disallow_unused_cache option enabled in #199 when using the patch from pyvista/pyvista#7886

@akaszynski akaszynski merged commit f956c1e into pyvista:main Sep 1, 2025
9 checks passed
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.

verify_image_cache not called when setting auto_close=False

4 participants