Skip to content

Tests: the eyedropper image-sampling test no longer flakes (colour settings changed by parallel tests) - #61

Merged
bflatastic merged 1 commit into
mainfrom
pr/eyedropper-test-flake
Oct 4, 2026
Merged

bflatastic merged 1 commit into
mainfrom
pr/eyedropper-test-flake

Conversation

@bflatastic

Copy link
Copy Markdown
Member

tests_eyedropper::clicking_an_image_samples_its_pixel failed at random in parallel CI runs (#008181 instead of #008080). This finds the cause and removes it.

  • Cause: all engine unit tests run in one process, and two colour-management tests briefly changed the process-wide Color Settings: the working RGB space and black-point compensation. They held tests_colormgmt::GLOBAL, but every test that shows or renders a colour reads those settings without the lock, so for that moment colours went through the wrong profile.
  • Fix:
    • The two settings-changing tests move to their own test binary (crates/engine/tests/color_settings.rs), so they run in a separate process. A lock couldn't fix this, because every colour reader would need it. No engine unit test now leaves the Color Settings off their defaults.
    • The eyedropper test compares the stored colour instead of its hex string, so it doesn't depend on the settings at all.
    • every_command_survives_empty_params toggles the process-wide proof view (view.proofColors, view.overprintPreview, view.separationsPreview), which other locked tests assert on, so it now takes the existing GLOBAL lock.
  • No commands, parameters or model fields change.

Checks

Rebased onto 0038e00. cargo xtask ci passes all 7 steps (fmt, clippy, test, assets, brands, layers, wasm): 1,886 tests, 0 failed.

Stress runs, with 0 failures in each:

  • the failure reproduced 1 time in 100 filtered runs before the fix;
  • after the fix, 300 full engine runs with 32 threads;
  • 300 filtered runs;
  • four concurrent loops of 60 full runs with 64 threads.

🤖 Generated with Claude Code

The engine unit tests share one process, and two colour-management tests
switched the process-wide Color Settings (the working RGB space to the wide
gamut profile, Black Point Compensation off) for a moment. Any test that
showed or rendered a colour at that moment saw it through the wrong profile:
clicking_an_image_samples_its_pixel read #008181 instead of #008080.

- Move the two tests that change the Color Settings to their own test
  binary (crates/engine/tests/color_settings.rs), so no unit test can see
  non-default settings.
- The image-sampling test compares the stored colour, not its display hex.
- every_command_survives_empty_params toggles the process-wide proof view
  (Proof Colors, Overprint Preview, Separations), so it now takes the
  colour-management test lock like the tests that read that view.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant