Skip to content

Conversation

@akaszynski
Copy link
Member

@akaszynski akaszynski commented Aug 13, 2025

Resolve #195 by inelegantly using a shared file with a simple locking mechanism (fcntl) to sync image names across multiple pytest-xdist nodes. I would have preferred using shared memory, but I can't seem to figure out how to hook into pytest-xdist's hooks correctly. To avoid disk writing, the file is only written once per worker.

Also, this PR checks if downstream tests can be run in parallel with pytest-xdist. Side effect in that test time is cut in half.

@github-actions github-actions bot added maintenance bug Something isn't working labels Aug 13, 2025
@codecov-commenter
Copy link

codecov-commenter commented Aug 13, 2025

Codecov Report

❌ Patch coverage is 23.07692% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 27.73%. Comparing base (114b35a) to head (be999ff).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pytest_pyvista/pytest_pyvista.py 23.07% 27 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #196       +/-   ##
===========================================
- Coverage   98.18%   27.73%   -70.45%     
===========================================
  Files           2        2               
  Lines         220      256       +36     
  Branches       32       39        +7     
===========================================
- Hits          216       71      -145     
- Misses          2      181      +179     
- Partials        2        4        +2     

☔ 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.

@akaszynski akaszynski changed the title test parallel in downstream tests Sync visited and shared images across pytest-xdist nodes Aug 13, 2025
@akaszynski
Copy link
Member Author

Thanks @user27182 for 1762794. Way better approach.

@akaszynski
Copy link
Member Author

This is ready for review. I really wanted to use pytest-xdist's plugin, but just couldn't figure out how in a reasonable amount of time. This PR relies on a deterministic location using pytest's cache to avoid creating new directories.

@akaszynski
Copy link
Member Author

Idea to add in --disallow_unused_cache for downstream tests. Looks like 17 images weren't registered as being generated despite zero failures. I'll debug.

One note: the default summary seems to be overwritten when reporting disallowed unused:

2025-08-14T05:22:27.9985911Z =============================== warnings summary ===============================
2025-08-14T05:22:27.9986786Z tests/plotting/test_plotting.py::test_user_matrix_silhouette
2025-08-14T05:22:27.9988961Z   /home/runner/work/pytest-pyvista/pytest-pyvista/pytest_pyvista/pytest_pyvista.py:444: UserWarning: test_user_matrix_silhouette Exceeded image regression warning of 200.0 with an image error of 348.8431372549011
2025-08-14T05:22:27.9990608Z     f(plotter)
2025-08-14T05:22:27.9990821Z 
2025-08-14T05:22:27.9991059Z tests/plotting/test_plotting.py::test_chart_plot
2025-08-14T05:22:27.9993062Z   /home/runner/work/pytest-pyvista/pytest-pyvista/pytest_pyvista/pytest_pyvista.py:444: UserWarning: test_chart_plot Exceeded image regression warning of 200.0 with an image error of 299.6196078431372
2025-08-14T05:22:27.9994322Z     f(plotter)
2025-08-14T05:22:27.9994504Z 
2025-08-14T05:22:27.9994816Z tests/plotting/test_plotting.py::test_set_environment_texture_cubemap[0.5]
2025-08-14T05:22:27.9996662Z   /home/runner/work/pytest-pyvista/pytest-pyvista/pytest_pyvista/pytest_pyvista.py:444: UserWarning: test_set_environment_texture_cubemap[0.5] Exceeded image regression warning of 200.0 with an image error of 460.15816993470435
2025-08-14T05:22:27.9998318Z     f(plotter)
2025-08-14T05:22:27.9998457Z 
2025-08-14T05:22:27.9998687Z tests/plotting/test_plotting.py::test_create_axes_orientation_box
2025-08-14T05:22:27.9999699Z   /home/runner/work/pytest-pyvista/pytest-pyvista/pytest_pyvista/pytest_pyvista.py:444: UserWarning: test_create_axes_orientation_box Exceeded image regression warning of 200.0 with an image error of 220.53725490196027
2025-08-14T05:22:28.0000482Z     f(plotter)
2025-08-14T05:22:28.0000590Z 
2025-08-14T05:22:28.0000865Z tests/plotting/test_plotting.py::test_orthogonal_planes_source_resolution[x_resolution]
2025-08-14T05:22:28.0001870Z   /home/runner/work/pytest-pyvista/pytest-pyvista/pytest_pyvista/pytest_pyvista.py:444: UserWarning: test_orthogonal_planes_source_resolution[x_resolution] Exceeded image regression warning of 200.0 with an image error of 228.0352941176472
2025-08-14T05:22:28.0002707Z     f(plotter)
2025-08-14T05:22:28.0002809Z 
2025-08-14T05:22:28.0003034Z tests/plotting/test_plotting.py::test_orthogonal_planes_source_resolution[y_resolution]
2025-08-14T05:22:28.0004024Z   /home/runner/work/pytest-pyvista/pytest-pyvista/pytest_pyvista/pytest_pyvista.py:444: UserWarning: test_orthogonal_planes_source_resolution[y_resolution] Exceeded image regression warning of 200.0 with an image error of 228.0274509803923
2025-08-14T05:22:28.0004849Z     f(plotter)
2025-08-14T05:22:28.0004945Z 
2025-08-14T05:22:28.0005375Z tests/plotting/test_plotting.py::test_orthogonal_planes_source_resolution[z_resolution]
2025-08-14T05:22:28.0006390Z   /home/runner/work/pytest-pyvista/pytest-pyvista/pytest_pyvista/pytest_pyvista.py:444: UserWarning: test_orthogonal_planes_source_resolution[z_resolution] Exceeded image regression warning of 200.0 with an image error of 201.6431372549022
2025-08-14T05:22:28.0007209Z     f(plotter)
2025-08-14T05:22:28.0007312Z 
2025-08-14T05:22:28.0007458Z tests/plotting/test_plotting.py::test_bounding_box[frame]
2025-08-14T05:22:28.0008270Z   /home/runner/work/pytest-pyvista/pytest-pyvista/pytest_pyvista/pytest_pyvista.py:444: UserWarning: test_bounding_box[frame] Exceeded image regression warning of 200.0 with an image error of 447.77777777778664
2025-08-14T05:22:28.0008994Z     f(plotter)
2025-08-14T05:22:28.0009094Z 
2025-08-14T05:22:28.0009289Z -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
2025-08-14T05:22:28.0009671Z ============================= pytest-pyvista ERROR =============================
2025-08-14T05:22:28.0010039Z Unused cached image file(s) detected (17). The following images are
2025-08-14T05:22:28.0010423Z cached, but were not generated or skipped by any of the tests:
2025-08-14T05:22:28.0012096Z ['enable_custom_trackball_style_1.png', 'enable_custom_trackball_style_2.png', 'enable_custom_trackball_style_3.png', 'enable_custom_trackball_style_4.png', 'enable_custom_trackball_style_5.png', 'enable_custom_trackball_style_6.png', 'enable_custom_trackball_style_7.png', 'enable_custom_trackball_style_8.png', 'enable_custom_trackball_style_9.png', 'fly_to_mouse_position.png', 'fly_to_right_click.png', 'fly_to_right_click_multi_render.png', 'image_properties_1.png', 'image_properties_2.png', 'interactive_update_1.png', 'plot_update.png', 'screenshot_notebook.png']
2025-08-14T05:22:28.0013631Z 
2025-08-14T05:22:28.0013812Z These images should either be removed from the cache, or the corresponding
2025-08-14T05:22:28.0014239Z tests should be modified to ensure an image is generated for comparison.
2025-08-14T05:22:28.0014760Z =========================== short test summary info ============================
2025-08-14T05:22:28.0015104Z SKIPPED [1] tests/plotting/jupyter/test_trame.py:284: #5262
2025-08-14T05:22:28.0015700Z SKIPPED [1] tests/conftest.py:404: title offset is a tuple of floats from vtk >= 9.3
2025-08-14T05:22:28.0016185Z SKIPPED [1] tests/plotting/test_plotting.py:685: Does not display correctly within OSMesa
2025-08-14T05:22:28.0016703Z SKIPPED [1] tests/plotting/test_plotting.py:3229: Does not display correctly within OSMesa
2025-08-14T05:22:28.0017338Z SKIPPED [1] tests/plotting/test_plotting.py:3251: Does not display correctly within OSMesa
2025-08-14T05:22:28.0017766Z SKIPPED [1] tests/conftest.py:404: This is broken on VTK 9.3
2025-08-14T05:22:28.0018186Z SKIPPED [1] tests/plotting/test_plotting.py:5242: Does not display correctly within OSMesa
2025-08-14T05:22:28.0019027Z XFAIL tests/plotting/test_plotting.py::test_direction_objects[Capsule_legacy-pos] - Test capsule separately for different vtk versions. Expected to fail if testing with wrong version.
2025-08-14T05:22:28.0020109Z XFAIL tests/plotting/test_plotting.py::test_direction_objects[Capsule_legacy-neg] - Test capsule separately for different vtk versions. Expected to fail if testing with wrong version.

It would be nice to see the test summary and we should add this back in either this or a followup PR.

@user27182
Copy link
Contributor

Idea to add in --disallow_unused_cache for downstream tests. Looks like 17 images weren't registered as being generated despite zero failures. I'll debug.

2025-08-14T05:22:28.0009671Z ============================= pytest-pyvista ERROR =============================
2025-08-14T05:22:28.0010039Z Unused cached image file(s) detected (17). The following images are
2025-08-14T05:22:28.0010423Z cached, but were not generated or skipped by any of the tests:
2025-08-14T05:22:28.0012096Z ['enable_custom_trackball_style_1.png', 'enable_custom_trackball_style_2.png', 'enable_custom_trackball_style_3.png', 'enable_custom_trackball_style_4.png', 'enable_custom_trackball_style_5.png', 'enable_custom_trackball_style_6.png', 'enable_custom_trackball_style_7.png', 'enable_custom_trackball_style_8.png', 'enable_custom_trackball_style_9.png', 'fly_to_mouse_position.png', 'fly_to_right_click.png', 'fly_to_right_click_multi_render.png', 'image_properties_1.png', 'image_properties_2.png', 'interactive_update_1.png',

A lot of these tests seem to not use basic plot() or show(), but instead use auto_close=False and/or call close() manually. I wonder if this is somehow related to #172 ?

@akaszynski
Copy link
Member Author

A lot of these tests seem to not use basic plot() or show(), but instead use auto_close=False and/or call close() manually. I wonder if this is somehow related to #172 ?

That might be it.

@user27182
Copy link
Contributor

A lot of these tests seem to not use basic plot() or show(), but instead use auto_close=False and/or call close() manually. I wonder if this is somehow related to #172 ?

That might be it.

@akaszynski try reverting #189, might temporarily fix this until the issue can be resolved, see #199

@user27182
Copy link
Contributor

Instead of reverting the commit from #189, we should instead revert the commit from this PR that added the test coverage for --disallow_unused_cache, i.e. revert c8c3a10

This option is known to currently be broken in main, see #199

Once reverted, CI should be green again, and I'll approve this PR

@akaszynski akaszynski merged commit 94813c8 into main Aug 29, 2025
9 checks passed
@akaszynski akaszynski deleted the fix/parallel branch August 29, 2025 20:16
@user27182
Copy link
Contributor

Codecov Report

❌ Patch coverage is 23.07692% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 27.73%. Comparing base (114b35a) to head (be999ff).

Files with missing lines Patch % Lines
pytest_pyvista/pytest_pyvista.py 23.07% 27 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #196       +/-   ##
===========================================
- Coverage   98.18%   27.73%   -70.45%     
===========================================
  Files           2        2               
  Lines         220      256       +36     
  Branches       32       39        +7     
===========================================
- Hits          216       71      -145     
- Misses          2      181      +179     
- Partials        2        4        +2     

☔ 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.

Looks like code coverage was negatively impacted by this PR

@user27182
Copy link
Contributor

Looks like code coverage was negatively impacted by this PR

Can confirm that removing -n2 restores coverage. From the Coverage docs it seems we need to use --parallel-mode and combine separate coverage reports from each worker. This is tried in #202 but it's not working 🤷‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

disallow_unused_cache is not compatible with pytest-xdist

5 participants