fix(ci): skip a test whose name is too large and say which one - #1793
Open
sileht wants to merge 1 commit into
Conversation
Contributor
Merge Protections🔴 2 of 6 protections blocking · waiting on 👀 reviews
🔴 👀 Review RequirementsWaiting for
This rule is failing.
🔴 🔎 ReviewsWaiting for
This rule is failing.
Show 4 satisfied protections🟢 🤖 Continuous Integration
🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 📕 PR description
🟢 🚦 Auto-queueWhen all merge protections are satisfied, this pull request will be queued automatically. |
There was a problem hiding this comment.
🟡 Changes recommended
The current merge of oversized case names clones potentially very large strings unnecessarily, which undermines the goal of safely handling oversized names and can cause avoidable memory/CPU overhead.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens the mergify ci junit-process OTLP span generation and reporting against unbounded JUnit test names by skipping spans for cases with excessively long names and surfacing the skipped cases to users (including GitHub Actions annotations).
Changes:
- Add a byte cap for test case names during span construction; skip oversized cases and omit suite spans that would otherwise contain zero cases.
- Plumb skipped oversized case names through to the command orchestrator and unify them with “too large to upload” reporting.
- Truncate skipped-case names for display in both the human report and GitHub Actions
::warning::annotations, with regression tests.
File summaries
| File | Description |
|---|---|
| crates/mergify-ci/src/junit_process/spans.rs | Enforces a max test-name byte limit during span building, tracks skipped case names, and avoids emitting empty suite spans. |
| crates/mergify-ci/src/junit_process/command.rs | Merges skipped-by-name cases into the existing “skipped upload” reporting path and truncates displayed names in reports/annotations, with tests. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /// Truncating would mint a second identity for the same test, which is the | ||
| /// one outcome worse than not having the result: the test would split into | ||
| /// two histories and its flakiness and quarantine state with it. | ||
| pub const MAX_TEST_NAME_BYTES: usize = 65_536; |
Comment on lines
+195
to
+198
| // Cases the span builder refused on name length join the ones the split | ||
| // refused on payload size: from the user's side both are "this result was | ||
| // not uploaded", and one list is what they need to act on. | ||
| oversized_cases.extend(built.oversized_case_names.iter().cloned()); |
sileht
marked this pull request as ready for review
August 31, 2026 11:47
A test name arrives from JUnit unbounded and goes straight onto the wire: `build_traces` copies it into the span name and two attributes. INC-2436 was a Vitest title interpolating stringified React source at 31,207 characters, and nothing between the runner and the backend refused it. Skip a case whose name exceeds 65,536 bytes and report it through the path already built for cases too large to upload: the human report and, on GitHub Actions, a `::warning::` annotation. The CI outcome is untouched, so a skipped result never breaks a customer's build. A suite left with no cases emits no span rather than an empty suite. Skipped rather than truncated on purpose. The backend identifies a test by `uuid_generate_v5` of its name, so a truncated name is a different test: the result would split its history, flakiness and quarantine state in two. Not uploading it says so plainly, and names it so the owner can rename the test. Names are cut to 120 bytes for display in both surfaces, since the name is itself what made the case too large. Fixes MRGFY-8951 Related to MRGFY-8902 Change-Id: I224cd6b770d1228bc0ae884ec546b05a94102e3d
sileht
force-pushed
the
devs/sileht/mrgfy-8951-cli-skip-oversized-test-names/skip-test-whose-name-too-large-say-which-one--224cd6b7
branch
from
August 31, 2026 12:44
42bb940 to
5062e98
Compare
Member
Author
Revision history
|
jd
approved these changes
Aug 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A test name arrives from JUnit unbounded and goes straight onto the wire:
build_tracescopies it into the span name and two attributes. INC-2436was a Vitest title interpolating stringified React source at 31,207
characters, and nothing between the runner and the backend refused it.
Skip a case whose name exceeds 65,536 bytes and report it through the
path already built for cases too large to upload: the human report and,
on GitHub Actions, a
::warning::annotation. The CI outcome isuntouched, so a skipped result never breaks a customer's build. A suite
left with no cases emits no span rather than an empty suite.
Skipped rather than truncated on purpose. The backend identifies a test
by
uuid_generate_v5of its name, so a truncated name is a differenttest: the result would split its history, flakiness and quarantine state
in two. Not uploading it says so plainly, and names it so the owner can
rename the test.
Names are cut to 120 bytes for display in both surfaces, since the name
is itself what made the case too large.
Fixes MRGFY-8951
Related to MRGFY-8902