ci(perf): fix Playwright/mypy caching and add timeouts to prevent hangs - #352
Merged
Merged
Conversation
tests-integration ran playwright install --with-deps chromium — a multi-minute apt-get + browser-binary download with zero caching — before every run, despite zero integration-marked tests importing Playwright anywhere in the repo (grep confirmed across tests/ and core/tests/). Browser-driving coverage already lives in the tests-e2e job, which keeps its own copy of this step.
Reverts the previous commit's premise: grepping test files for "import playwright" missed that core/tests/test_exports.py's PDF export tests call application code (core/views/__init__.py, sync_ playwright + chromium.launch) that needs a real browser, even though the test file itself never imports playwright directly. Removing the install step broke 3 previously-passing tests (confirmed on this PR's own CI run: test_pdf_export_returns_200/content_type_is_pdf/ includes_financing_section all failed with the browser executable missing). The actual fix for the slow step is caching ~/.cache/ms-playwright keyed on the pinned playwright version (1.62.0) so the ~150-300MB browser download only happens once per cache key instead of every run. System deps (apt-get, not cacheable across fresh runner VMs) still install every run via the lighter `install-deps` on cache hit.
paruff
changed the title
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
1 participant
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
Started as a single Playwright-removal fix; grew into a full CI suite audit after that fix broke tests and a separate job was found hung for 1h16m with no timeout to kill it.
actions/cachefor~/.cache/ms-playwrightso the ~150-300MB browser download only happens once per cache key..mypy_cachecaching — this was the actual critical path of the whole PR-gate suite (~16 min cold, zero cache persistence across runs previously, and every job runs in parallel so this alone determined total wall time).timeout-minutes. Root cause of the observed hour-plus hang — only 2 of 37 jobs had any timeout; GitHub defaults to 360 minutes when unset, so any hang (interactive apt/dpkg prompt, network stall) blocks a PR indefinitely with zero automatic recovery. Sized off this session's observed real durations for ci-quality.yml, conservative defaults elsewhere.Expected outcome: total PR-gate wall time drops from ~16-18 min to roughly the new longest job (ZAP Scan, ~7-8 min) — plus no more silent multi-hour hangs.
Test plan
yaml.safe_load)pre-commit runacross all changed files — cleantimeout-minutes(job count == timeout-minutes line count per file)build-image, post-deploymentsecurity) untouched