Skip to content

ci: run lint as its own job and spread pytest over the runner's CPUs - #73

Open
doublewhy wants to merge 5 commits into
devfrom
61-ci-critical-path
Open

doublewhy wants to merge 5 commits into
devfrom
61-ci-critical-path

Conversation

@doublewhy

@doublewhy doublewhy commented Oct 9, 2026 •

Copy link
Copy Markdown

Summary

Shortens CI's critical path without removing or weakening a check. ci.yml still runs on every pull request and every push to dev and main, with no path or change-size filter.

  • ruff check and ruff format --check move out of test into a new lint job, which starts with audit and test. A lint failure no longer stops the tests or the Sonar analysis. sonar starts when test passes and analyses its coverage.xml.
  • test runs the same 148 tests across the runner's 4 CPUs with pytest-xdist. Its coverage.xml matched a serial run's line for line in all six measured runs.

Stacked on #74, so merge it after #74. The branch now carries #74's uv.lock fix (commit 6ee2564; #74 is the PR for #76) under this PR's four own commits. That clears CI's audit job, which failed here only on dev's lock. The new lint job's checkout also sets persist-credentials: false.

Coverage is ready (the test verdict, and the earliest sonar start) at a median of 43 s after run start, against 59 s before. The pytest step drops from a 46 s median to 30 s.

Related Issues

Refs #61

Remaining for #61: the after-change median and p95 of final completion and first failure. Both need a full sonar scan, and every run stops before one (HTTP 403) until the maintainer replaces the repository token (#75); no code change can produce them sooner. This PR adds those timings and completes #61 once sonar runs, so it uses Refs #61 until then. #61's first acceptance criterion (required verification before protected-branch integration) also depends on a repository setting: dev and main require no status checks, tracked in #78.

Changes

Measurements

Method: job and step times come from gh run view <id> --json jobs,startedAt, test counts from each test job's log. The clock starts at the run's startedAt, so runner pickup is included. A verdict is the job's completion; first failure is the earliest completion of a failed job. Cancelled runs are excluded. p95 is nearest-rank, so with n < 20 it is the maximum.

Before, for the two measures #61 names, over the last 40 PR runs (2026-07-12 → 2026-10-04): final completion median 89.5 s, p95 132 s; first failure median 68 s, p95 132 s, over the 19 failed runs.

Like-for-like segments. Before: the 33 CI runs (23 PR, 10 push) from 2026-07-15, when the suite reached 148 tests, to 2026-10-04. After: five workflow_dispatch runs of this branch at 84968a6 and this PR's run at 775cdeb.

From run start Before, median / p95 (n=33) After, median / p95 (n=6)
Lint/format verdict 10 s / 115 s 13 s / 16 s
audit verdict (job unchanged) 23 s / 132 s 24 s / 65 s
pytest step 46 s / 47 s 30 s / 32 s
Coverage ready (test verdict) 59 s / 164 s 43 s / 46 s
sonar start after test ends 2 s / 6 s 2 s / 2 s
sonar job with a scan (13 non-Dependabot runs) 70 s / 80 s not measurable (#75)
Final, non-Dependabot runs (n=13) 133 s / 257 s not measurable (#75); about 115 s at the median by construction (43 + 2 + 70)

After first failure: median 24 s, p95 58 s. In five of six after-runs it is audit; in 37901543628 it is the pre-existing sonar 403 (58 s). Compare the medians: the before p95s of 115–257 s come from two push runs that queued behind another push, and six runs on one day is a small sample.

Per-run detail, check names and evaluated alternatives
  • Run list: gh run list -R OpenRAE/workbench --workflow ci.yml --limit 200 --json databaseId,event,headBranch,conclusion,createdAt,startedAt,attempt. Logs: gh api repos/OpenRAE/workbench/actions/jobs/<job>/logs. The lint verdict before is the end of the "Format check" step inside test; after, it is the end of the lint job.
  • The 19 failed runs of the last 40: 7 audit failures on Dependabot PRs (2026-08-09 → 10-04); 7 July Dependabot runs whose Sonar scan ran without a token, before the Dependabot skip step existed; and 5 Sonar quality-gate failures on maintainer PRs ("QUALITY GATE STATUS: FAILED"): 29181741353, 29181887912, 29282821131, 29283061862 and 29299763491. 24 of the 40 runs are Dependabot PRs; 17 of them skip the scan step.
  • All 33 like-for-like before runs logged "collected 148 items" and "148 passed". The after-runs are the dispatch runs 37901326740, 37901437078, 37901543628, 37901672960 and 37901782766, and the PR run 37902221136. Each logged "4 workers [148 items]" and "148 passed" (in 25.4–28.1 s).
  • Before p95s: they come from push runs 29696374943 and 34800652984, whose jobs queued 109–117 s behind an earlier push to dev; ci.yml's concurrency group serializes pushes to one ref. None of the six after-runs queued.
  • Lint verdict: it arrives about 3 s later, when the separate lint job finishes. Before, a failing ruff step ended test within a second or two. What changes is that a lint failure no longer suppresses the pytest result or the Sonar analysis.
  • Sonar quality-gate results, including the 5 gate failures above, can arrive about 16 s earlier at the median, because coverage is ready 16 s sooner.
  • The after-runs' final completion (median 58.5 s, p95 65 s) is not compared: sonar stops at the 403 about 12 s in instead of running a 70 s scan.
  • Check names: lint is new. dev and main have no required status checks (gh api repos/OpenRAE/workbench/branches/<branch>/protection/required_status_checks returns 404 "Required status checks not enabled" for both), so no protection setting depends on the job names. Requiring checks on both branches is tracked in Dev and main require no status checks before merge #78.
  • Not adopted, cross-runner shards: pytest took a median of 46 s for 148 tests, and each extra runner would repeat about 10 s of fixed cost before its first test (runner set-up, checkout, uv sync, pytest and Django start-up, collection: 07:12:45 → 07:12:55 in run 30737366583), with test-database migrations on top. Shard coverage would also need merging before sonar. In-job xdist uses the runner's other three CPUs with no extra runner and no artifact merge.
  • Not adopted, a change-based early lane: most tests drive Django views and models end to end, so no file-to-test map could select tests safely, and the whole suite now reports in about 30 s of pytest time.
  • Not adopted, more caching: setup-uv already caches by default, keyed on a hash of pyproject.toml and uv.lock. A pull request that changes neither file restores dev's cache, as ci: add an OpenSSF Scorecard workflow and README badge #71's run 37899310319 did. It misses when one of them changes, as in this PR's run 37902221136, or when dev's cache has expired after more than seven days unused: PR run 30737366583 missed seven days after 30192386828, the previous run to use that key, and pull requests kept missing until push run 34800634673 saved it again on 2026-09-14. Dependency download plus install takes about 1–2 s per job. The Sonar scanner download and JRE provisioning take about 6.5 s per scan (29382669364: download 01:52:43.6 → 01:52:46.6, JRE 01:52:50.2 → 01:52:53.6). A cache restore would recover only part of that, and caches written by one PR's runs are not visible to other PRs.

Test Plan

  • Local, at 775cdeb: uv lock --check ("Resolved 95 packages"; the lock diff is additions only), uv run ruff check . ("All checks passed!"), uv run ruff format --check . ("47 files already formatted"), actionlint 1.7.12 over all workflows (exit 0), and uvx pre-commit run --files .github/workflows/ci.yml CONTRIBUTING.md pyproject.toml uv.lock (pre-commit 4.6.2, exit 0).
  • Local, at 01643af (a comment in ci.yml, wording in CONTRIBUTING.md and one .gitignore line): actionlint 1.7.12 over all workflows (exit 0); zizmor 1.30.1 reports the same findings on ci.yml as at 775cdeb; uvx pre-commit run --files .github/workflows/ci.yml CONTRIBUTING.md .gitignore (exit 0); ruff 0.15.21, the locked version, through uvx: check . ("All checks passed!") and format --check . ("47 files already formatted"); uv lock --check ("Resolved 95 packages"). Before the .gitignore line, the audit export command from CONTRIBUTING.md left ?? audit-requirements.txt in git status and git check-ignore exited 1. With it, git status stays clean and git check-ignore -v names .gitignore:24.
  • uvx zizmor --offline --format plain (1.30.1) on ci.yml, dev against 8f90950: unpinned-uses 7 → 9 and artipacked 3 → 3. The two new unpinned-uses are the lint job's copies of the existing actions/checkout@v7.0.1 and astral-sh/setup-uv@v9.0.0 references. At 01643af, before the persist-credentials: false commit, artipacked was 4. Pinning every action to a commit SHA, and the persisted-credential, template-injection and cache-poisoning findings in the other workflows, are tracked in Workflow lint reports unpinned actions and persisted checkout credentials #77.
  • xdist against serial on four modules (test_hardening, test_cli, test_collab, test_accounts; 41 tests; -n 4): 41 passed both ways, coverage.xml has identical per-file measured and hit lines, and JUnit has the same 41 unique cases as --collect-only. I did not run the full suite locally.
  • Restack onto chore(deps): update locked cryptography, django, pyjwt and sqlparse for published advisories #74 (head 6ee2564): git rebase --onto origin/uv-lock-vulnerability-fixes origin/dev replayed the three commits without a conflict, and git range-diff reports each unchanged (84968a6 = 7923846, 775cdeb = 7077137, 01643af = 3d6f91c). The rebased tree equals git merge-tree --write-tree of the old head 01643af and 6ee2564 (both 2b35326). uv lock --check resolves 95 packages (exit 0).
  • Local, at 8f90950: actionlint over all workflows (exit 0); uvx pre-commit run --files .github/workflows/ci.yml CONTRIBUTING.md .gitignore pyproject.toml uv.lock (exit 0); uvx ruff@0.15.21 check . ("All checks passed!") and format --check . ("47 files already formatted"); uv run --frozen pytest -n 4 on the same four modules as above, with the merged lock (Django 5.2.18, cryptography 50.0.2, pytest-xdist 3.8.0): 41 passed.
  • Audit, as CI runs it: uv export --frozen --no-dev --all-extras --no-emit-project --no-hashes --format requirements-txt, then uvx pip-audit --strict --no-deps on the export printed "No known vulnerabilities found" (exit 0). Locally I added --disable-pip, because pip-audit's temporary virtual environment aborts in ensurepip on this machine; with --no-deps it audits the same pins. The export's pins equal chore(deps): update locked cryptography, django, pyjwt and sqlparse for published advisories #74's. On dev's lock the same commands print "Found 27 known vulnerabilities in 4 packages" (exit 1).
  • Overlaps at 8f90950: with chore: bump astral-sh/setup-uv from 9.0.0 to 10.2.0 #69's head (6448554) git merge-tree reports a content conflict in ci.yml; chore: update django requirement from <6,>=5.1 to >=5.1,<7 #63, chore: bump SonarSource/sonarqube-scan-action from 8.2.1 to 8.3.0 #70, ci: add an OpenSSF Scorecard workflow and README badge #71 and chore: use one pre-push Codex review and disable automatic review disposition #72 merge cleanly.
  • CI: lint and test passed in all six runs above, each with "148 passed" and TOTAL 1958 statements, 83 missed, 96%. Each run's coverage-report artifact matches the serial run 37899310319 from ci: add an OpenSSF Scorecard workflow and README badge #71, which has the same src/ and tests/ as dev: identical per-file measured and hit lines, 1,875 of 1,958 lines hit across 28 files. At 01643af, run 38076112589: lint and test pass ("4 workers [148 items]", "148 passed", TOTAL 1958 statements, 83 missed, 96%). That run is not in the timing sample, which stays the six runs above.
  • CI at 8f90950, run 38196396994: audit passes ("No known vulnerabilities found"), and lint and test pass ("4 workers [148 items]", "148 passed", TOTAL 1958 statements, 83 missed, 96%). PR title guard (38196396916), CodeQL actions, javascript-typescript and python (38196393933) and GitGuardian also pass. That run is not in the timing sample.
  • sonar is the only red check, and no code change can fix it: it stops before analysis because SonarCloud rejects the repository SONAR_TOKEN ("HTTP 403 Forbidden" in run 38196396994, as in the earlier run 38076112589). Only the maintainer can replace the secret; tracked in ci: SonarCloud rejects the repository SONAR_TOKEN #75.

…or published advisories

Upgrade only these four uv.lock entries, within the existing pyproject.toml
constraints:

- cryptography 49.0.0 -> 50.0.2 (PYSEC-2026-3552)
- django 5.2.16 -> 5.2.18 (PYSEC-2026-3717, PYSEC-2026-4035)
- pyjwt 2.13.0 -> 2.15.1 (PYSEC-2026-4140 to PYSEC-2026-4152, PYSEC-2026-4183)
- sqlparse 0.5.5 -> 0.6.0 (PYSEC-2026-3696 to PYSEC-2026-3699, PYSEC-2026-3923)

CI's audit job runs pip-audit --strict over the locked runtime dependencies
and fails on dev with 27 known vulnerabilities in these packages (22 distinct
advisories: pip-audit lists PYSEC-2026-3552 and PYSEC-2026-3696 to
PYSEC-2026-3699 twice); with this lock it reports none. No other package is
added, removed or moved.

Closes #76
Move ruff check and ruff format --check out of the test job into a lint
job that starts with audit and test. A lint failure no longer prevents
the tests from running or SonarCloud from analysing them, and sonar now
waits only for the coverage report it reads.

Run the suite with pytest-xdist (-n auto) in the single test job. Every
collected test still runs exactly once, and pytest-cov combines the
workers' data into the same coverage.xml, so the quality-gate inputs are
unchanged. pytest-xdist joins the dev dependency group; the lock adds
pytest-xdist 3.8.0 and execnet 2.1.2 and changes nothing else.

Document the CI jobs, their dependencies, test ownership and the local
reproduction commands in CONTRIBUTING.md.
sonar runs only when test passes, and Dependabot pull requests skip its scan step. A failing test still stops sonar, so a single push does not report every failing check.
Pushes trigger ci.yml only on dev and main, and sonar runs only when
test passes, because it needs test. Say so in the comment above the
jobs and in the fast-feedback paragraph of CONTRIBUTING.md, instead of
saying that every job runs on every push and that sonar waits only for
the coverage report.

CONTRIBUTING.md no longer calls ci.yml the full gate: dev and main have
no required status checks, and the PR title guard is a separate
workflow. It now says what holds for every change: no job is skipped
by path, by change scope or by the size of a change.

The reproduce command in the audit row writes audit-requirements.txt
into the working tree. Ignore that file so that running the command
leaves the tree clean.
The lint job only runs ruff, so it never needs the token that
actions/checkout writes into .git/config by default. Setting
persist-credentials: false removes zizmor's artipacked finding for the
new job, so ci.yml is back to dev's three artipacked findings (audit,
test and sonar).

This branch has not been deployed

No deployments
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