diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6c650f8..270b76a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -145,9 +145,8 @@ jobs: echo cat coverage-pr-build.md } > coverage-pr-comment.md - - name: Publish run summary - run: cat coverage-pr-comment.md >> "$GITHUB_STEP_SUMMARY" - name: Upload PR-build coverage + id: coverage-artifact uses: actions/upload-artifact@v7 with: name: coverage-pr-build @@ -155,6 +154,13 @@ jobs: path: | coverage-pr-head.lcov coverage-pr-comment.md + # The archived report has the run link; the upload URL exists only after upload. + - name: Link PR-build artifact and publish run summary + env: + ARTIFACT_URL: ${{ steps.coverage-artifact.outputs.artifact-url }} + run: | + printf '\n📦 [PR-build coverage report (LCOV and Markdown)](%s)\n' "$ARTIFACT_URL" >> coverage-pr-comment.md + cat coverage-pr-comment.md >> "$GITHUB_STEP_SUMMARY" - name: Comment PR-build coverage if: github.event.pull_request.head.repo.fork != true uses: marocchino/sticky-pull-request-comment@v3 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2e2f2da..0d9357f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -116,6 +116,9 @@ PR head, builds its debug `patchcov` binary with the lockfile, then measures tha checkout with cargo-llvm-cov. The local binary renders a separate sticky comment and enforces 90% patch coverage and 95% overall line coverage. Its report is also in the run summary and the `coverage-pr-build` artifact, including when a coverage gate fails. +The posted comment and run summary link directly to that artifact, labeled as LCOV and +Markdown. The archived comment includes the run link; the direct artifact link is added +after upload, when its URL is available. Fork PRs skip the comment and still get the summary, artifact and gates. This job is not in the ruleset or required-check list and is skipped on pushes and merge groups. diff --git a/README.md b/README.md index 5385440..10278bf 100644 --- a/README.md +++ b/README.md @@ -55,10 +55,9 @@ report so the per-file table appears: > > > -> Full per-file summary is attached as the **coverage-summary** build artifact. Without `--baseline-report` there is no delta on the `Total` line, no per-file table and no -indirect changes; a "No baseline available yet" notice stands in for them and the patch +indirect changes; a "No baseline report supplied" notice stands in for them and the patch section is unchanged. With `-o json` or `-o yaml` the same result is structured data, in the shape described in the [output schema](docs/reference.md#output-schema): diff --git a/docs/reference.md b/docs/reference.md index f0d3f78..a7f5f54 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -60,10 +60,16 @@ use the repository working directory. | `--ignore-filename-regex ` | Exclude matching files from both reports (repeatable or comma-separated); unioned with `diff.ignore-filename-regex` | | `--config-dir ` | Directory searched for `config.yaml` (default: the discovered `.patchcov/`, honouring `PATCHCOV_CONFIG_DIR`) | | `--artifact-url ` | Link to the full coverage-summary artifact, in the markdown footer | -| `--run-url ` | Link to the CI run, in the markdown footer | +| `--run-url ` | Link to the CI run, in the markdown footer, independently of `--artifact-url` | | `--commit-url ` | Commit-URL prefix for linking SHAs | | `--base-sha ` / `--head-sha ` | SHAs shown in the markdown `Comparing` line (both are needed for the line to appear) | +Markdown reports omit artifact guidance when no nonempty artifact URL is supplied. An explicit +`--artifact-url` (or `COVERAGE_ARTIFACT_URL`) is labeled as a full per-file coverage summary; +only use it for an artifact containing that summary. A run URL renders even without an +artifact URL. Without `--baseline-report`, per-file deltas and indirect coverage changes +are unavailable; the report makes no assumptions about CI baseline publication. + For both `--fail-under-*` flags, space-separated negative float values such as `-inf`, `-nan` and `-.5` receive the same explanatory usage error as the `--flag=value` spelling. A following option instead of a value reports `a value is required`. diff --git a/src/render.rs b/src/render.rs index 17e5330..b018a49 100644 --- a/src/render.rs +++ b/src/render.rs @@ -60,7 +60,7 @@ pub struct RenderOptions { pub no_explanation: bool, /// Link to the full coverage-summary artifact. pub artifact_url: Option, - /// Link to the CI run. + /// Link to the CI run, rendered independently of the artifact URL. pub run_url: Option, /// Base (merge-base) commit SHA. pub base_sha: Option, @@ -249,9 +249,8 @@ fn render_markdown(diff: &CoverageDiff, opts: &RenderOptions) -> String { render_notable_unchanged(diff, &mut out); } else { out.push_str( - "_No baseline available yet (first run, or the `main` baseline artifact was \ - missing). Per-file deltas will appear on PRs once a baseline has been published \ - from `main`._\n\n", + "_No baseline report supplied. Per-file deltas and indirect coverage changes \ + require a baseline report._\n\n", ); } // Also without a baseline: `ignore` still shapes the total and the patch. @@ -542,23 +541,22 @@ fn render_indirect_section(diff: &CoverageDiff, out: &mut String) { } fn render_footer(opts: &RenderOptions, out: &mut String) { - match opts.artifact_url.as_deref().filter(|u| !u.is_empty()) { - Some(artifact) => { - out.push_str(&format!( - "📦 [Full per-file coverage summary]({artifact})" - )); - if let Some(run) = opts.run_url.as_deref().filter(|u| !u.is_empty()) { - out.push_str(&format!(" · [run summary]({run})")); - } - out.push_str("\n"); - } - None => { - out.push_str( - "Full per-file summary is attached as the **coverage-summary** build \ - artifact.\n", - ); + let artifact = opts.artifact_url.as_deref().filter(|u| !u.is_empty()); + let run = opts.run_url.as_deref().filter(|u| !u.is_empty()); + if artifact.is_none() && run.is_none() { + return; + } + out.push_str(""); + if let Some(artifact) = artifact { + out.push_str(&format!("📦 [Full per-file coverage summary]({artifact})")); + } + if let Some(run) = run { + if artifact.is_some() { + out.push_str(" · "); } + out.push_str(&format!("[run summary]({run})")); } + out.push_str("\n"); } // --------------------------------------------------------------------------- @@ -937,7 +935,11 @@ mod tests { assert!(md.contains("### Patch coverage")); assert!(md.contains("Patch: **80%** (4/5 new lines covered)")); assert!(md.contains("`src/a.rs:9`")); - assert!(md.contains("No baseline available yet")); + assert!(md.contains("No baseline report supplied")); + assert!(md.contains("require a baseline report")); + assert!(!md.contains("will appear")); + assert!(!md.contains("baseline artifact")); + assert!(!md.contains("`main`")); } #[test] @@ -1390,6 +1392,32 @@ mod tests { assert!(md.contains("[run summary](https://run)")); } + #[test] + fn markdown_footer_links_are_independent_and_optional() { + for artifact in [None, Some(""), Some("https://artifact")] { + for run in [None, Some(""), Some("https://run")] { + let opts = RenderOptions { + artifact_url: artifact.map(str::to_string), + run_url: run.map(str::to_string), + ..Default::default() + }; + let md = render(&sample_diff(), &opts, OutputFormat::Markdown).unwrap(); + let has_artifact = artifact == Some("https://artifact"); + let has_run = run == Some("https://run"); + assert_eq!( + md.contains("[Full per-file coverage summary](https://artifact)"), + has_artifact, + "{md}" + ); + assert_eq!(md.contains("[run summary](https://run)"), has_run, "{md}"); + assert_eq!(md.contains(""), has_artifact || has_run, "{md}"); + assert_eq!(md.contains(" · "), has_artifact && has_run, "{md}"); + assert!(!md.contains("coverage-summary"), "{md}"); + assert!(!md.contains("attached"), "{md}"); + } + } + } + #[test] fn markdown_comparing_line_and_covered_indirect() { let mut diff = sample_diff();