fix(test-insights): make the closing CTA one partial, not eleven copies - #12417
Conversation
|
This pull request is part of a Mergify stack:
|
Merge Protections🟢 All 6 merge protections satisfied — ready to merge. Show 6 satisfied protections🟢 🤖 Continuous Integration
🟢 👀 Review Requirements
🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 🔎 Reviews
🟢 📕 PR description
🟢 🚦 Auto-queueWhen all merge protections are satisfied, this pull request will be queued automatically. |
|
This stack answers three of the four docs asks filed on 2026-08-17 (Mergifyio/ci-bot#369, #370, #372). The fourth, Mergifyio/ci-bot#366, turned out not to be a docs change and is Mergifyio/monorepo#38775 instead. #366 asked whether this repo should grow a section on how a ruleset bypass mode interacts with condition injection, taking the I read the flag that decides it. One thing that fix deliberately does not settle, noted on that PR: the |
c604f0e to
10ece46
Compare
Revision history
|
There was a problem hiding this comment.
Pull request overview
This PR fixes broken/absent “review results in Test Insights” links across the Test Framework recipes by centralizing the CTA into a shared MDX partial that points to the existing Test Insights → Detection dashboard page. It also updates the GitHub Actions recipe snippets to reliably propagate the test step outcome into the quarantine/upload step by adding id: tests (and ensuring continue-on-error: true where required).
Changes:
- Replace per-recipe “review in Test Insights” copy/links with a shared
<ReviewInTestInsights />partial targeting/test-insights/detection. - Add
id: teststo the test-runner step in GitHub Actions examples so${{ steps.tests.outcome }}resolves correctly. - Add missing
continue-on-error: truein the affected workflow examples to match the quarantine setup requirements.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/content/docs/test-insights/test-frameworks/testng.mdx | Adds id: tests to test steps and replaces the broken dashboard link with the shared review partial. |
| src/content/docs/test-insights/test-frameworks/rust.mdx | Adds id: tests to both alternative test steps and replaces the broken dashboard link with the shared review partial. |
| src/content/docs/test-insights/test-frameworks/phpunit.mdx | Adds id: tests to the test step and replaces the broken dashboard link with the shared review partial. |
| src/content/docs/test-insights/test-frameworks/pest.mdx | Adds id: tests to the test step and replaces the broken dashboard link with the shared review partial. |
| src/content/docs/test-insights/test-frameworks/nunit.mdx | Adds id: tests to the test step and replaces the broken dashboard link with the shared review partial. |
| src/content/docs/test-insights/test-frameworks/mstest.mdx | Adds id: tests to the test step and replaces the broken dashboard link with the shared review partial. |
| src/content/docs/test-insights/test-frameworks/minitest.mdx | Adds id: tests (and missing continue-on-error: true) to the test steps and replaces the broken dashboard link with the shared review partial. |
| src/content/docs/test-insights/test-frameworks/junit.mdx | Adds id: tests (and missing continue-on-error: true for Gradle) and replaces the broken dashboard link with the shared review partial. |
| src/content/docs/test-insights/test-frameworks/jest.mdx | Adds id: tests to the test step and replaces the broken dashboard link with the shared review partial. |
| src/content/docs/test-insights/test-frameworks/golang.mdx | Adds id: tests to the test step and replaces the broken dashboard link with the shared review partial. |
| src/content/docs/test-insights/test-frameworks/cypress.mdx | Adds the shared review partial where the dashboard was previously mentioned without a link. |
| src/content/docs/test-insights/test-frameworks/_review-in-test-insights.mdx | Introduces a reusable “review results” partial linking to the Test Insights Detection page. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
10ece46 to
0ec2b78
Compare
|
@jd this pull request is now in conflict 😩 |
0ec2b78 to
3d4d468
Compare
|
Rebased onto main and resolved the conflict #12439 landed the What is left in this PR is therefore the dashboard link fix (all eleven recipes, via the new |
Every test framework recipe ends by sending the reader to the Test Insights dashboard, and each one carried its own copy of that sentence. Ten of the copies had drifted onto `https://dashboard.mergify.com/test-insights/jobs`, which is not a page — Test Insights has Prevention, Detection and Mitigation — and an eleventh named the dashboard with no link at all. #12490 has since corrected all eleven in place, so the links work today; what it could not fix is why one stale URL became ten broken links, which is that the sentence exists eleven times. So the paragraph becomes `_review-in-test-insights.mdx`, included by the recipes. The rendered text is byte-identical to what #12490 shipped; the next time that link moves it is one edit rather than eleven, and a recipe cannot quietly drift out of step with its siblings again. Shared partials are already how these files handle the Buildkite quarantine setup and the upload steps. Separately, two inline GitHub Actions examples were missing the `continue-on-error: true` that the quarantine partial they include requires: the Gradle example in the JUnit recipe and the `rake test` example in the Minitest one. Without it a failing test step ends the job before the upload step runs, so the run that most needs a report produces none — and in both files the sibling example directly above already had the line, so this was drift rather than a deliberate difference. Part of MRGFY-8720 Change-Id: I482ec81d7710f37d83dcffdf65abc029b73437fc
3d4d468 to
a2f4cc7
Compare
|
Rebased onto main and rewrote this one, because #12490 landed the same link fix while this sat here — #12490 corrected all eleven closing links in place, to the same page this PR was pointing them at ( What is left here is the part #12490 could not do, and I think it is still worth landing:
If you would rather not carry the partial, the two |
Merge Queue Status
This pull request spent 3 minutes 24 seconds in the queue, including 2 minutes 51 seconds running CI. Required conditions to merge
|
…lease (#12426) The two Buildkite upload snippets hardcode `mergifyio/mergify-ci#v1` in their `.astro` source. `v1` is the oldest of six releases — `v1` through `v6` are distinct tags, not a floating major — so every reader who copies one of these snippets pins the first version ever cut and never receives anything shipped since. Nothing else in the docs does this. Every Buildkite page writes `@@BUILDKITE_PLUGIN_VERSION@@`, which `plugins/remark-buildkite-version.ts` substitutes from `src/data/buildkite-plugin-version.json` at build time. That plugin only visits markdown nodes, so the sentinel does nothing inside an `.astro` component and these two were left pinning by hand. The fix is the one the GitHub Actions counterpart already uses: import the data file and interpolate it, exactly as `MergifyCIUploadStepMatrix.astro` does with `gha-mergify-ci-version.json`. Bumping `v1` to `v6` would have recreated the same problem one release later. This matters now because the commit at the bottom of this stack recommends the plugin's `job_name` property again. A reader who copies the matrix snippet and pins `v1` gets a plugin where that property predates the code reading it — the docs would recommend a knob that is dead in the version the snippet installs. The non-matrix component has the same pin and is the more widely used of the two: eleven test-framework recipes render it. Both are fixed here, since it is one bug with one cause. Verified on the built output: all 78 rendered occurrences of the plugin reference now read `v6`, none read `v1`, and no template literal leaked into the HTML. `pnpm check` 0 errors, `pnpm build` 384 pages, `pnpm test` 144 passed, `pnpm check:internal-leaks` clean. Refs MRGFY-8720 Depends-On: #12417
Every test framework recipe ends by sending the reader to the Test Insights
dashboard, and each one carried its own copy of that sentence. Ten of the copies
had drifted onto
https://dashboard.mergify.com/test-insights/jobs, which is nota page — Test Insights has Prevention, Detection and Mitigation — and an
eleventh named the dashboard with no link at all. #12490 has since corrected all
eleven in place, so the links work today; what it could not fix is why one stale
URL became ten broken links, which is that the sentence exists eleven times.
So the paragraph becomes
_review-in-test-insights.mdx, included by therecipes. The rendered text is byte-identical to what #12490 shipped; the next
time that link moves it is one edit rather than eleven, and a recipe cannot
quietly drift out of step with its siblings again. Shared partials are already
how these files handle the Buildkite quarantine setup and the upload steps.
Separately, two inline GitHub Actions examples were missing the
continue-on-error: truethat the quarantine partial they include requires:the Gradle example in the JUnit recipe and the
rake testexample in theMinitest one. Without it a failing test step ends the job before the upload step
runs, so the run that most needs a report produces none — and in both files the
sibling example directly above already had the line, so this was drift rather
than a deliberate difference.
Part of MRGFY-8720