Skip to content

test: evaluate the release gates' if: expressions instead of matching their text #962

Description

@JarryShaw

tests/project/test_release_gates.py pins the release gates by asserting over the text of each job's if: expression. Four rounds of cross-review on #961 produced twenty mutations of one clause; the assertions were strengthened three times, and each strengthening was defeated by a mutation the previous one did not anticipate.

The current state after #961. test_version_check_must_strictly_succeed carries three assertions over a whitespace-stripped condition: the clause appears as a top-level && conjunct, the set of comparisons against version_check.result is exactly {('==', 'success')}, and .result is referenced exactly once. That catches all twenty mutations except one: prefixing the whole expression with true || short-circuits it from outside while leaving the inner conjunction intact. Closing that by anchoring the expression's opening was considered and rejected — it would fail on any legitimate reordering of conjuncts, and a test people route around is worse than a known residual.

The structural fix. Evaluate the expressions instead of matching them. For each of tag, pypi and conda, walk the four values needs.<job>.result can take — success, failure, cancelled, skipped — and assert the gate is false for every value but success. That is immune to spelling: reversed operands, contains(fromJSON(...)), a negation, a true || prefix and a &&→|| slip all change the truth table, which is what gets checked.

This module already does exactly that, for exactly this reason. TestReleaseStatusScriptExecutesCorrectly extracts the release_status shell script and runs it over a table of scenarios, and its own docstring says the earlier tests were "being a string assertion over the YAML, structurally unable to catch" the two #905 defects. The same argument now applies one layer up, to the if: expressions themselves.

Scope. A full GitHub Actions expression parser is not wanted. What the gates use is a small subset — !cancelled(), &&, ||, parentheses, ==/!= against single-quoted literals, startsWith(...), contains(fromJSON(...), ...), and needs.<job>.result / needs.<job>.outputs.<name> lookups. An evaluator over that subset, with the lookups supplied from a scenario table, is a bounded piece of work.

Two things to settle in doing it: whether the text assertions stay as a cheap first line of defence or are replaced, and what the scenario table covers beyond version_check — the github/tag equality pairs and the startsWith(github.ref_name, 'v') short-circuit have the same structural blind spot.

Found by the cross-review on #961, which built the twenty mutations and identified this as the principled endpoint. Not a gate on that pull request.

Activity

  1. added
    enhancementIssues requesting a new capability (set by the feature request template)
    testPull requests that add or correct tests (test: subject prefix)
    on Oct 1, 2026
  2. JarryShaw commented on Oct 1, 2026

    @JarryShaw
    OwnerAuthor

    Blocked on #961, and the blocker is checkable:

    gh pr view 961 -R JarryShaw/PyPCAPKit --json state -q .state
    

    While that returns OPEN, this stays blocked. #961 owns tests/project/test_release_gates.py and is in its fifth review round; the work here replaces or supplements assertions in that same file, so starting now would conflict and would also be reviewing a moving target.

    When it returns MERGED, this unblocks. Worth noting #961 leaves exactly one known hole for this issue to close — a top-level || outside the conjunction chain — and its docstring records that, so the first scenario in the evaluator's table is already written down.

  3. added
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    and removed
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Oct 1, 2026
  4. JarryShaw commented on Oct 1, 2026

    @JarryShaw
    OwnerAuthor

    Unblocked — #961 merged as 15ad99302 (13:01:58Z), and it is on main:

    gh pr view 961 -R JarryShaw/PyPCAPKit --json state,mergeCommit   # MERGED, 15ad993029a2d
    git merge-base --is-ancestor 15ad99302 origin/main               # exit 0
    

    tests/project/test_release_gates.py on main now carries the three text
    assertions this issue proposes to replace, so the work has its baseline.

    Starting it with one default on the first of the two open questions, to be
    overruled if you disagree: the text assertions stay alongside the
    evaluator rather than being replaced. They cost nothing to run, they fail
    with a readable message naming the clause, and an evaluator bug would
    otherwise leave the gates unpinned entirely. The second question — how far
    the scenario table reaches past version_check — is part of the work.

  5. added
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Oct 1, 2026
  6. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Oct 1, 2026
  7. added this to the 1.5 milestone on Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementIssues requesting a new capability (set by the feature request template)testPull requests that add or correct tests (test: subject prefix)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions