Compare best-match cached baseline first for multi-cache tests#295
Compare best-match cached baseline first for multi-cache tests#295banesullivan wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (84.61%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #295 +/- ##
==========================================
- Coverage 96.05% 95.60% -0.45%
==========================================
Files 4 4
Lines 988 1024 +36
Branches 126 132 +6
==========================================
+ Hits 949 979 +30
- Misses 29 32 +3
- Partials 10 13 +3 🚀 New features to boost your workflow:
|
| def test_cached_image_sort_key_ranks_current_env_first() -> None: | ||
| """The current-OS + current-VTK baseline must sort before the rest.""" | ||
| env_info = _EnvInfo() | ||
| system = platform.system() | ||
| os_token = {"Darwin": "macOS", "Linux": "linux", "Windows": "windows"}.get(system, system.lower()) | ||
| vtk_major, vtk_minor = pv.vtk_version_info[:2] | ||
|
|
||
| correct = Path(f"{os_token}-vtk-{vtk_major}.{vtk_minor}.png") | ||
| wrong_os = Path(f"otheros-vtk-{vtk_major}.{vtk_minor}.png") | ||
| no_tokens = Path("totally-unrelated-name.png") | ||
| paths = [wrong_os, no_tokens, correct] | ||
|
|
||
| ordered = sorted(paths, key=lambda p: _cached_image_sort_key(p, env_info)) | ||
| assert ordered[0] == correct | ||
| # An unparsable name must not raise and must sort last. | ||
| assert _cached_image_sort_key(no_tokens, env_info) # does not raise | ||
| assert ordered[-1] == no_tokens |
There was a problem hiding this comment.
Highly recommend using str(_EnvInfo() here somewhere (or in a separate test), since this is how actual files are named with the --generate_subdirs option enabled. So we should make sure that the naming conventions used by that option work with the new sorting. Otherwise, are we relying on some sort of assumption for how users should otherwise format these file names, if they're not using --generate_subdirs to name them? Should we document that file names with specific tokens are sorted by priority when there are multiple cached images?
E.g.
correct = Path(f"{env_info}.png")Which looks something like
macOS-23.4.0_arm64_gpu-Apple_py-3.14.0_pyvista-0.49.dev0_vtk-9.6.1_no-CI.png
3 participants
A test with multiple cached baselines (one per OS or VTK version) compared against the lexicographically first image. On a platform whose baseline sorted later, the first comparison failed and the test passed against the correct image but was downgraded to a noisy warning, even though the cache was correct. PyVista CI was full of these.
Cached candidates are now ranked by environment match (exact env string, current OS with darwin/macos aliasing, then exact and nearest VTK version) before comparison, so the right baseline is checked first and a correctly cached test stays silent. Token matching is boundary-aware so names like
non-linux-fooormyvtk-1.2do not false-match.Behavior note: for an existing multi-image cache the first compared image changes. It still falls back to every candidate, so pass/fail outcomes are unchanged; only the comparison order and the spurious warnings differ. Single-image caches and the string/None paths keep the previous lexicographic order.
resolves #286