Skip to content

ci(unit-tests): stop the integration job re-running the unit tier - #727

Merged
JarryShaw merged 1 commit into
mainfrom
ci-partition-integration-tier
Sep 24, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
ci-partition-integration-tier

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly

  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])

  • Use Preview tab to see how your pull request will actually look like

  • Searched for similar pull requests

  • Followed the coding style (make pylint, make mypy, make isort) -- N/A: those targets cover pcapkit/ only. Ran isort --check-only (clean) and py_compile by hand on both changed .py files.

  • make test passes, and a test case covers the change -- not run in full. tests/test_tier_guard.py: 36 passed, 57 subtests. New selection, run for real in a venv built from the exact pip install -e '.[test,Scapy]' the integration/gate jobs use: 142 passed, 31 skipped, 0 failed (see caveat below on the skip count). Unit job's own selection over the 2 affected files: 131 passed, 3 skipped.

  • Added a changelog entry -- N/A — changelog centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

  • ci — workflows or build tooling

Description

Part of #715. integration ran the whole suite on every matrix version despite test already running the unit tier for real on the same commit (~130 min/PR push wasted). Fix:

Job Before After
integration bare pytest -q (whole suite) positive selection via tests._tiers.fixture_tier_paths()
gate bare pytest -q unchanged — deliberate full-suite re-verification, not per-push, out of scope
  • Selection = tests/integration + every *_runtime.py/*_regression.py file + precise node IDs for tests that only skip (not read) a generated capture in test.
  • Skip-set diff (measured, not estimated): exactly 3 methods in 2 files — test_dpkt_unit.py:444,812, test_pcapng_unit.py:3058. Resolved to node IDs, not whole modules, so they don't drag in ~95 unrelated tests.
  • Caveat found by cross-review, independently reproduced: 2 of those 3 node IDs are in test_dpkt_unit.py, whose classes carry @unittest.skipUnless(HAS_DPKT, ...). dpkt sits behind the separate DPKT extra, which neither integration nor gate installs (.[test,Scapy]) — true before this PR too, so not a regression, but it means only the test_pcapng_unit.py node ID actually executes for real in CI today; the 2 dpkt ones stay skipped there exactly as before, and only run for real in a fuller venv (or once/if DPKT is added to that install line, a separate, out-of-scope change). The selection is still correct and forward-compatible either way.
  • Selection is asked of tests/_tiers.py at CI run time, not hand-copied into YAML — can't drift. WorkflowAgreementTests now polices this structurally; verified it fails against a deliberately regressed workflow.
  • timeout-minutes: 45 untouched (CI: raise timeout-minutes on the three pytest jobs from 30 to 45 #713/ci(unit-tests): raise pytest job timeouts from 30 to 45 minutes #716's territory) — with this partition integration drops to ~2 min/version, so 45 is generous headroom, not a tight fit.

Composes with #725 (open, ci-xdist-drop-py315-matrix): both edit integration's "Run full test suite" step. On merge, keep this PR's guard-check structure and add -n auto --dist load to the final pytest -q $selection line. #725's "Report available parallelism" step is untouched and stays put.

Not closing #715 — item 1 (purge_modules to class scope) and item 2 (stop gate re-running on main pushes) remain open.

@JarryShaw JarryShaw added perf Pull requests that improve performance (perf: subject prefix) ci Pull requests that change CI or workflow configuration (ci: subject prefix) labels Sep 24, 2026
- Replace the `integration` job's bare `pytest -q` (the whole suite) with a
  positive selection: `tests/integration`, every `*_runtime.py`/
  `*_regression.py` file, plus the exact test methods that skip a generated
  capture in the `test` job and only run for real once fixtures exist.
  Computed at run time via a new `tests._tiers.fixture_tier_paths()`, not
  spelled out as literal flags, so the workflow cannot drift from it.
- Add `tests._tiers.skip_idiom_modules()`/`skip_idiom_test_ids()`, which find
  those skip-idiom call sites via the AST machinery the guard already has,
  and resolve each to a precise pytest node ID so pulling in one skip-idiom
  test does not also pull in its module's unrelated ~100 other tests.
- Extend `WorkflowAgreementTests` (`tests/test_tier_guard.py`) so the
  guard also polices the `integration` job's positive selection, not just
  the `test` job's `--ignore`/`--ignore-glob` flags; add
  `EnclosingScopeTests` and `FixtureTierSelectionTests` (including a
  grep-based check textually independent of the AST machinery) to pin the
  new machinery, plus a canary for a capture name it cannot see statically.
- Leave the `gate` job's full-suite run untouched: it is a deliberate
  independent re-verification for the release path, not part of the
  per-PR-push duplication this change targets.

Measured: the skip-idiom set is exactly 3 methods across 2 files
(`test_dpkt_unit.py:444,812`, `test_pcapng_unit.py:3058`). Under the exact
`.[test,Scapy]` venv the `integration`/`gate` jobs install, the new selection
collects 173 tests and runs 142 passed/31 skipped, 0 failed -- 2 of the 3
recovered methods still skip there (pre-existing, unrelated: `dpkt` sits
behind its own extra that job never installed) but no test runs twice and
nothing regresses. `tests/test_tier_guard.py`: 37 passed, 57 subtests.
@JarryShaw

Copy link
Copy Markdown
Owner Author

✅ GOOD TO MERGE @ 3afdfcd7e — pending its own CI, which is still queued (0/15 required checks reported).

Cross-review ran on opus (no substitution needed) and returned GOOD TO GO. Posted here by the coordinator: that reviewer was a read-only helper with no GitHub write grant, so its verdict existed only in a hand-back report. Independently verified before posting:

Claim Result
Merges into current main git merge exit 0
tests/test_tier_guard.py on the merged tree 37 passed, 57 subtests
skip_idiom_test_ids() exactly 3 node IDs, as the PR body states
dpkt behind an extra CI never installs confirmed — DPKT extra at pyproject.toml:135; all three jobs install .[test] / .[test,Scapy]

Two corrections the reviewer asked for were applied after it reported, so its GOOD TO GO covers the pre-amend state; the delta is exactly those two fixes (corrected 161→142 passed / 31 skipped, plus the second textually-independent guard test at tests/test_tier_guard.py:290), both of which I checked directly in the diff.

Known conflict: #725 also edits this job's run step. Merge order and resolution are in the PR body.

✅ GOOD TO MERGE @ 3afdfcd7e — once CI reports.

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) perf Pull requests that improve performance (perf: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant