fix(ci-insights): name the job-name variable the uploader actually reads - #12415
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. |
|
Context for the reviewer, since this commit answers a question rather than just fixing prose. The docs and the dashboard's flaky-detection setup prompt named two different environment variables for the same thing, so I settled it from the source rather than picking one.
Two consequences outside this repo, which I have not fixed here and which need their own change:
Both are recorded for follow-up work rather than left in a comment thread. |
There was a problem hiding this comment.
Pull request overview
Aligns CI Insights setup documentation and matrix helper components to the single job-name override variable actually consumed by Mergify uploaders (MERGIFY_TEST_JOB_NAME), and clarifies provider-specific defaults and matrix behavior so reports are filed under the intended job name.
Changes:
- Updates CI Insights setup pages (Jenkins, GitHub Actions, Buildkite) to document
MERGIFY_TEST_JOB_NAMEas the override and explain default job-name sources. - Reworks the GitHub Actions and Buildkite matrix helper components to remove guidance around unused job-name inputs/properties and show where to set the env var.
- Clarifies why matrix runs need overrides on GitHub Actions (shared
GITHUB_JOB) and when overrides are needed on Buildkite.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/content/docs/ci-insights/setup/jenkins.mdx | Expands the “Job name” guidance and clarifies how to override via MERGIFY_TEST_JOB_NAME. |
| src/content/docs/ci-insights/setup/github-actions.mdx | Explains default job name (GITHUB_JOB), matrix collision, and documents MERGIFY_TEST_JOB_NAME usage. |
| src/content/docs/ci-insights/setup/buildkite.mdx | Updates “Job name” guidance to use MERGIFY_TEST_JOB_NAME in step env. |
| src/components/MergifyCIUploadStepMatrix.astro | Updates matrix helper to set MERGIFY_TEST_JOB_NAME at the job level and removes job_name input guidance. |
| src/components/BuildkiteCIUploadStepMatrix.astro | Updates matrix helper to use MERGIFY_TEST_JOB_NAME in step env and removes job_name plugin property guidance. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
da5211b to
c6280ed
Compare
Revision history
|
|
Re-pushed to address both review comments: Both flagged the same sentence, duplicated in The caution now says what holds either way: a step's Nothing else in the commit changed. #12416 and #12417 were restacked on top and carry no content change. |
mergify
Bot
added
review threads unresolved
and removed
review threads unresolved
labels
The CI Insights setup pages and the two matrix helpers disagreed with each other about how to override the job name a test report is filed under, and none of them said where the default comes from. The three setup pages now name `MERGIFY_TEST_JOB_NAME` — the variable the CLI reads, and the one every framework plugin (pytest, rspec, vitest) already documents — say where the default comes from on each provider (`GITHUB_JOB` on GitHub Actions, Jenkins' own `JOB_NAME`, the step label on Buildkite), and show where to set it. They also state why it matters: the job name is part of a test's identity, so a scheduled run that reports a different name splits its results off from the pull request ones. The GitHub Actions matrix case is stated the way it actually breaks: every leg of a matrix reports the same job name because they share one `GITHUB_JOB`. The old text left the reader to infer why an override was needed at all. The two matrix helpers keep recommending the native override — the action's `job_name` input and the Buildkite plugin's `job_name` property. Both currently export `MERGIFY_JOB_NAME`, which no uploader reads, so neither takes effect today; that is a bug in the three producers of that variable, not something the docs should route readers around. Fixing them is tracked separately, and the snippets pin moving tags (the action's `v25`, the plugin's `#v1`), so readers pick the fix up when those tags move. The pytest, rspec and vitest pages already agreed with this and are unchanged. Settles Mergifyio/ci-bot#369. Part of MRGFY-8720 Change-Id: I96e06bb4500bf9e7664d36d941ef0d962004e9a9
c6280ed to
42d7aae
Compare
|
Re-pushed: The two matrix helpers go back to recommending the native override — the action's I had it backwards. Both properties are declared and documented by their producers, and both are dead only because those producers export That leaves the docs briefly ahead of the code: Everything else in the PR is unchanged: job-name-as-test-identity, the per-provider defaults ( |
|
One thing worth knowing while reviewing this commit: the Buildkite snippet it restores Fixed at the top of the stack in #12426, which points both Buildkite components at |
jd
left a comment
jd
left a comment
There was a problem hiding this comment.
we're fixing the code here rather than this
Merge Queue Status
This pull request spent 3 minutes 26 seconds in the queue, including 2 minutes 21 seconds running CI. Required conditions to merge
|
The JUnit and TestNG recipes told Gradle users to configure their report output
with a `reports { junitXml.enabled = true; junitXml.destination = ... }` block.
Gradle 8.0 removed both of those properties ("The deprecated `destination`, and
`enabled` properties of the `Report` type have been removed", per Gradle's own
upgrade notes), so on any current Gradle that block fails the build. A reader
following either page was stopped at the very first step, before ever reaching
the upload step.
Nothing needs to replace it. Gradle's `test` task already writes JUnit XML to
`build/test-results/test/`, which is the path both pages then tell the reader to
upload, so the block was configuring the default. It goes, the default location
is stated instead, and only the line that does have to be there stays:
`useJUnitPlatform()` for JUnit 5, `useTestNG()` for TestNG.
That also settles the version question the recipe would otherwise have to
answer, since `useJUnitPlatform()` and `useTestNG()` are accepted by every
Gradle version anyone is running.
Settles Mergifyio/ci-bot#370. The same broken block was in the TestNG recipe,
which that issue did not mention.
Part of MRGFY-8720
Depends-On: #12415
The CI Insights setup pages and the two matrix helpers disagreed with each other
about how to override the job name a test report is filed under, and none of
them said where the default comes from.
The three setup pages now name
MERGIFY_TEST_JOB_NAME— the variable the CLIreads, and the one every framework plugin (pytest, rspec, vitest) already
documents — say where the default comes from on each provider (
GITHUB_JOBonGitHub Actions, Jenkins' own
JOB_NAME, the step label on Buildkite), and showwhere to set it. They also state why it matters: the job name is part of a
test's identity, so a scheduled run that reports a different name splits its
results off from the pull request ones.
The GitHub Actions matrix case is stated the way it actually breaks: every leg
of a matrix reports the same job name because they share one
GITHUB_JOB. Theold text left the reader to infer why an override was needed at all.
The two matrix helpers keep recommending the native override — the action's
job_nameinput and the Buildkite plugin'sjob_nameproperty. Both currentlyexport
MERGIFY_JOB_NAME, which no uploader reads, so neither takes effecttoday; that is a bug in the three producers of that variable, not something the
docs should route readers around. Fixing them is tracked separately, and the
snippets pin moving tags (the action's
v25, the plugin's#v1), so readerspick the fix up when those tags move.
The pytest, rspec and vitest pages already agreed with this and are unchanged.
Settles Mergifyio/ci-bot#369.
Part of MRGFY-8720