Skip to content

ci: cut queue pressure in unit-tests (#1052) - #1055

Merged
JarryShaw merged 1 commit into
mainfrom
ci/1052-unit-tests-queue
Oct 6, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
ci/1052-unit-tests-queue

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

Part of #1052: --verbose from A, plus C, E, G and H. B, D and F follow in sibling PRs.

What is the purpose of your pull request?

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • chore — anything else

Description of your pull request and other information

  • E: engine-tests is now one job per engine. Each job loops over 3.10–3.15, one venv per Python version, and every cell gets its own log group, annotations and summary row. 3.15 runs as a separate step with continue-on-error. No required check names an engine job.
  • G: a new changes job classifies the PR diff. On a docs-only PR the four test legs are skipped and project-tests runs instead. required-checks accepts skipped only in that case.
  • H/A/C: unittest-ordering no longer runs on PRs. Its run step gets --verbose and a 20-minute step cap. The apt steps are capped at 5 minutes, with one bounded retry; the slowest of 120 observed runs took 34s.
  • tests/project/test_unit_tests_queue.py (30 tests) runs the gate shell, the classifier (against a scratch merge commit), the cell loops and the apt retry, and checks that project-tests runs every test that reads docs/ or Markdown.

Unit Tests jobs per code PR push: 62 listed / 61 run before, 24 listed / 21 run after (docs-only, by construction: 4). On b00096a59, each engine job took 2.2–3.8 min, all 35 checks passed, and the Python 3.14 leg needed one re-run: the first attempt was killed by a runner shutdown at 41 min.

@JarryShaw JarryShaw added ci Pull requests that change CI or workflow configuration (ci: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 6, 2026
@JarryShaw JarryShaw added this to the 1.5 milestone Oct 6, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 6, 2026
@JarryShaw
JarryShaw force-pushed the ci/1052-unit-tests-queue branch from b00096a to 442ebcd Compare October 6, 2026 03:44
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 6, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 442ebcd4f: NEEDS CHANGES (ran on Sonnet; author Opus)

  1. A docs-only PR can break a test that never runs. tests/corekit/test_sentinel_exports_unit.py:210 reads docs/source/contributing/conventions/sentinel-convention.rst. Change NULL to NULLX there and that module fails, yet the project leg that project-tests runs still passes 530 of 530. The PR would classify as docs-only, skip test, and pass required-checks. Fix: run every test that reads docs/ on docs-only PRs, or count docs/source/contributing/conventions/ as code, and correct the stated justification.
  2. Paths with spaces are misclassified. The classifier splits paths on whitespace (unit-tests.yml:1012, diff.stdout.split()), so a root-level file named x.md docs/y reads as docs. Use git diff -z and split on NUL.
  3. 8 of 19 mutations survive test_unit_tests_queue.py. The survivors: the OWN_PR/workflow check, the HEAD^1 base, the empty-diff-is-code fallback, || true on the engine cell, the failed[] exit, the apt retry count, the apt step cap, and the whitespace split.

Confirmed:

  • All 36 old engine cells match the 36 new ones on pass/skip counts (main run 37407908831 against PR run 37410433253).
  • A failing cell fails the step. The 3.15 continue-on-error applies to that step only.
  • All four workflow_call callers pass gate-only: true.
  • The required contexts are unchanged.
  • No new ${{ }} appears in run:.
  • Every docs line reference matches.
  • Merging with ci(coverage): report test coverage from a workflow (#1063) #1064: unit-tests.yml merges cleanly; workflows.rst (6 hunks) and releasing.rst (2) conflict on line citations.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 6, 2026
@JarryShaw
JarryShaw force-pushed the ci/1052-unit-tests-queue branch from 442ebcd to 1f075fb Compare October 6, 2026 04:15
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 6, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 1f075fb94: NEEDS CHANGES (ran on Sonnet; author Opus)

  • Blocking (mechanical): the new project-tests step adds 10 lines, and the docs citations below it were not renumbered. Each is 10 lines off:

    Location Cites Should be
    releasing.rst:49 unit-tests.yml:1065-1134 1075-1144 (gate)
    workflows.rst:347 and :365 1238 1248
    workflows.rst:370 1137 1147
    workflows.rst:374 1136-1157 1146-1167
    workflows.rst:384 1237-1291 1247-1301
    workflows.rst:390 1240 1250

    I confirmed it myself: gate sits at 1075 and required-checks at 1247.

  • All three earlier points are fixed:

    • The corekit docs reader now runs on docs-only PRs. A mutated sentinel-convention.rst exits 1 there.
    • The classifier splits on NUL, so x.md docs/y now reads as code.
    • 30 executing tests now catch all 8 earlier surviving mutations.
  • No missed docs reader found across tests/, pcapkit/ and util/.

  • Non-blocking mutation survivors: continue-on-error on either project-tests step, the dpkg --configure -a removal, and required-checks dropping one leg.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 6, 2026
@JarryShaw
JarryShaw force-pushed the ci/1052-unit-tests-queue branch from 1f075fb to 9224e70 Compare October 6, 2026 04:27
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 6, 2026
- engine-tests: fold the 36 Python x engine jobs into one job per engine that loops over 3.10-3.15 in fresh venvs; 3.15 is a step with continue-on-error.
- changes/project-tests: classify PR diffs; a docs-only PR skips test, integration, engine-tests and pypcap-parity and runs tests/project instead.
- required-checks: accept skipped legs only when changes reports a docs-only diff, and then require project-tests.
- unittest-ordering: off pull requests; --verbose and a 20-minute step cap on its run step.
- apt steps: 5-minute cap with one bounded retry of update+install.
- docs: refresh the unit-tests.yml citations in workflows.rst and releasing.rst; add tests/project/test_unit_tests_queue.py.
@JarryShaw
JarryShaw force-pushed the ci/1052-unit-tests-queue branch from 9224e70 to c7a047e Compare October 6, 2026 04:52
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Coverage: 88.74% (unit tier, Python 3.14, c7a047efc, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 4 93.55%
pcapkit/const 18757 1035 2342 846 90.23%
pcapkit/corekit 1874 91 578 22 94.33%
pcapkit/dumpkit 136 0 40 0 100.00%
pcapkit/foundation 2422 143 842 37 92.52%
pcapkit/interface 112 7 40 5 92.11%
pcapkit/protocols 15653 187 3942 162 98.19%
pcapkit/toolkit 487 71 144 3 84.15%
pcapkit/utilities 429 4 122 4 98.55%
pcapkit/vendor 4409 2359 1006 158 42.84%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on c7a047efc: GOOD TO GO (ran on Sonnet; author Opus). It is confirmed by this head's own CI: Required checks passed is green.

  • Rebase onto 62c9878e6 is clean. The only changes beyond main's own are resolved conflicts and renumbered citations in releasing.rst and workflows.rst. The test file is byte-identical to the reviewed head. ci(coverage): report test coverage from a workflow (#1063) #1064's coverage leg, steps and docs section are intact.
  • Fixed across rounds:
    • the corekit docs reader now runs on docs-only PRs;
    • the classifier splits on NUL;
    • 33 executing tests catch every mutation from the earlier rounds;
    • every unit-tests.yml:N citation is pinned by content.
  • Engines: 36 engine jobs became 6 Engines (<engine>) jobs, and the pass/skip counts per cell match the old jobs.
  • Live CI check: the first coverage-comment.yml comment posted on this PR (88.74%, from github-actions[bot]), so ci(coverage): report test coverage from a workflow (#1063) #1064's workflow_run path works.
  • Non-blocking survivors:
    • an || true on the project leg step;
    • dropping dpkg --configure -a between apt attempts.

@JarryShaw JarryShaw removed the review: running A cross-review is in flight against the current head - no verdict yet label Oct 6, 2026
@JarryShaw JarryShaw added the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 6, 2026
@JarryShaw
JarryShaw merged commit 7958b64 into main Oct 6, 2026
38 of 40 checks passed
@JarryShaw
JarryShaw deleted the ci/1052-unit-tests-queue branch October 6, 2026 13:20
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Pull requests that change CI or workflow configuration (ci: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant