Skip to content

ci: make the Coverage check required once it is proven #133

Closed

Description

@newhoggy

Summary

#131 added an advisory Coverage job to ci.yml that runs action-works/patchcov-action@v1. It is deliberately not in .github/rulesets/main.json or the required-checks list in CONTRIBUTING.md. Once it has run reliably, decide whether to require it.

Before requiring it

  • Confirm the baseline publishes: after ci: dogfood patchcov-action on pull requests #131 merges, the push run on main should upload the coverage-baseline artifact, and the next PR's comment should show deltas against it without the "recomputed" note.
  • Confirm actions: read is needed. ci: dogfood patchcov-action on pull requests #131 grants it for the baseline download, but I could not check whether the download fails without it. If it is not needed, drop it from ci.yml and the README example.
  • Check the run history for flakes and timing (the first run took 1m39s against a 15-minute limit), and for red runs caused by a new patchcov release, since the job uses version: latest.
  • Check the merge queue: the job is skipped on merge_group, and a skipped job counts as passing.

To require it

Add Coverage to .github/rulesets/main.json, apply the ruleset (CONTRIBUTING.md, "Admin: configuring the ruleset"), and add it to the required-checks list and the smoke-test count ("all six required checks") in CONTRIBUTING.md.

Activity

  1. newhoggy commented on Oct 10, 2026

    @newhoggy
    ContributorAuthor

    Implementation plan

    Goal and acceptance criteria

    Make Coverage a required GitHub Actions check after verifying #131's prerequisites. Add it (integration ID 15368) to the checked-in and active main rulesets, update CONTRIBUTING's required checks and smoke-test count to seven, and remove obsolete advisory descriptions. Preserve the existing coverage thresholds, fork behavior, and merge queue skip.

    Evidence and approach

    • ci: dogfood patchcov-action on pull requests #131 merged as 9f564e3. The main push run succeeded and published the live coverage-baseline artifact 11669917167.
    • The next PR, chore: release v0.4.1 #132, downloaded that exact baseline; its log says a baseline was downloaded; skipping recompute. Its comment shows deltas without a recomputed note.
    • The three completed Coverage executions since introduction succeeded in 99 s (initial PR), 58 s (main push), and 37 s (chore: release v0.4.1 #132), comfortably below 15 minutes. No flakes or release-induced failures are observed, but this is a small sample, not proof of future stability. Keep version: latest; changing the binary under test is ci: also dogfood the pull request's own patchcov build #134's separate work.
    • Retain actions: read in CI and the README: the v1 action downloads via dawidd6/action-download-artifact@v27, and GitHub's Download an artifact API explicitly requires Actions read for installation tokens. No deliberately failing permission experiment is needed; a workflow token without this permission has not been tested here.
    • The merge_group Coverage job completed as skipped. Keep the job-level condition: skipped jobs satisfy required status checks, while PRs still enforce the gates. Running instrumented tests again in the queue would not run the action's PR-only patch gate.

    Changes, in order

    1. .github/rulesets/main.json: append Coverage to required status checks, pinned to GitHub Actions, without changing the other rules.
    2. CONTRIBUTING.md: describe Coverage as required, explicitly explain the queue skip, add it to the list, and update the smoke test to seven checks (Coverage skipped in queue).
    3. .github/workflows/ci.yml: update the Coverage comments to match required behavior; leave executable workflow configuration unchanged.
    4. After local checks, signed commit, direct review, any fixes, rebase, and PR creation, wait for this PR's CI. Once Coverage is green, update existing live ruleset 24650034 and read it back along with effective main rules. Build the API payload from the current live ruleset, adding only Coverage, so server-side settings absent from the historical checked-in payload are preserved. Do not create a duplicate ruleset.

    Tests and verification

    All commands run in /Users/jky/wrk/work-trees/patchcov/issue-133-require-coverage-check; Cargo commands explicitly pass that worktree's Cargo.toml.

    • actionlint .github/workflows/ci.yml (from the explicit worktree).
    • Parse ruleset JSON; verify seven unique check names, GitHub Actions integration IDs, names matching the workflow and CONTRIBUTING, and the retained merge_group trigger/skip.
    • cargo fmt --manifest-path "$WT/Cargo.toml" --check
    • cargo test --manifest-path "$WT/Cargo.toml" --locked --all-targets
    • cargo test --manifest-path "$WT/Cargo.toml" --locked --doc
    • cargo clippy --manifest-path "$WT/Cargo.toml" --locked --all-targets -- -D warnings
    • cargo run --manifest-path "$WT/Cargo.toml" --locked -- lint-markers
    • cargo +1.88.0 test --manifest-path "$WT/Cargo.toml" --locked --all-targets
    • cargo +1.88.0 test --manifest-path "$WT/Cargo.toml" --locked --doc
    • RUSTDOCFLAGS='-D warnings' cargo doc --manifest-path "$WT/Cargo.toml" --locked --no-deps --document-private-items
    • Repeat checks after rebase. Inspect this PR's GitHub CI, coverage comment, and live ruleset readback.

    Risks, assumptions, and out of scope

    • A future latest release or moving action tag could fail required Coverage; no such failure has occurred in the available sample. ci: also dogfood the pull request's own patchcov build #134 owns testing the PR-built binary.
    • Applying the active ruleset is part of this issue, authorized by its explicit request. Preserve every other live setting. If API permissions prevent application, document that limitation in the PR.
    • Do not merge this PR, touch other issues, or enqueue a smoke-test PR. A full post-change queue/release smoke test requires a merge and remains unverified; existing queue evidence and static checks do not prove that flow.
    • No Rust behavior changes, new permanent tests, thresholds, or README permission changes are needed.
  2. newhoggy commented on Oct 10, 2026

    @newhoggy
    ContributorAuthor

    Follow-up verification: required Coverage works in the queue

    The end-to-end queue/release smoke-test gap reported in #136 now has runtime evidence from the independently merged #135:

    • The active ruleset was updated to require Coverage at 2026-10-10 12:53:07 UTC (API updated_at: 2026-10-10T23:53:07.495+11:00).
    • The merge_group run for #135 started at 12:53:20 UTC, after that change. All six executing required jobs succeeded, and Coverage completed as skipped.
    • #135 merged at 12:55:04 UTC, producing main commit 38365d8d02a5e1063f26a009a4d7df90960d42b6. This demonstrates that the skipped Coverage job satisfies the live queue requirement.
    • The matching push-triggered release run succeeded: both Release and Release PR passed. Release PR's log explicitly records updated pr https://github.com/rust-works/patchcov/pull/132 at 12:55:58 UTC. Binary jobs were skipped; this is successful release-PR maintenance, not evidence of binary publication.

    No separate smoke-test issue is needed. The PR-build dogfooding follow-up #134 is already resolved by #135. The isolated local Clippy .rmeta failure did not recur on retry or either repeated full suite, so there is insufficient evidence for a defect issue. No new loose-end issues were filed in this pass.

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

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions