bun test --reporter=junit: fix broken XML, double-escaped classname, and empty <failure>

src/jsc/VirtualMachine.rs

@@ -300,6 +300,13 @@ pub struct VirtualMachine {
     pub on_unhandled_rejection_ctx: Option<*mut c_void>,
     pub on_unhandled_rejection_exception_list: Option<NonNull<ExceptionList>>,
     pub unhandled_error_counter: usize,
+    /// When set, `print_error_instance_body` calls this with the remapped
+    /// `ZigException` (same lifecycle as the GitHub Actions annotation hook),
+    /// so observers can read name/message/stack without re-running the
+    /// formatter. Installed by `bun test --reporter=junit` around
+    /// `run_error_handler`.
+    pub on_print_error_zig_exception: Option<fn(*mut c_void, &ZigException)>,
+    pub on_print_error_zig_exception_ctx: *mut c_void,
     pub is_handling_uncaught_exception: bool,
     pub exit_on_uncaught_exception: bool,
 
@@ -5711,6 +5718,9 @@ impl VirtualMachine {
             if let Some(debugger) = self.debugger.as_deref_mut() {
                 debugger.lifecycle_reporter_agent.report_error(exception);
             }
+            if let Some(cb) = self.on_print_error_zig_exception {
+                cb(self.on_print_error_zig_exception_ctx, exception);
+            }
         }
 
         // Defer the GitHub-annotation print to scope exit.

src/runtime/cli/test_command.rs

@@ -157,9 +157,22 @@ pub(crate) fn escape_xml(str_: &[u8], writer: &mut impl bun_io::Write) -> crate:
                 writer.write_all(bun_core::strings::xml_escape_entity(c).unwrap())?;
                 last = i + 1;
             }
-            0..=0x1f => {
-                // Escape all control characters
+            b'\t' | b'\n' | b'\r' => {
+                // Valid XML 1.0 Char. Emit as a numeric reference so the literal
+                // byte survives attribute-value normalisation (XML 1.0 §3.3.3).
+                if i > last {
+                    writer.write_all(&str_[last..i])?;
+                }
                 write!(writer, "&#{};", c)?;
+                last = i + 1;
+            }
+            0..=0x1f => {
+                // Any other C0 control character is not a valid XML 1.0 Char and
+                // cannot be represented even as a numeric reference, so drop it.
+                if i > last {
+                    writer.write_all(&str_[last..i])?;
+                }
+                last = i + 1;
             }
             _ => {}
         }
@@ -211,6 +224,33 @@ pub fn write_test_status_line(
 // `&mut io::Writer`; the previous local `err_w`/`out_w` wrappers were no-op
 // reborrows. Call sites use the `Output` accessors directly.
 
+#[derive(Default)]
+pub struct JunitFailure {
+    pub name: Vec<u8>,
+    pub message: Vec<u8>,
+    pub body: Vec<u8>,
+}
+
+/// Append `input` to `out`, dropping CSI sequences (`ESC '[' ... final`), so a
+/// matcher message built with colour does not reach the report as SGR residue.
+fn push_stripping_ansi(out: &mut Vec<u8>, input: &[u8]) {
+    let mut i = 0;
+    while i < input.len() {
+        if input[i] == 0x1b && i + 1 < input.len() && input[i + 1] == b'[' {
+            i += 2;
+            while i < input.len() && !(0x40..=0x7e).contains(&input[i]) {
+                i += 1;
+            }
+            if i < input.len() {
+                i += 1;
+            }
+            continue;
+        }
+        out.push(input[i]);
+        i += 1;
+    }
+}
+
 // Remaining TODOs:
 // - Add stdout/stderr to the JUnit report
 // - Add timestamp field to the JUnit report
@@ -227,6 +267,10 @@ pub struct JunitReporter {
     pub suite_stack: Vec<SuiteInfo>,
     pub current_depth: u32,
 
+    /// Error captured by `on_uncaught_exception` for the currently-failing
+    /// test; consumed by `write_test_case` on the next `Result::Fail`.
+    pub last_failure: Option<JunitFailure>,
+
     pub hostname_value: Option<Box<[u8]>>,
 }
 
@@ -301,6 +345,94 @@ impl JunitReporter {
 
     // `pub const new = bun.TrivialNew(JunitReporter);` → Box::new
 
+    /// Capture name/message/stack from the `ZigException` that
+    /// `print_error_instance_body` has already populated, so the next
+    /// `write_test_case` can emit a useful `<failure>` without re-running
+    /// the exception formatter.
+    pub fn record_failure(&mut self, exception: &jsc::ZigException) {
+        let failure = self.last_failure.get_or_insert_default();
+        let name = exception.name.to_utf8();
+        let raw_message = exception.message.to_utf8();
+        let mut message = Vec::with_capacity(raw_message.slice().len());
+        push_stripping_ansi(&mut message, raw_message.slice());
+
+        let is_assertion = strings::has_prefix_comptime(&message, b"expect(")
+            && (name.slice().is_empty() || strings::eql(name.slice(), b"Error"));
+
+        if failure.name.is_empty() {
+            if is_assertion {
+                failure.name.extend_from_slice(b"AssertionError");
+            } else {
+                failure.name.extend_from_slice(name.slice());
+            }
+        }
+        if failure.message.is_empty() {
+            failure.message.extend_from_slice(&message);
+        }
+
+        let body = &mut failure.body;
+        if !body.is_empty() {
+            body.push(b'\n');
+        }
+        let header: &[u8] = if is_assertion {
+            b"AssertionError"
+        } else {
+            name.slice()
+        };
+        match (header.is_empty(), message.is_empty()) {
+            (true, true) => body.extend_from_slice(b"error"),
+            (true, false) => body.extend_from_slice(&message),
+            (false, true) => body.extend_from_slice(header),
+            (false, false) => {
+                body.extend_from_slice(header);
+                body.extend_from_slice(b": ");
+                body.extend_from_slice(&message);
+            }
+        }
+        body.push(b'\n');
+        let dir = FileSystem::instance().top_level_dir;
+        for frame in exception.stack.frames() {
+            let source_url = frame.source_url.to_utf8();
+            let file = resolve_path::relative(dir, source_url.slice());
+            let func = frame.function_name.to_utf8();
+            if file.is_empty() && func.slice().is_empty() {
+                continue;
+            }
+            body.extend_from_slice(b"      at ");
+            if !func.slice().is_empty() {
+                let _ = write!(body, "{} (", frame.name_formatter(false));
+            }
+            let file_start = body.len();
+            body.extend_from_slice(file);
+            if cfg!(windows) {
+                for b in &mut body[file_start..] {
+                    if *b == b'\\' {
+                        *b = b'/';
+                    }
+                }
+            }
+            let pos = frame.position;
+            if pos.line.is_valid() && pos.column.is_valid() {
+                let _ = write!(body, ":{}:{}", pos.line.one_based(), pos.column.one_based());
+            } else if pos.line.is_valid() {
+                let _ = write!(body, ":{}", pos.line.one_based());
+            }
+            if !func.slice().is_empty() {
+                body.push(b')');
+            }
+            body.push(b'\n');
+        }
+    }
+
+    /// VirtualMachine::on_print_error_zig_exception thunk.
+    pub fn record_failure_cb(ctx: *mut core::ffi::c_void, exception: &jsc::ZigException) {
+        // SAFETY: `ctx` was set to `&mut JunitReporter` by `on_uncaught_exception`
+        // for the duration of a single `run_error_handler` call; single-threaded,
+        // no other borrow of the reporter is live across that call.
+        let this = unsafe { &mut *ctx.cast::<JunitReporter>() };
+        this.record_failure(exception);
+    }
+
     fn generate_properties_list(&mut self) -> crate::Result<()> {
         struct PropertiesList<'a> {
             ci: &'a [u8],
@@ -583,16 +715,34 @@ impl JunitReporter {
                     let last = self.suite_stack.len() - 1;
                     self.suite_stack[last].metrics.failures += 1;
                 }
-                // TODO: add the failure message
-                // if (failure_message) |msg| {
-                //     try this.contents.appendSlice(bun.default_allocator, " message=\"");
-                //     try escapeXml(msg, this.contents.writer(bun.default_allocator));
-                //     try this.contents.appendSlice(bun.default_allocator, "\"");
-                // }
                 self.contents.extend_from_slice(b">\n");
                 self.contents.extend_from_slice(indent);
-                self.contents
-                    .extend_from_slice(b"  <failure type=\"AssertionError\" />\n");
+                let failure = self.last_failure.take();
+                let type_name: &[u8] = failure
+                    .as_ref()
+                    .map(|f| f.name.as_slice())
+                    .filter(|n| !n.is_empty())
+                    .unwrap_or(b"Error");
+                self.contents.extend_from_slice(b"  <failure type=\"");
+                escape_xml(type_name, &mut self.contents)?;
+                self.contents.extend_from_slice(b"\"");
+                if let Some(f) = failure.as_ref() {
+                    if !f.message.is_empty() {
+                        self.contents.extend_from_slice(b" message=\"");
+                        escape_xml(&f.message, &mut self.contents)?;
+                        self.contents.extend_from_slice(b"\"");
+                    }
+                }
+                match failure.as_ref().filter(|f| !f.body.is_empty()) {
+                    Some(f) => {
+                        self.contents.extend_from_slice(b">");
+                        escape_xml(&f.body, &mut self.contents)?;
+                        self.contents.extend_from_slice(b"</failure>\n");
+                    }
+                    None => {
+                        self.contents.extend_from_slice(b" />\n");
+                    }
+                }
                 self.contents.extend_from_slice(indent);
                 self.contents.extend_from_slice(b"</testcase>\n");
             }
@@ -686,13 +836,15 @@ impl JunitReporter {
                 }
                 self.contents.extend_from_slice(b">\n");
                 self.contents.extend_from_slice(indent);
-                self.contents
-                    .extend_from_slice(b"  <failure type=\"TimeoutError\" />\n");
+                self.contents.extend_from_slice(
+                    b"  <failure type=\"TimeoutError\" message=\"test timed out\" />\n",
+                );
                 self.contents.extend_from_slice(indent);
                 self.contents.extend_from_slice(b"</testcase>\n");
             }
             R::Pending => unreachable!(),
         }
