fix(ci): skip a test whose name is too large and say which one

crates/mergify-ci/src/junit_process/command.rs

@@ -171,12 +171,12 @@ async fn run_with_cap(
             .map(|c| c.name.clone())
             .collect(),
     };
-    let built = spans::build_traces(&parsed, &metadata);
+    let mut built = spans::build_traces(&parsed, &metadata);
 
     // Cap each gzipped upload at MAX_GZIPPED_UPLOAD_BYTES. A normal
     // report is one chunk (byte-identical to before); only an
     // oversized payload fans out into several uploads.
-    let (chunks, oversized_cases, mut upload_error) =
+    let (chunks, mut oversized_cases, mut upload_error) =
         match split::split_request(&built.request, upload_cap) {
             Ok(outcome) => (outcome.chunks, outcome.oversized_cases, None),
             // gzip is an in-memory write and effectively never fails,
@@ -192,6 +192,16 @@ async fn run_with_cap(
             ),
         };
 
+    // 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.
+    //
+    // Moved, not cloned. Every string in here is over MAX_TEST_NAME_BYTES --
+    // that is why it was refused -- so copying them would allocate another
+    // 65 kB apiece on the one path guaranteed to be holding the biggest names
+    // in the run. `built` is not read for this field again.
+    oversized_cases.append(&mut built.oversized_case_names);
+
     let client = upload::default_client();
     // Nothing reached the backend when the split produced no chunks
     // (every case individually oversized) or gzip failed. Captured
@@ -603,10 +613,16 @@ fn gha_oversized_annotation(names: &[String]) -> Option<String> {
         return None;
     }
     Some(format!(
-        "::warning title=Mergify Test Insights::{n} test result(s) exceeded the upload size \
-         limit and were skipped: {names}. The rest of the run was uploaded.",
+        "::warning title=Mergify Test Insights::{n} test result(s) were too large to upload \
+         and were skipped: {names}. The rest of the run was uploaded.",
         n = names.len(),
-        names = gha_escape_data(&names.join(", ")),
+        names = gha_escape_data(
+            &names
+                .iter()
+                .map(|n| display_name(n))
+                .collect::<Vec<_>>()
+                .join(", "),
+        ),
     ))
 }
 
@@ -640,14 +656,33 @@ fn write_upload_error_block(out: &mut String, error: &str, rejected: bool) {
 /// is no upload it can fit into. The CI verdict is unaffected (it's
 /// computed from the parsed cases, not the upload), so this is a
 /// best-effort data-loss notice, not a failure.
+/// How much of a skipped test's name the report prints. A name can be the
+/// reason it was skipped, so printing it whole would bury the report under
+/// the same 65 kB that caused the problem.
+const SKIPPED_NAME_DISPLAY_BYTES: usize = 120;
+
+/// `name` cut to [`SKIPPED_NAME_DISPLAY_BYTES`] on a character boundary,
+/// marked when it was cut so nobody copies the prefix as the real name.
+fn display_name(name: &str) -> String {
+    if name.len() <= SKIPPED_NAME_DISPLAY_BYTES {
+        return name.to_string();
+    }
+    let mut end = SKIPPED_NAME_DISPLAY_BYTES;
+    while end > 0 && !name.is_char_boundary(end) {
+        end -= 1;
+    }
+    format!("{}… ({} bytes total)", &name[..end], name.len())
+}
+
 fn write_oversized_cases(out: &mut String, names: &[String]) {
-    out.push_str("\n  ⚠️ Some test results were too large to upload\n");
-    out.push_str("    A single test's output exceeded the upload size limit and was skipped.\n");
+    out.push_str("\n  ⚠️ Some test results were skipped\n");
+    out.push_str("    A test whose name or output is too large to upload is skipped; the rest\n");
+    out.push_str("    of the run was uploaded. Rename the test to get its history back.\n");
     out.push_str("    Quarantine status and CI outcome are unaffected.\n");
     out.push('\n');
     out.push_str("      ┌ Skipped\n");
     for name in names {
-        out.push_str(&format!("      │  {name}\n"));
+        out.push_str(&format!("      │  {}\n", display_name(name)));
     }
     out.push_str("      └─\n");
 }
@@ -1025,7 +1060,44 @@ mod tests {
         .unwrap();
         assert!(ann.starts_with("::warning"), "{ann}");
         assert!(ann.contains("a.big, b.huge"), "{ann}");
-        assert!(ann.contains("exceeded the upload size limit"), "{ann}");
+        assert!(ann.contains("too large to upload"), "{ann}");
+    }
+
+    /// A name can itself be the reason a case was skipped, so neither the
+    /// report nor the annotation may print it whole -- 65 kB of generated
+    /// title would bury the very message telling you which test to rename.
+    #[test]
+    fn a_skipped_name_is_truncated_for_display() {
+        let long = "z".repeat(super::SKIPPED_NAME_DISPLAY_BYTES + 500);
+
+        let shown = display_name(&long);
+        assert!(shown.len() < long.len(), "{shown}");
+        // Marked as cut, and carrying the real size, so nobody copies the
+        // prefix believing it is the test's name.
+        assert!(shown.contains('…'), "{shown}");
+        assert!(
+            shown.contains(&format!("{} bytes total", long.len())),
+            "{shown}"
+        );
+
+        let mut report = String::new();
+        write_oversized_cases(&mut report, std::slice::from_ref(&long));
+        assert!(!report.contains(&long), "the report printed the whole name");
+
+        let ann = temp_env::with_var("GITHUB_ACTIONS", Some("true"), || {
+            gha_oversized_annotation(std::slice::from_ref(&long))
+        })
+        .unwrap();
+        assert!(
+            !ann.contains(&long),
+            "the annotation printed the whole name"
+        );
+    }
+
+    /// A short name is printed exactly, with no truncation marker.
+    #[test]
+    fn a_short_name_is_printed_verbatim() {
+        assert_eq!(display_name("tests.test_a"), "tests.test_a");
     }
 
     // ── End-to-end orchestrator tests. Drive the full `run()`

crates/mergify-ci/src/junit_process/spans.rs

@@ -47,6 +47,22 @@ pub struct UploadMetadata {
     pub quarantined: BTreeSet<String>,
 }
 
+/// The widest test name this client will upload, in bytes.
+///
+/// Not a storage limit. The engine keys every table an oversized name can
+/// reach on `uuid_generate_v5` of the name rather than the name itself, so
+/// the name is stored whole and no btree tuple limit is tripped (MRGFY-8902).
+/// What this bounds is what a wire payload and a UI can usefully carry: a
+/// name at this size is 65 kB of generated text — INC-2436 was a Vitest title
+/// interpolating stringified React source — that no one reads and that costs
+/// every downstream reader who has to render it.
+///
+/// Skipped rather than truncated, and reported rather than dropped quietly.
+/// 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(crate) const MAX_TEST_NAME_BYTES: usize = 65_536;
+
 /// Result of converting a [`ParseResult`] (one or more `JUnit`
 /// files) into a wire-ready OTLP request.
 #[derive(Debug, Clone)]
@@ -55,6 +71,10 @@ pub struct BuiltTraces {
     /// Same value populates the `test.run.id` resource attribute.
     pub run_id: String,
     pub request: ExportTraceServiceRequest,
+    /// Names of cases left out because they exceed
+    /// [`MAX_TEST_NAME_BYTES`]. Reported to the user by the caller;
+    /// empty for every ordinary report.
+    pub oversized_case_names: Vec<String>,
 }
 
 /// Convert a [`ParseResult`] (the union of every parsed `JUnit`
@@ -87,6 +107,7 @@ fn build_traces_with(
     let common_attrs = common_attributes(metadata);
 
     let mut spans: Vec<Span> = Vec::new();
+    let mut oversized_case_names: Vec<String> = Vec::new();
 
     // Suite spans are appended after we know each suite's earliest
     // case start (so the suite's start_time covers all its cases).
@@ -101,7 +122,17 @@ fn build_traces_with(
         let suite_span_id = rng.bytes8();
         let mut suite_start_time_unix_nanos = now_unix_nanos;
 
+        let mut suite_case_count = 0_usize;
+
         for case in suite_cases {
+            // `len()` on a Rust `String` is already bytes, which is the unit
+            // the limit is stated in.
+            if case.name.len() > MAX_TEST_NAME_BYTES {
+                oversized_case_names.push(case.name.clone());
+                continue;
+            }
+            suite_case_count += 1;
+
             let case_span_id = rng.bytes8();
             let start_time_unix_nanos = case_start_time(now_unix_nanos, case.duration);
             suite_start_time_unix_nanos = suite_start_time_unix_nanos.min(start_time_unix_nanos);
@@ -164,6 +195,13 @@ fn build_traces_with(
             });
         }
 
+        // A suite that kept nothing emits nothing: a suite span with no
+        // children is a test suite the backend would show as having run zero
+        // cases, which is a different lie from the one being fixed.
+        if suite_case_count == 0 {
+            continue;
+        }
+
         session_start_time_unix_nanos =
             session_start_time_unix_nanos.min(suite_start_time_unix_nanos);
 
@@ -234,6 +272,7 @@ fn build_traces_with(
         request: ExportTraceServiceRequest {
             resource_spans: vec![resource_spans],
         },
+        oversized_case_names,
     }
 }
 
@@ -470,6 +509,98 @@ mod tests {
         }
     }
 
