Skip to content

ci: also dogfood the pull request's own patchcov build #134

Closed

Description

@newhoggy

Summary

patchcov's Coverage job (#131) runs the last published patchcov (version: latest), not the code in the pull request. A PR that changes diff, lcov or render logic is therefore commented on and gated by the previous release, so the dogfooding only covers the action and the release, not the change under review.

Idea

Build the PR's own binary and have the job use it, so a regression in a change shows up in that change's own coverage comment. The action installs a release binary or runs cargo install patchcov, so this probably needs either a thin-mode job (run-coverage: false, with the report made by cargo llvm-cov and the PR's target/debug/patchcov doing the diff) or an action input for a local binary. Weigh the extra build time against the value: the current job takes under two minutes.

Considerations

  • A thin-mode job loses the action's merge-base worktree recompute, so deltas depend on a published baseline.
  • The release-binary job is still the better test of what users run. Keep it, or make this a second job.
  • Decide whether it should be advisory, like Coverage.

Activity

  1. newhoggy commented on Oct 10, 2026

    @newhoggy
    ContributorAuthor

    Implementation plan

    Goal and acceptance criteria

    Keep testing the published action/release in Coverage, and additionally run the PR's own patchcov binary against coverage measured at that same PR head. Produce a distinct report/comment and enforce the existing 90% patch / 95% overall line thresholds in an advisory job. Fork PRs must work without comment permission; skip this extra job on pushes and merge groups.

    Approach

    Add an independent Coverage (PR build) job using the CLI directly. The current action's v1 implementation always installs a release binary, including in thin mode, and has no local-binary input. Changing that external repository is outside this issue. Keep the release job intact so users' shipped workflow remains tested. Check out the immutable PR head, build with cargo build --locked --bin patchcov, measure it with cargo-llvm-cov, and invoke ./target/debug/patchcov explicitly for rendering and gating. Compute the merge base against the event's immutable base SHA with full history. A second sticky-comment header prevents replacing the release report.

    No baseline is supplied in the new job: patch and overall coverage do not require one, while baseline retrieval/recompute would duplicate the action's orchestration and build cost. Consequently project deltas and indirect changes stay in the existing release comment. This is recorded in the new report and contributor docs. The extra job performs a debug build plus an instrumented test run; use a separate cache key and a 15-minute timeout, and observe actual GitHub timing after opening the PR.

    Changes, in order

    1. .github/workflows/ci.yml: add PR-only advisory job, head checkout/full history, stable toolchain with llvm-tools-preview, Rust cache, cargo-llvm-cov installation, locked debug build and instrumented tests, LCOV export, merge-base computation, direct CLI markdown rendering, run summary, separate sticky comment (same-repo only), distinct artifacts, and final CLI gates. Keep job permissions minimal and route event values through environment variables.
    2. CONTRIBUTING.md: describe which binary each job tests, thresholds/advisory status, where reports appear, lack of baseline deltas in the PR-build report, fork behavior, and extra build cost. Do not change the ruleset or required-check list.

    Tests and verification

    • actionlint .github/workflows/ci.yml to validate workflow expressions, actions and shell snippets.
    • Local equivalents, explicitly rooted at this worktree: cargo build --manifest-path "$WT/Cargo.toml" --locked --bin patchcov; cargo llvm-cov --manifest-path "$WT/Cargo.toml" --locked --all-features --workspace --no-report; cargo llvm-cov report --manifest-path "$WT/Cargo.toml" --lcov --output-path "$WT/coverage-pr-head.lcov"; run the built CLI on that LCOV against the merge base, render markdown and enforce both thresholds. Validate a deliberately impossible threshold fails.
    • Repository checks: 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, MSRV all-targets/doc tests, and docs with RUSTDOCFLAGS='-D warnings'.
    • Review the committed diff once, fix confirmed findings, fetch/rebase onto origin/main and rerun checks. Push and open a PR; inspect its new job if GitHub completes it in-session. Actual fork-token behavior and hosted execution/timing cannot be fully proven locally.

    Risks, assumptions, and out of scope

    The extra job is advisory, matching the issue's request and current Coverage configuration in this checkout. Concurrent #133 can change required status of the existing job independently; this issue does not alter that decision. Debug build plus instrumentation increases CI compute but does not slow other parallel jobs. The new comment uses a separate header and no baseline artifact name collision. No external action changes, release behavior changes, ruleset updates, or merge are included.

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