Add tests to limit sphinx gallery example execution times#7386
Add tests to limit sphinx gallery example execution times#7386tkoyama010 merged 27 commits intomainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7386 +/- ##
=======================================
Coverage 96.15% 96.15%
=======================================
Files 149 149
Lines 30704 30704
Branches 4026 4026
=======================================
Hits 29524 29524
Misses 567 567
Partials 613 613 |
|
new tests are working, see https://github.com/pyvista/pyvista/actions/runs/14337838724/job/40189412437?pr=7386#step:14:328 FAILED tests/doc/tst_doc_images.py::test_gallery_example_execution_time[backface_props] - AssertionError: Sphinx gallery example 'Setting Backface Properties' from file
../../examples/02-plot/backface_props.py
has an execution time: 385.58251428604126 seconds
which exceeds the maximum allowed: 60.0 seconds.
assert 385.58251428604126 < 60.0
FAILED tests/doc/tst_doc_images.py::test_gallery_example_execution_time[depth_of_field] - AssertionError: Sphinx gallery example 'Depth of Field Plotting' from file
../../examples/02-plot/depth_of_field.py
has an execution time: 792.1083476543427 seconds
which exceeds the maximum allowed: 60.0 seconds.
assert 792.1083476543427 < 60.0
FAILED tests/doc/tst_doc_images.py::test_gallery_example_execution_time[edl] - AssertionError: Sphinx gallery example 'Eye Dome Lighting' from file
../../examples/02-plot/edl.py
has an execution time: 70.12101650238037 seconds
which exceeds the maximum allowed: 60.0 seconds.
assert 70.12101650238037 < 60.0
FAILED tests/doc/tst_doc_images.py::test_gallery_example_execution_time[pbr] - AssertionError: Sphinx gallery example 'Physically Based Rendering' from file
../../examples/02-plot/pbr.py
has an execution time: 765.3375141620636 seconds
which exceeds the maximum allowed: 60.0 seconds.
assert 765.3375141620636 < 60.0
FAILED tests/doc/tst_doc_images.py::test_gallery_example_execution_time[planets] - AssertionError: Sphinx gallery example '3D Earth and Celestial Bodies' from file
../../examples/99-advanced/planets.py
has an execution time: 620.1809523105621 seconds
which exceeds the maximum allowed: 60.0 seconds.
assert 620.1809523105621 < 60.0Thinking of setting the limit to |
tests/doc/tst_doc.py
Outdated
|
|
||
| # Same value as `sphinx_gallery_conf['junit']` in `conf.py` | ||
| SPHINX_GALLERY_CONF_JUNIT = Path('sphinx-gallery') / 'junit-results.xml' | ||
| SPHINX_GALLERY_EXAMPLE_MAX_TIME = 900.0 # Measured in seconds |
There was a problem hiding this comment.
This needs to be set to a reasonable value. From #7397 , the slowest example takes about 2 minutes:
https://67f84843a3147b162189bc78--pyvista-dev.netlify.app/sg_execution_times.html
But I think this is too long. Should we aim to make this 30 seconds? (Plus fudge-factor so maybe warning after 30, fail after 45 seconds?). We can allow exceptions for the few examples that currently exceed this, but put a hard cap on new ones
There was a problem hiding this comment.
I like the idea of enforcing a default time that is quite fast and then a list of slow execution examples that is opt-in via list. I don't think we should exempt them completely, otherwise we may get regressions that creep into those examples making them take longer. This way new examples are by default checked to be fast. If they aren't we can assess whether it should be trimmed down for execution speed or added to the list.
There was a problem hiding this comment.
I like the idea of enforcing a default time that is quite fast and then a list of slow execution examples that is opt-in via list.
If we define "quite fast" to be 10 seconds, I think we can shave about 20 minutes from the build time by skipping any examples that take longer than this to run.
I don't think we should exempt them completely, otherwise we may get regressions that creep into those examples making them take longer.
We can maybe add a new CI env var and corresponding GitHub label SKIP_SLOW to opt-in to skip these? We can couple this with #7392, where the package pytest-fail-slow can be used to mark tests as slow (and optionally skip them). And maybe mirror how the cache labels works, and have skip-slow-doctest, skip-slow-unittest, skip-slow-gallery, etc. labels for controlling this.
This way new examples are by default checked to be fast. If they aren't we can assess whether it should be trimmed down for execution speed or added to the list.
I agree, slow examples should first fail and require adjustments of some kind to make them pass.
There was a problem hiding this comment.
The suggestions above are good and I think more needs to be done to make this all work as we'd like it to. But for this PR, is it OK if we at least enable the tests as-is for now, and simply place a hard limit of maybe 150 seconds (30 seconds slower than the slowest example) ?
There was a problem hiding this comment.
Coming back to this, it seems fine to me to get this in to avoid future problems and then refine later. Will do another review.
That's a good idea. Would also be nice if it worked with abort_build_on_first_fail such that the build fails early instead of having to wait until after the build completes to find that an example took too long. |
Overview
Sphinx gallery generates an xml file with computation times, see https://sphinx-gallery.github.io/stable/configuration.html#junit-xml
This PR parses this xml file and generates pytest cases. The tests fail if they exceed the specified maximum allowed time.