+    /// A name past the cap costs its own result and nothing else.
+    #[test]
+    fn an_oversized_name_is_skipped_and_its_suite_mates_still_upload() {
+        let mut parsed = sample_parsed();
+        // One byte over, so the assertion is about the boundary and not about
+        // some comfortably-huge number that a wrong `>=` would also pass.
+        let oversized = "x".repeat(MAX_TEST_NAME_BYTES + 1);
+        parsed.cases.push(TestCase {
+            name: oversized.clone(),
+            suite_name: "pytest".to_string(),
+            duration: Some(Duration::from_secs_f64(0.003)),
+            file: None,
+            line: None,
+            status: TestStatus::Passed,
+            failure: Failure::default(),
+        });
+
+        let mut bytes: Vec<u8> = Vec::with_capacity(16 + 4 * 8);
+        bytes.extend(std::iter::repeat_n(0xAA, 16));
+        bytes.extend(std::iter::repeat_n(0x11, 8));
+        bytes.extend(std::iter::repeat_n(0x22, 8));
+        bytes.extend(std::iter::repeat_n(0x33, 8));
+        bytes.extend(std::iter::repeat_n(0x44, 8));
+        let mut rng = FixedRng::new(bytes);
+
+        let now: u64 = 1_700_000_000_000_000_000;
+        let metadata = UploadMetadata::default();
+        let built = with_ci_env(&[], || build_traces_with(&parsed, &metadata, now, &mut rng));
+
+        assert_eq!(built.oversized_case_names, vec![oversized.clone()]);
+
+        let spans = &built.request.resource_spans[0].scope_spans[0].spans;
+        // Reported, not uploaded -- in no span name and no attribute value.
+        assert!(!spans.iter().any(|s| s.name == oversized));
+        assert!(!spans.iter().any(|s| s.attributes.iter().any(|a| {
+            matches!(
+                &a.value.as_ref().and_then(|v| v.value.as_ref()),
+                Some(AnyValueOneof::StringValue(v)) if v == &oversized
+            )
+        })));
+        // The two well-named cases in the same suite are untouched.
+        assert!(
+            spans
+                .iter()
+                .any(|s| s.name == "tests.test_func.test_success")
+        );
+        assert!(
+            spans
+                .iter()
+                .any(|s| s.name == "tests.test_func.test_failed")
+        );
+    }
+
+    /// A name exactly at the cap is uploaded: the limit is inclusive, and a
+    /// test sitting on it must not silently lose its history.
+    #[test]
+    fn a_name_exactly_at_the_cap_is_still_uploaded() {
+        let mut parsed = sample_parsed();
+        let at_cap = "y".repeat(MAX_TEST_NAME_BYTES);
+        parsed.cases.push(TestCase {
+            name: at_cap.clone(),
+            suite_name: "pytest".to_string(),
+            duration: Some(Duration::from_secs_f64(0.003)),
+            file: None,
+            line: None,
+            status: TestStatus::Passed,
+            failure: Failure::default(),
+        });
+
+        let mut bytes: Vec<u8> = Vec::with_capacity(16 + 5 * 8);
+        bytes.extend(std::iter::repeat_n(0xAA, 16));
+        bytes.extend(std::iter::repeat_n(0x11, 8));
+        bytes.extend(std::iter::repeat_n(0x22, 8));
+        bytes.extend(std::iter::repeat_n(0x33, 8));
+        bytes.extend(std::iter::repeat_n(0x44, 8));
+        bytes.extend(std::iter::repeat_n(0x55, 8));
+        let mut rng = FixedRng::new(bytes);
+
+        let built = with_ci_env(&[], || {
+            build_traces_with(
+                &parsed,
+                &UploadMetadata::default(),
+                1_700_000_000_000_000_000,
+                &mut rng,
+            )
+        });
+
+        assert!(built.oversized_case_names.is_empty());
+        let spans = &built.request.resource_spans[0].scope_spans[0].spans;
+        assert!(spans.iter().any(|s| s.name == at_cap));
+    }
+
     #[test]
     fn builds_session_suite_and_case_spans_with_consistent_parent_chain() {
         // 16 bytes for trace_id; 4×8 bytes for session, suite,