+        self.last_failure = None;
         Ok(())
     }
 
@@ -1187,10 +1339,11 @@ impl CommandLineReporter {
                     if let Some(name) = unsafe { (*scope).base.name.as_deref() } {
                         if !name.is_empty() {
                             if initial_length != concatenated_describe_scopes.len() {
-                                concatenated_describe_scopes.extend_from_slice(b" &gt; ");
+                                concatenated_describe_scopes.extend_from_slice(b" > ");
                             }
 
-                            escape_xml(name, &mut concatenated_describe_scopes).expect("oom");
+                            // write_test_case escapes class_name once; do not pre-escape here.
+                            concatenated_describe_scopes.extend_from_slice(name);
                         }
                     }
                 }

src/runtime/test_runner/Execution.rs

@@ -537,13 +537,15 @@ impl Execution {
             // Handle retry logic: if test failed and we have retries remaining, retry it
             if test_failed && sequence.remaining_retry_count > 0 {
                 sequence.remaining_retry_count -= 1;
+                Execution::discard_junit_failure(buntest);
                 Execution::reset_sequence(sequence);
                 return;
             }
 
             // Handle repeat logic: if test passed and we have repeats remaining, repeat it
             if test_passed && sequence.remaining_repeat_count > 0 {
                 sequence.remaining_repeat_count -= 1;
+                Execution::discard_junit_failure(buntest);
                 Execution::reset_sequence(sequence);
                 return;
             }
@@ -701,6 +703,22 @@ impl Execution {
         }
     }
 
+    /// Drop any captured junit failure so the next retry/repeat starts fresh.
+    /// Kept out of `reset_sequence` so within-attempt errors (e.g. a throwing
+    /// afterEach after the test body already threw) accumulate instead of
+    /// clobbering the primary failure.
+    fn discard_junit_failure(buntest: NonNull<BunTest>) {
+        // SAFETY: `buntest` points at the live per-file BunTest; single-threaded
+        // test runner, no other borrow live here.
+        if let Some(reporter) = unsafe { (*buntest.as_ptr()).reporter } {
+            // SAFETY: `reporter` is a `NonNull<CommandLineReporter>` with write
+            // provenance (see BunTest docs); single-threaded, no other borrow.
+            if let Some(junit) = unsafe { (*reporter.as_ptr()).reporters.junit.as_deref_mut() } {
+                junit.last_failure = None;
+            }
+        }
+    }
+
     pub fn reset_sequence(sequence: &mut ExecutionSequence) {
         debug_assert!(!sequence.executing);
         {

src/runtime/test_runner/bun_test.rs

@@ -1312,6 +1312,21 @@ impl BunTest {
             return; // the exception should not be visible (eg m_terminationException)
         };
 
+        let junit_ctx: *mut core::ffi::c_void = 'ctx: {
+            if handle_status != HandleUncaughtExceptionResult::ShowHandledError {
+                break 'ctx core::ptr::null_mut();
+            }
+            let Some(reporter) = self.reporter else {
+                break 'ctx core::ptr::null_mut();
+            };
+            // SAFETY: `BunTest.reporter` carries write provenance from `enter_file`'s
+            // `&mut`; single-threaded test runner, no other borrow live here.
+            match unsafe { (*reporter.as_ptr()).reporters.junit.as_deref_mut() } {
+                Some(junit) => core::ptr::from_mut(junit).cast(),
+                None => core::ptr::null_mut(),
+            }
+        };
+
         self.bun_test_root.on_before_print();
         if matches!(
             handle_status,
@@ -1330,7 +1345,17 @@ impl BunTest {
             Output::flush();
         }
 
-        global_this.bun_vm().as_mut().run_error_handler(exception, None);
+        let vm = global_this.bun_vm().as_mut();
+        if !junit_ctx.is_null() {
+            vm.on_print_error_zig_exception =
+                Some(crate::cli::test_command::JunitReporter::record_failure_cb);
+            vm.on_print_error_zig_exception_ctx = junit_ctx;
+        }
+        vm.run_error_handler(exception, None);
+        if !junit_ctx.is_null() {
+            vm.on_print_error_zig_exception = None;
+            vm.on_print_error_zig_exception_ctx = core::ptr::null_mut();
+        }
 
         if matches!(
             handle_status,