test: skip kaleido image-export tests when no browser is available - #833
Merged
Conversation
kaleido v1 renders via a headless Chrome subprocess. In a dev environment with the 'plot' extra installed but no Chrome/Chromium (and no browser downloaded via plotly_get_chrome), to_image()/write_image() block until the 300s watchdog kills them — make test fails on a 5-minute hang. Add a _chrome_available() guard that resolves a browser the same way kaleido does (choreographer's chromium locator, honouring $BROWSER_PATH and the choreo download cache) and skips the 5 image-export tests cleanly when none is found. If choreographer's API ever moves, it falls back to NOT skipping so CI — where Chrome ships on the runners — never silently loses coverage. Verified: tests skip in 0.05s locally; full suite 1210 passed, 5 skipped; coverage still 100%.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes the Kaleido image-export tests resilient in developer environments where kaleido is installed but Chrome/Chromium is not present (which otherwise can cause to_image()/write_image() to hang until the pytest timeout triggers).
Changes:
- Add a
_chrome_available()helper to detect whether Kaleido can locate a Chromium-based browser via choreographer. - Skip Kaleido image-export tests when no browser is available (in addition to the existing Windows skip), avoiding long hangs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+29
to
+30
| if os.environ.get("BROWSER_PATH"): | ||
| return True |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
make testfailed locally with a 300-second hang intest_kaleido.py::test_portfolio_snapshot_write_image. Root cause: kaleido v1renders images by driving a headless Chrome subprocess. In a dev
environment where the
plotextra (kaleido) is installed but no Chrome/Chromiumis present — and none has been downloaded via
plotly_get_chrome—to_image()/write_image()block forever and thepytest-timeoutwatchdogkills them at 300s. The same tests pass on CI, where GitHub runners ship Chrome.
The existing guards covered kaleido-not-installed (
importorskip) andWindows (
skipif), but not kaleido-installed-but-no-browser.Change
tests/test_jquantstats/test__plots/test_kaleido.py: add a_chrome_available()helper and a
skipifmarker. It resolves a browser exactly the way kaleidodoes —
choreographer.browsers.chromium.get_browser_path(...), which honours$BROWSER_PATHand the choreographer download cache — and skips the 5image-export tests cleanly when none is found. If choreographer's API ever
moves, the helper falls back to not skipping, so CI never silently loses
coverage.
Verification
make fmt: ✅ green.make test: ✅ exit 0 — 1210 passed, 5 skipped, no failures.src/line.Scope: locally-owned
tests/only; nosrc/or Rhiza-managed files touched.🤖 Generated with Claude Code