From cb3cfb5da98a78d0353cb006df836b27e709a4ca Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 7 Nov 2026 01:19:33 +0000 Subject: [PATCH 1/3] bun test --reporter=junit: write a describe block as one in any order of results The nested report ended the of a describe block when the next test case belonged somewhere else, and started a second one when the block reported again. Concurrent tests of two blocks did that, and since #44833 so does an error outside of a test, which is a case of the file. The writer now keeps the describe blocks of the open file as a tree and writes their suites when the file ends. It finds the suite of a block by a key of the block: the run of the file and the address of the scope. A --parallel worker sends the key on its TestDone frame. Two blocks with one name still share a suite when one reports just after the other, as in every release. --- docs/test/reporters.mdx | 6 +- src/runtime/cli/test/parallel/Frame.rs | 4 +- src/runtime/cli/test/parallel/runner.rs | 15 +- src/runtime/cli/test_command.rs | 545 ++++++++++++++++-------- test/js/junit-reporter/junit.test.js | 453 +++++++++++++++++++- 5 files changed, 847 insertions(+), 176 deletions(-) diff --git a/docs/test/reporters.mdx b/docs/test/reporters.mdx index b599f96f865e..d046b3adf8ad 100644 --- a/docs/test/reporters.mdx +++ b/docs/test/reporters.mdx @@ -70,7 +70,7 @@ Every test file that ran has a ``, even if it has no tests. Its `time An uncaught exception or unhandled rejection while a test is running fails that test, and its ``, `` or `` has `uncaught="true"`. Examples are an error thrown by a timer and a rejected promise that the test didn't await. The attribute is absent when the test function itself throws, returns a promise that rejects, or calls `done(error)`. -An error outside of any test is reported as a failed `` named `(unhandled error)` in that file's ``. Examples are a test file or a preload script that throws while it loads, has a syntax error, or imports a module that doesn't exist. +An error outside of any test is reported as a failed `` named `(unhandled error)` in that file's ``. Examples are a test file or a preload script that throws while it loads, has a syntax error, or imports a module that doesn't exist. The `` is directly inside the file's ``, also when the error comes while the tests of a `describe` block run, for example from a timer that an earlier test left behind. With `--parallel`, a file whose worker crashed has a failed `` named `(worker crashed)`. The files that the crash kept from finishing or from starting have one named `(aborted)`. After `SIGINT` or `SIGTERM`, a file that was running has one named `(interrupted)`, and a file that hadn't started has one named `(aborted)`. @@ -85,7 +85,9 @@ junit = "path/to/junit.xml" # Output path for JUnit XML report #### Describe Blocks in JUnit Reports -By default, each `describe` block becomes a `` nested inside its file's ``. Some tools only read the `` elements directly inside a file's ``. For those tools, use `--reporter-junit-suites=flat`: +By default, each `describe` block becomes a `` nested inside its file's ``. A block is one ``, in whatever order its tests end. A block with an empty title, or with no test in the report, has no ``. + +Some tools only read the `` elements directly inside a file's ``. For those tools, use `--reporter-junit-suites=flat`: ```sh terminal icon="terminal" bun test --reporter=junit --reporter-outfile=./junit.xml --reporter-junit-suites=flat diff --git a/src/runtime/cli/test/parallel/Frame.rs b/src/runtime/cli/test/parallel/Frame.rs index 7035adecbd62..f63f80345a4b 100644 --- a/src/runtime/cli/test/parallel/Frame.rs +++ b/src/runtime/cli/test/parallel/Frame.rs @@ -14,7 +14,9 @@ pub(crate) enum Kind { /// then, only when the worker runs with `--reporter`, the structured /// result (`runner::encode_test_case`): u32 status, u32 assertions, /// u64 elapsed_ns, u32 line, str name, u32 n × {str scope_name, - /// u32 scope_line}, u32 has_failure [, str name, str message, str body, u32 uncaught] + /// u32 scope_line, u32 scope_run, u64 scope_address}, u32 has_failure, + /// u32 earlier_failures, then for each earlier failure and for the last + /// one if has_failure: str name, str message, str body, u32 uncaught TestDone, /// 10 × u32: file_idx, pass, fail, skip, todo, expectations, skipped_label, /// files, shuffled, unhandled; u64 elapsed_ns; 4 × u32, 0 where nobody measures: the megabytes by which the worker grew at most, those of its largest child, diff --git a/src/runtime/cli/test/parallel/runner.rs b/src/runtime/cli/test/parallel/runner.rs index bc8ea39e8ed6..b812a214acbb 100644 --- a/src/runtime/cli/test/parallel/runner.rs +++ b/src/runtime/cli/test/parallel/runner.rs @@ -1204,9 +1204,12 @@ fn encode_test_case(wf: &mut Frame, t: &test_command::TestCaseReport<'_>) { wf.u32(t.line_number); wf.str(t.name); wf.u32(u32::try_from(t.scopes.len()).expect("int cast")); - for &(name, line) in &t.scopes { - wf.str(name); - wf.u32(line); + for scope in &t.scopes { + wf.str(scope.name); + wf.u32(scope.line); + let (run, address) = scope.block.to_wire(); + wf.u32(run); + wf.u64(address); } // Earlier attempts' failures first, then the last attempt's if it failed. wf.u32(u32::from(t.failure.is_some())); @@ -1238,7 +1241,11 @@ pub(crate) fn decode_test_case<'a>( } let mut scopes = Vec::with_capacity(n); for _ in 0..n { - scopes.push((rd.str(), rd.u32())); + scopes.push(test_command::ScopeReport { + name: rd.str(), + line: rd.u32(), + block: test_command::BlockKey::from_wire(rd.u32(), rd.u64()), + }); } let has_failure = rd.u32() != 0; let earlier = rd.u32() as usize; diff --git a/src/runtime/cli/test_command.rs b/src/runtime/cli/test_command.rs index 7f6f844ec866..319b44c89b51 100644 --- a/src/runtime/cli/test_command.rs +++ b/src/runtime/cli/test_command.rs @@ -2133,12 +2133,49 @@ pub(crate) fn junit_file_name(path: &[u8]) -> &[u8] { } } +/// Names one `describe` block of one run of a test file. Its name and line do +/// not: the rows of a `describe.each` and the calls of a function that calls +/// `describe` have the same. +#[derive(Clone, Copy, PartialEq, Eq, Hash)] +pub(crate) struct BlockKey { + /// `BunTestRoot::file_generation`. Each `--rerun-each` repeat builds its blocks again, and can get the addresses of the repeat before. + run: u32, + /// Of the `DescribeScope`. Only a number: the report can outlive the scope, and a `--parallel` coordinator reads it in another process. + address: u64, +} + +impl BlockKey { + fn of(scope: &bun_test::DescribeScope, run: u32) -> BlockKey { + BlockKey { + run, + address: core::ptr::from_ref(scope).addr() as u64, + } + } + + /// What a `--parallel` worker sends on its `TestDone` frame. + pub(crate) fn to_wire(self) -> (u32, u64) { + (self.run, self.address) + } + + pub(crate) fn from_wire(run: u32, address: u64) -> BlockKey { + BlockKey { run, address } + } +} + +/// A named `describe` block around a finished test. +#[derive(Clone, Copy)] +pub(crate) struct ScopeReport<'a> { + pub name: &'a [u8], + pub line: u32, + pub block: BlockKey, +} + /// One finished test as structured reporters see it. pub(crate) struct TestCaseReport<'a> { /// Relative to the project root. pub file: &'a [u8], - /// Enclosing named `describe` blocks, outermost first: (name, line). - pub scopes: Vec<(&'a [u8], u32)>, + /// Enclosing named `describe` blocks, outermost first. + pub scopes: Vec>, pub name: &'a [u8], pub status: bun_test::Execution::Result, pub assertions: u32, @@ -2214,25 +2251,21 @@ pub(crate) struct JunitReporter { pub(crate) file_start_unix_ms: i64, pub(crate) properties_list_to_repeat_in_every_test_suite: Option>, - pub(crate) suite_stack: Vec, - pub(crate) current_depth: u32, + /// The `` of `current_file`. Its opening tag is in `contents`. + file_suite: Option, + /// The `describe` blocks of `current_file`, for the nested form. + blocks: DescribeBlocks, pub(crate) suites: JunitSuites, pub(crate) hostname_value: Option>, } -#[derive(Default)] -pub(crate) struct SuiteInfo { - pub name: Box<[u8]>, - pub(crate) offset_of_attributes: usize, - pub(crate) metrics: Metrics, - pub(crate) is_file_suite: bool, - pub(crate) started_ns: u64, +struct FileSuite { + offset_of_attributes: usize, + metrics: Metrics, + started_ns: u64, } -// We dupe the name unconditionally in begin_test_suite_with_line, so the -// unconditional drop is correct. - #[derive(Default, Clone, Copy)] pub(crate) struct Metrics { pub(crate) test_cases: u32, @@ -2251,6 +2284,195 @@ impl Metrics { } } +/// The `describe` blocks of the file whose `` is open. A block's +/// `` is written when the file ends, because the cases of one block +/// don't arrive together: concurrent tests of two blocks end in any order, and +/// an error outside of a test is a case of the file. +#[derive(Default)] +struct DescribeBlocks { + suites: Vec, + by_key: bun_collections::HashMap, + /// The innermost block of the case before and its suite, which are most often those of the next case. + last: Option<(BlockKey, usize)>, + /// The `` elements of the blocks, and those of the file itself that came after its first block. + cases: Vec, + /// What the file's `` holds from its first block on. + top_level: Vec, +} + +struct BlockSuite { + name: Box<[u8]>, + line: u32, + parent: Option, + /// 1 for a block at the top level of the file. + depth: u32, + /// Of the suite's own cases, until `write` adds those of the blocks inside it. + metrics: Metrics, + /// In the order reported. + children: Vec, +} + +enum Child { + /// A range of `DescribeBlocks::cases`. + Cases(core::ops::Range), + /// An index of `DescribeBlocks::suites`. + Suite(usize), +} + +impl DescribeBlocks { + /// The suite of the innermost named block around the test. `None` for a case of the file itself. + fn suite_of(&mut self, t: &TestCaseReport<'_>) -> Option { + // Only the flat form includes describe blocks with an empty title, like Vitest. + let named = |scope: &&ScopeReport<'_>| !scope.name.is_empty(); + let innermost = t.scopes.iter().rfind(named)?; + if let Some((block, suite)) = self.last + && block == innermost.block + { + return Some(suite); + } + + // The innermost block that has a suite, then a suite for each block inside it. + let mut suite = None; + let mut first_without = t.scopes.len(); + for (index, scope) in t.scopes.iter().enumerate().rev() { + if scope.name.is_empty() { + continue; + } + if let Some(&found) = self.by_key.get(&scope.block) { + suite = Some(found); + break; + } + first_without = index; + } + for scope in t.scopes[first_without..].iter().filter(named) { + // As in every release, a block shares the suite of a block with its name that reported just before it: + // the rows of a `describe.each` with one title, and the repeats of `--rerun-each`. + let before = match self.children_of(suite).last() { + Some(&Child::Suite(before)) => Some(before), + _ => None, + }; + let inside = match before { + Some(before) if strings::eql(&self.suites[before].name, scope.name) => before, + _ => self.add(suite, scope), + }; + self.by_key.insert(scope.block, inside); + suite = Some(inside); + } + self.last = suite.map(|suite| (innermost.block, suite)); + suite + } + + fn add(&mut self, parent: Option, scope: &ScopeReport<'_>) -> usize { + let added = self.suites.len(); + self.suites.push(BlockSuite { + name: Box::from(scope.name), + line: scope.line, + parent, + depth: parent.map_or(1, |parent| self.suites[parent].depth + 1), + metrics: Metrics::default(), + children: Vec::new(), + }); + self.children_of(parent).push(Child::Suite(added)); + added + } + + fn children_of(&mut self, parent: Option) -> &mut Vec { + match parent { + Some(suite) => &mut self.suites[suite].children, + None => &mut self.top_level, + } + } + + /// Gives `parent` the `` elements written to `cases` from `start` on. + fn append_cases(&mut self, parent: Option, start: usize) { + let end = self.cases.len(); + let children = self.children_of(parent); + if let Some(Child::Cases(range)) = children.last_mut() + && range.end == start + { + range.end = end; + } else { + children.push(Child::Cases(start..end)); + } + } + + /// Writes the `` of each block around what it holds, adds the + /// counts of the blocks to those of the file, and forgets the file. + #[optimize(size)] + fn write( + &mut self, + out: &mut Vec, + file: &[u8], + file_metrics: &mut Metrics, + hostname: &[u8], + started_unix_ms: i64, + ) -> crate::Result<()> { + if self.suites.is_empty() { + return Ok(()); + } + + // A block is added after the block around it, so its counts are complete before they are added to the parent's. + for index in (0..self.suites.len()).rev() { + let metrics = self.suites[index].metrics; + match self.suites[index].parent { + Some(parent) => self.suites[parent].metrics.add(&metrics), + None => file_metrics.add(&metrics), + } + } + + // The suites that are open, outermost first, each with the number of its children that are written. + let mut open: Vec<(Option<&BlockSuite>, usize)> = vec![(None, 0)]; + while let Some((parent, written)) = open.last_mut() { + let children = parent.map_or(&self.top_level, |suite| &suite.children); + let Some(child) = children.get(*written) else { + if let Some(suite) = parent { + out.extend_from_slice(JunitReporter::get_indent(suite.depth)); + out.extend_from_slice(b"\n"); + } + open.pop(); + continue; + }; + *written += 1; + match child { + Child::Cases(range) => out.extend_from_slice(&self.cases[range.clone()]), + Child::Suite(index) => { + let suite = &self.suites[*index]; + JunitReporter::write_suite_tag( + out, + suite.depth, + &suite.name, + file, + suite.line, + )?; + JunitReporter::write_suite_attributes( + out, + &suite.metrics, + suite.metrics.elapsed_time as f64 / bun::time::MS_PER_S as f64, + hostname, + started_unix_ms, + ); + out.extend_from_slice(b">\n"); + open.push((Some(suite), 0)); + } + } + } + + self.suites.clear(); + self.by_key.clear(); + self.last = None; + self.cases.clear(); + self.top_level.clear(); + Ok(()) + } +} + +/// Where a `` goes: the `` of a file, or that of a `describe` block. +struct CaseWriter<'a> { + contents: &'a mut Vec, + metrics: &'a mut Metrics, + depth: u32, +} + impl JunitReporter { pub(crate) fn get_hostname(&mut self) -> Option<&[u8]> { if self.hostname_value.is_none() { @@ -2552,70 +2774,75 @@ impl JunitReporter { } } - pub(crate) fn begin_test_suite(&mut self, name: &[u8]) -> crate::Result<()> { - self.begin_test_suite_with_line(name, 0, true) - } - - #[optimize(size)] - pub(crate) fn begin_test_suite_with_line( - &mut self, + /// Writes `, + depth: u32, name: &[u8], + file: &[u8], line_number: u32, - is_file_suite: bool, ) -> crate::Result<()> { - self.start_document(); - - let indent = Self::get_indent(self.current_depth); - self.contents.extend_from_slice(indent); - self.contents.extend_from_slice(b" 0 { // Std::io::Write removed; bun_io::Write (top-level) provides write_fmt. - let _ = write!(&mut self.contents, " line=\"{}\"", line_number); + let _ = write!(out, " line=\"{}\"", line_number); } + out.extend_from_slice(b" "); + Ok(()) + } + + fn write_suite_attributes( + out: &mut Vec, + metrics: &Metrics, + elapsed_time_seconds: f64, + hostname: &[u8], + started_unix_ms: i64, + ) { + // Std::io::Write removed; bun_io::Write (top-level) provides write_fmt. + let _ = write!( + out, + "tests=\"{}\" assertions=\"{}\" failures=\"{}\" skipped=\"{}\" time=\"{}\" hostname=\"{}\" timestamp=\"", + metrics.test_cases, + metrics.assertions, + metrics.failures, + metrics.skipped, + elapsed_time_seconds, + bstr::BStr::new(hostname), + ); + write_iso_8601(out, started_unix_ms); + out.push(b'"'); + } - self.contents.extend_from_slice(b" "); + /// Opens the `` of a test file. + #[optimize(size)] + pub(crate) fn begin_test_suite(&mut self, name: &[u8]) -> crate::Result<()> { + self.start_document(); + + Self::write_suite_tag(&mut self.contents, 0, name, name, 0)?; let offset_of_attributes = self.contents.len(); self.contents.extend_from_slice(b">\n"); - if is_file_suite { - if self.properties_list_to_repeat_in_every_test_suite.is_none() { - self.generate_properties_list()?; - } + if self.properties_list_to_repeat_in_every_test_suite.is_none() { + self.generate_properties_list()?; + } - if let Some(properties_list) = &self.properties_list_to_repeat_in_every_test_suite { - if !properties_list.is_empty() { - self.contents.extend_from_slice(properties_list); - } + if let Some(properties_list) = &self.properties_list_to_repeat_in_every_test_suite { + if !properties_list.is_empty() { + self.contents.extend_from_slice(properties_list); } } - self.suite_stack.push(SuiteInfo { - name: Box::<[u8]>::from(name), + self.file_suite = Some(FileSuite { offset_of_attributes, metrics: Metrics::default(), - is_file_suite, - started_ns: if is_file_suite { self.file_start_ns } else { 0 }, + started_ns: self.file_start_ns, }); - - self.current_depth += 1; - if is_file_suite { - self.current_file = Box::<[u8]>::from(name); - } + self.current_file = Box::<[u8]>::from(name); Ok(()) } @@ -2634,131 +2861,122 @@ impl JunitReporter { self.end_file(None, None)?; self.begin_test_suite(file)?; } - while !self.suite_stack.is_empty() { - self.end_test_suite(elapsed_ns)?; - } + self.end_test_suite(elapsed_ns)?; self.current_file = Box::default(); Ok(()) } + /// Writes the suites of the file's `describe` blocks, then closes the file's own. #[optimize(size)] fn end_test_suite(&mut self, file_elapsed_ns: Option) -> crate::Result<()> { - if self.suite_stack.is_empty() { + let Some(mut suite) = self.file_suite.take() else { return Ok(()); - } - - self.current_depth -= 1; - let suite_info = self.suite_stack.swap_remove(self.suite_stack.len() - 1); - - let elapsed_time_seconds = if suite_info.is_file_suite - && let Some(ns) = file_elapsed_ns - { - ns as f64 / bun::time::NS_PER_S as f64 - } else if suite_info.is_file_suite && suite_info.started_ns > 0 { - bun::Timespec::now(bun::TimespecMockMode::ForceRealTime) - .ns() - .saturating_sub(suite_info.started_ns) as f64 - / bun::time::NS_PER_S as f64 - } else { - suite_info.metrics.elapsed_time as f64 / bun::time::MS_PER_S as f64 }; // Reshaped for borrowck — get hostname first let hostname = self.get_hostname().map(|h| h.to_vec()).unwrap_or_default(); + self.blocks.write( + &mut self.contents, + &self.current_file, + &mut suite.metrics, + &hostname, + self.file_start_unix_ms, + )?; + + let elapsed_ns = file_elapsed_ns.unwrap_or_else(|| { + bun::Timespec::now(bun::TimespecMockMode::ForceRealTime) + .ns() + .saturating_sub(suite.started_ns) + }); + let elapsed_time_seconds = elapsed_ns as f64 / bun::time::NS_PER_S as f64; + // Insert the summary attributes let mut summary = Vec::new(); - { - // Std::io::Write removed; bun_io::Write (top-level) provides write_fmt. - let _ = write!( - &mut summary, - "tests=\"{}\" assertions=\"{}\" failures=\"{}\" skipped=\"{}\" time=\"{}\" hostname=\"{}\" timestamp=\"", - suite_info.metrics.test_cases, - suite_info.metrics.assertions, - suite_info.metrics.failures, - suite_info.metrics.skipped, - elapsed_time_seconds, - bstr::BStr::new(&hostname), - ); - write_iso_8601(&mut summary, self.file_start_unix_ms); - summary.push(b'"'); - } - + Self::write_suite_attributes( + &mut summary, + &suite.metrics, + elapsed_time_seconds, + &hostname, + self.file_start_unix_ms, + ); self.contents.splice( - suite_info.offset_of_attributes..suite_info.offset_of_attributes, + suite.offset_of_attributes..suite.offset_of_attributes, summary.iter().copied(), ); - let indent = Self::get_indent(self.current_depth); - self.contents.extend_from_slice(indent); + self.contents.extend_from_slice(Self::get_indent(0)); self.contents.extend_from_slice(b"\n"); - if !self.suite_stack.is_empty() { - let last = self.suite_stack.len() - 1; - self.suite_stack[last].metrics.add(&suite_info.metrics); - } else { - self.total_metrics.add(&suite_info.metrics); - } + self.total_metrics.add(&suite.metrics); Ok(()) } - /// Open/close `` elements so the stack matches `t.file` and - /// `t.scopes`, then write the ``. + /// Writes the `` into the suite of `t.file`. In the nested form + /// a test inside a named `describe` block goes into the suite of that block. #[optimize(size)] pub(crate) fn record_test_case(&mut self, t: &TestCaseReport<'_>) -> crate::Result<()> { - if !strings::eql(&self.current_file, t.file) { - while let Some(top) = self.suite_stack.last() - && !top.is_file_suite - { - self.end_test_suite(None)?; - } - if !self.current_file.is_empty() { - self.end_test_suite(None)?; - } + if self.file_suite.is_none() || !strings::eql(&self.current_file, t.file) { + self.end_test_suite(None)?; self.begin_test_suite(t.file)?; } + let file_suite = self.file_suite.as_mut().expect("opened above"); if self.suites == JunitSuites::Flat { let mut name: Vec = Vec::new(); - for &(scope, _) in &t.scopes { - name.extend_from_slice(scope); + for scope in &t.scopes { + name.extend_from_slice(scope.name); name.extend_from_slice(b" > "); } name.extend_from_slice(t.name); - return self.write_test_case(t, &name, t.file); - } - - // Only the flat form includes describe blocks with an empty title, like Vitest. - let mut scopes = t.scopes.clone(); - scopes.retain(|(name, _)| !name.is_empty()); - - // Keep the longest prefix of open describe suites that matches - // `scopes`; close the rest, then open what is missing. - let open = &self.suite_stack[1..]; - let keep = open - .iter() - .zip(&scopes) - .take_while(|(suite, (name, _))| strings::eql(&suite.name, name)) - .count(); - for _ in keep..open.len() { - self.end_test_suite(None)?; - } - for &(name, line) in &scopes[keep..] { - self.begin_test_suite_with_line(name, line, false)?; + return CaseWriter { + contents: &mut self.contents, + metrics: &mut file_suite.metrics, + depth: 1, + } + .write_test_case(t, &name, t.file); } // Innermost scope first, matching what the serial reporter always did. + // Only the flat form includes describe blocks with an empty title, like Vitest. let mut class_name: Vec = Vec::new(); - for (i, &(name, _)) in scopes.iter().rev().enumerate() { - if i > 0 { + for scope in t.scopes.iter().rev().filter(|scope| !scope.name.is_empty()) { + if !class_name.is_empty() { class_name.extend_from_slice(b" > "); } - class_name.extend_from_slice(name); + class_name.extend_from_slice(scope.name); } - self.write_test_case(t, t.name, &class_name) + let suite = self.blocks.suite_of(t); + if suite.is_none() && self.blocks.suites.is_empty() { + // No block of the file has a suite yet, so this case comes before all of them. + return CaseWriter { + contents: &mut self.contents, + metrics: &mut file_suite.metrics, + depth: 1, + } + .write_test_case(t, t.name, &class_name); + } + let start = self.blocks.cases.len(); + let (metrics, depth) = match suite { + Some(suite) => { + let suite = &mut self.blocks.suites[suite]; + (&mut suite.metrics, suite.depth + 1) + } + None => (&mut file_suite.metrics, 1), + }; + CaseWriter { + contents: &mut self.blocks.cases, + metrics, + depth, + } + .write_test_case(t, t.name, &class_name)?; + self.blocks.append_cases(suite, start); + Ok(()) } +} +impl CaseWriter<'_> { /// Writes ``, or Surefire's `` / `` for a retried test. #[optimize(size)] fn write_failure( @@ -2820,18 +3038,11 @@ impl JunitReporter { let elapsed_ns_f64: f64 = elapsed_ns as f64; let elapsed_ms = elapsed_ns_f64 / bun::time::NS_PER_MS as f64; - if !self.suite_stack.is_empty() { - let last = self.suite_stack.len() - 1; - let current_suite = &mut self.suite_stack[last]; - current_suite.metrics.elapsed_time = current_suite - .metrics - .elapsed_time - .saturating_add(elapsed_ms as u64); - current_suite.metrics.test_cases += 1; - current_suite.metrics.assertions += assertions; - } + self.metrics.elapsed_time = self.metrics.elapsed_time.saturating_add(elapsed_ms as u64); + self.metrics.test_cases += 1; + self.metrics.assertions += assertions; - let indent = Self::get_indent(self.current_depth); + let indent = JunitReporter::get_indent(self.depth); self.contents.extend_from_slice(indent); self.contents.extend_from_slice(b"\n"); } _ if status.is_fail() => { - if !self.suite_stack.is_empty() { - let last = self.suite_stack.len() - 1; - self.suite_stack[last].metrics.failures += 1; - } + self.metrics.failures += 1; self.contents.extend_from_slice(b">\n"); let described = TestFailure::from_result(status, assertions); self.write_failure(indent, b"failure", described.as_ref().or(failure))?; @@ -2885,10 +3093,7 @@ impl JunitReporter { self.contents.extend_from_slice(b"\n"); } R::SkippedBecauseLabel | R::Skip => { - if !self.suite_stack.is_empty() { - let last = self.suite_stack.len() - 1; - self.suite_stack[last].metrics.skipped += 1; - } + self.metrics.skipped += 1; self.contents.extend_from_slice(b">\n"); self.contents.extend_from_slice(indent); self.contents.extend_from_slice(b" \n"); @@ -2896,10 +3101,7 @@ impl JunitReporter { self.contents.extend_from_slice(b"\n"); } R::Todo => { - if !self.suite_stack.is_empty() { - let last = self.suite_stack.len() - 1; - self.suite_stack[last].metrics.skipped += 1; - } + self.metrics.skipped += 1; self.contents.extend_from_slice(b">\n"); self.contents.extend_from_slice(indent); self.contents @@ -2911,7 +3113,9 @@ impl JunitReporter { } Ok(()) } +} +impl JunitReporter { pub(crate) fn write_to_file(&mut self, path: &[u8]) -> crate::Result<()> { if self.contents.is_empty() { return Ok(()); @@ -3331,8 +3535,9 @@ impl CommandLineReporter { }; let file = junit_file_name(file); + let run = buntest.bun_test_root.file_generation; // Innermost first while walking up; reversed below. - let mut scopes: Vec<(&'a [u8], u32)> = Vec::new(); + let mut scopes: Vec> = Vec::new(); let mut parent = test_entry.base.parent.map(|p| p.cast_const()); while let Some(scope) = parent { if scopes.len() == 64 { @@ -3341,7 +3546,11 @@ impl CommandLineReporter { // SAFETY: describe scopes outlive the file's test run. let scope: &'a bun_test::DescribeScope = unsafe { &*scope }; if let Some(name) = scope.base.name.as_deref() { - scopes.push((name, scope.base.line_no)); + scopes.push(ScopeReport { + name, + line: scope.base.line_no, + block: BlockKey::of(scope, run), + }); } parent = scope.base.parent.map(|p| p.cast_const()); } diff --git a/test/js/junit-reporter/junit.test.js b/test/js/junit-reporter/junit.test.js index ccaee74e0edb..26915cb94008 100644 --- a/test/js/junit-reporter/junit.test.js +++ b/test/js/junit-reporter/junit.test.js @@ -1,6 +1,6 @@ import { file, spawn } from "bun"; import { describe, expect, it } from "bun:test"; -import { bunEnv, bunExe, tempDir } from "harness"; +import { bunEnv, bunExe, tempDir, withoutAsanQuarantine } from "harness"; import { join } from "node:path"; const xml2js = require("xml2js"); @@ -1235,6 +1235,457 @@ describe("only block", () => { expect(stderr).toContain(`Failed to write JUnit report to ${outfile}`); expect(exitCode).toBe(0); }); + + // Not concurrent: each test starts its `bun test` runs at once, and four tests at once make a run several times slower. + describe("a describe block is one ", () => { + // Each test file says which process loaded it. + const sayPid = `require("node:fs").appendFileSync(process.env.PIDS, process.pid + "\\n");\n`; + + async function report(files, args, env = bunEnv) { + const fixtures = { "package.json": "{}" }; + for (const [name, source] of Object.entries(files)) fixtures[name] = source + sayPid; + using dir = tempDir("junit-blocks", fixtures); + const pids = join(String(dir), "pids"); + await using proc = spawn([bunExe(), "test", "--reporter=junit", "--reporter-outfile=junit.xml", ...args], { + cwd: String(dir), + env: { ...env, PIDS: pids }, + stdout: "ignore", + stderr: "ignore", + }); + const exitCode = await proc.exited; + const loadedIn = (await file(pids).text()).trim().split("\n").map(Number); + return { + xml: filterJunitXmlOutput(await file(join(String(dir), "junit.xml")).text()), + exitCode, + // With one test file, `bun test --parallel` loads the file itself and starts no worker. + usedWorkers: !loadedIn.includes(proc.pid), + }; + } + + // The and elements of each file, one per line, indented as they are nested. + function outline(xml) { + const files = {}; + let lines; + for (const [, indent, element, name, attributes] of xml.matchAll( + /^( *)<(testsuite|testcase) name="([^"]*)"([^>]*)>$/gm, + )) { + const attribute = key => attributes.match(new RegExp(` ${key}="([^"]*)"`))?.[1]; + if (indent.length === 2) lines = files[name] = []; + const line = attribute("line") ? ` line=${attribute("line")}` : ""; + lines.push( + indent.slice(2) + + (element === "testcase" + ? `case ${name.replaceAll(">", ">")}` + : `suite ${name}${line} tests=${attribute("tests")} failures=${attribute("failures")}`), + ); + } + return files; + } + + // Attributes of and of each file's . + const totals = xml => + Array.from(xml.matchAll(/^ {0,2}$/gm), m => m[1]); + + // Lets a concurrent test end after the runner has reported another one, without waiting for time to pass. + const turns = `const turn = () => new Promise(resolve => setImmediate(resolve)); +const turns = async () => { for (let i = 0; i < 5; i++) await turn(); };`; + + // `endInOrder(order)(name)` is a test that ends after the runner has reported the tests before `name` in `order`. + const endInOrder = `function endInOrder(order) { + const ended = order.map(() => Promise.withResolvers()); + return name => async () => { + const at = order.indexOf(name); + if (at > 0) { + await ended[at - 1].promise; + await turns(); + } + ended[at].resolve(); + }; +}`; + + const noBlock = `import { test } from "bun:test"; +test("not in a block", () => {}); +`; + + it("whatever the order of the tests and of the errors outside of a test", async () => { + const files = { + "hook-each.test.js": `import { afterEach, describe, test } from "bun:test"; +describe("d", () => { + afterEach(done => { + done(new Error("from afterEach")); + }); + test("a", () => {}); + test("b", () => {}); +}); +`, + "hook-all.test.js": `import { beforeAll, describe, test } from "bun:test"; +describe("outer", () => { + test("o1", () => {}); + describe("inner", () => { + beforeAll(done => { + done(new Error("from beforeAll")); + }); + test("i1", () => {}); + }); + test("o2", () => {}); +}); +`, + "timer.test.js": `import { describe, test } from "bun:test"; +${turns} +describe("d", () => { + test.concurrent("a", async () => {}); + test.concurrent("b", async () => { + await turns(); + const thrown = Promise.withResolvers(); + setImmediate(() => { + thrown.resolve(); + throw new Error("from a timer"); + }); + await thrown.promise; + await turns(); + }); +}); +`, + "concurrent.test.js": `import { describe, test } from "bun:test"; +${turns} +${endInOrder} +const end = endInOrder(["a1", "b1", "a2"]); +describe("a", () => { + test.concurrent("a1", end("a1")); + test.concurrent("a2", end("a2")); +}); +describe("b", () => { + test.concurrent("b1", end("b1")); +}); +`, + // Both rows report before "third" does, and a row reports again after it. + "rows.test.js": `import { describe, test } from "bun:test"; +${turns} +${endInOrder} +const end = endInOrder(["a of 0", "a of 1", "in third", "b of 1", "b of 0"]); +describe.each([0, 1])("row", row => { + test.concurrent("a of " + row, end("a of " + row)); + test.concurrent("b of " + row, end("b of " + row)); +}); +describe("third", () => { + test.concurrent("in third", end("in third")); +}); +`, + "same-name.test.js": `import { describe, test } from "bun:test"; +describe.each([1, 2])("each", row => { + test("of row " + row, () => {}); +}); +describe("twice", () => { + test("in the first", () => {}); +}); +describe("twice", () => { + test("in the second", () => {}); +}); +function helper(name) { + describe("helper", () => { + test(name, () => {}); + }); +} +helper("first call"); +describe("other", () => { + test("between", () => {}); +}); +helper("second call"); +`, + }; + const [serial, parallel, flat] = await Promise.all([ + report(files, []), + report(files, ["--parallel=2"]), + report(files, ["--reporter-junit-suites=flat"]), + ]); + + // The flat form has the cases in the order reported. + expect(outline(flat.xml)).toEqual({ + "concurrent.test.js": [ + "suite concurrent.test.js tests=3 failures=0", + " case a > a1", + " case b > b1", + " case a > a2", + ], + "hook-all.test.js": [ + "suite hook-all.test.js tests=4 failures=1", + " case outer > o1", + " case (unhandled error)", + " case outer > inner > i1", + " case outer > o2", + ], + "hook-each.test.js": [ + "suite hook-each.test.js tests=4 failures=2", + " case (unhandled error)", + " case d > a", + " case (unhandled error)", + " case d > b", + ], + "rows.test.js": [ + "suite rows.test.js tests=5 failures=0", + " case row > a of 0", + " case row > a of 1", + " case third > in third", + " case row > b of 1", + " case row > b of 0", + ], + "same-name.test.js": [ + "suite same-name.test.js tests=7 failures=0", + " case each > of row 1", + " case each > of row 2", + " case twice > in the first", + " case twice > in the second", + " case helper > first call", + " case other > between", + " case helper > second call", + ], + "timer.test.js": [ + "suite timer.test.js tests=3 failures=1", + " case d > a", + " case (unhandled error)", + " case d > b", + ], + }); + // An error outside of a test stays a case of the file, where it was reported among the file's blocks and cases. + // Two blocks with one name share a when one reports just after the other, as before. + expect(outline(serial.xml)).toEqual({ + "concurrent.test.js": [ + "suite concurrent.test.js tests=3 failures=0", + " suite a line=16 tests=2 failures=0", + " case a1", + " case a2", + " suite b line=20 tests=1 failures=0", + " case b1", + ], + "hook-all.test.js": [ + "suite hook-all.test.js tests=4 failures=1", + " suite outer line=2 tests=3 failures=0", + " case o1", + " suite inner line=4 tests=1 failures=0", + " case i1", + " case o2", + " case (unhandled error)", + ], + "hook-each.test.js": [ + "suite hook-each.test.js tests=4 failures=2", + " case (unhandled error)", + " suite d line=2 tests=2 failures=0", + " case a", + " case b", + " case (unhandled error)", + ], + "rows.test.js": [ + "suite rows.test.js tests=5 failures=0", + " suite row line=16 tests=4 failures=0", + " case a of 0", + " case a of 1", + " case b of 1", + " case b of 0", + " suite third line=20 tests=1 failures=0", + " case in third", + ], + "same-name.test.js": [ + "suite same-name.test.js tests=7 failures=0", + " suite each line=2 tests=2 failures=0", + " case of row 1", + " case of row 2", + " suite twice line=5 tests=2 failures=0", + " case in the first", + " case in the second", + " suite helper line=12 tests=1 failures=0", + " case first call", + " suite other line=17 tests=1 failures=0", + " case between", + " suite helper line=12 tests=1 failures=0", + " case second call", + ], + "timer.test.js": [ + "suite timer.test.js tests=3 failures=1", + " suite d line=4 tests=2 failures=0", + " case a", + " case b", + " case (unhandled error)", + ], + }); + expect(parallel.xml).toBe(serial.xml); + expect(totals(serial.xml)).toEqual(totals(flat.xml)); + expect(totals(flat.xml)).toHaveLength(7); + expect([serial.usedWorkers, parallel.usedWorkers]).toEqual([false, true]); + expect([serial.exitCode, parallel.exitCode, flat.exitCode]).toEqual([1, 1, 1]); + }); + + it("in every order in which the tests of three blocks can end", async () => { + const orders = [["a1"]]; + for (const name of ["i1", "a2", "b1"]) { + for (const order of orders.splice(0)) { + for (let at = 0; at <= order.length; at++) orders.push(order.toSpliced(at, 0, name)); + } + } + const files = { + "orders.test.js": `import { describe, test } from "bun:test"; +${turns} +${endInOrder} +for (const order of ${JSON.stringify(orders)}) { + const end = endInOrder(order); + describe(order.join(" "), () => { + describe("a", () => { + test.concurrent("a1", end("a1")); + describe("inner", () => { + test.concurrent("i1", end("i1")); + }); + test.concurrent("a2", end("a2")); + }); + describe("b", () => { + test.concurrent("b1", end("b1")); + }); + }); +} +`, + }; + const [nested, flat] = await Promise.all([report(files, []), report(files, ["--reporter-junit-suites=flat"])]); + + // The flat form has the cases in the order reported. + const reported = outline(flat.xml)["orders.test.js"].slice(1); + expect(orders.map(order => reported.filter(line => line.startsWith(` case ${order.join(" ")} > `)))).toEqual( + orders.map(order => + order.map(name => ` case ${order.join(" ")} > ${name === "i1" ? "a > inner" : name[0]} > ${name}`), + ), + ); + // Each with the names of those around it. Which block reports first decides the order of two blocks. + const around = []; + const suites = []; + for (const line of outline(nested.xml)["orders.test.js"].slice(1)) { + const [, indent, name, tests] = line.match(/^( *)suite (.*) line=\d+ (tests=\d+) /) ?? []; + if (name === undefined) continue; + around.length = indent.length / 2 - 1; + around.push(name); + suites.push(`${around.join(" > ")} ${tests}`); + } + const once = orders + .flatMap(order => { + const block = order.join(" "); + return [`${block} tests=4`, `${block} > a tests=3`, `${block} > a > inner tests=1`, `${block} > b tests=1`]; + }) + .sort(); + expect(orders).toHaveLength(24); + expect(suites.sort()).toEqual(once); + expect([nested.exitCode, flat.exitCode]).toEqual([0, 0]); + }); + + it("with --rerun-each, when a block gets the address that a block of the run before had", async () => { + const files = { + "concurrent.test.js": `import { describe, test } from "bun:test"; +${turns} +${endInOrder} +const end = endInOrder(["a1", "b1", "a2"]); +describe("a", () => { + test.concurrent("a1", end("a1")); + test.concurrent("a2", end("a2")); +}); +describe("b", () => { + test.concurrent("b1", end("b1")); +}); +`, + "many.test.js": `import { describe, test } from "bun:test"; +for (let block = 0; block < 50; block++) { + describe("block " + block, () => { + test("in block " + block, () => {}); + }); +} +`, + "wrapped.test.js": `import { describe, test } from "bun:test"; +describe("outer", () => { + describe("x", () => { + test("x1", () => {}); + }); + describe("y", () => { + test("y1", () => {}); + }); +}); +`, + }; + // Without ASAN's quarantine, a block of the second run can get the address that a block of the first run had. + const env = withoutAsanQuarantine(bunEnv); + const [serial, parallel] = await Promise.all([ + report(files, ["--rerun-each=2"], env), + report(files, ["--rerun-each=2", "--parallel=2"], env), + ]); + + const many = Array.from({ length: 50 }, (_, block) => [ + ` suite block ${block} line=3 tests=1 failures=0`, + ` case in block ${block}`, + ]).flat(); + // The block around a whole file is one for all the runs, as before. + expect(outline(serial.xml)).toEqual({ + "concurrent.test.js": [ + "suite concurrent.test.js tests=6 failures=0", + " suite a line=16 tests=2 failures=0", + " case a1", + " case a2", + " suite b line=20 tests=1 failures=0", + " case b1", + " suite a line=16 tests=2 failures=0", + " case a1", + " case a2", + " suite b line=20 tests=1 failures=0", + " case b1", + ], + "many.test.js": ["suite many.test.js tests=100 failures=0", ...many, ...many], + "wrapped.test.js": [ + "suite wrapped.test.js tests=4 failures=0", + " suite outer line=2 tests=4 failures=0", + " suite x line=3 tests=1 failures=0", + " case x1", + " suite y line=6 tests=1 failures=0", + " case y1", + " suite x line=3 tests=1 failures=0", + " case x1", + " suite y line=6 tests=1 failures=0", + " case y1", + ], + }); + expect(parallel.xml).toBe(serial.xml); + expect([serial.usedWorkers, parallel.usedWorkers]).toEqual([false, true]); + expect([serial.exitCode, parallel.exitCode]).toEqual([0, 0]); + }); + + it("when --bail ends the run inside it", async () => { + const { xml, exitCode } = await report( + { + "a.test.js": `import { describe, test } from "bun:test"; +${turns} +${endInOrder} +const end = endInOrder(["a1", "b1", "a2 fails"]); +describe("a", () => { + test.concurrent("a1", end("a1")); + test.concurrent("a2 fails", async () => { + await end("a2 fails")(); + throw new Error("from the test"); + }); +}); +describe("b", () => { + test.concurrent("b1", end("b1")); +}); +test("never runs", () => {}); +`, + "b.test.js": noBlock, + }, + ["--bail=1"], + ); + + expect(outline(xml)).toEqual({ + "a.test.js": [ + "suite a.test.js tests=3 failures=1", + " suite a line=16 tests=2 failures=1", + " case a1", + " case a2 fails", + " suite b line=23 tests=1 failures=0", + " case b1", + ], + }); + expect(exitCode).toBe(1); + }); + }); }); function filterJunitXmlOutput(xmlContent) { From 835bc52be7bb8d37ec06d5c6bf4a8b24c2945ba5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 7 Nov 2026 12:54:40 +0000 Subject: [PATCH 2/3] junit reporter: stamp a describe suite with the run it reported in A describe gets the start of the run in which its block first reported. Written at the end of the file, every suite had the start of the last --rerun-each run. The headroom of a --parallel frame covers the key that each enclosing block adds to the fixed part of a TestDone frame. The docs say when two blocks with one name share a . Tests: every report is parsed as XML, the test of all orders checks the suite of each case, and two fixtures pin which blocks with one name join and which timestamp a block of an earlier run gets. --- docs/test/reporters.mdx | 2 +- src/runtime/cli/test/parallel/Frame.rs | 4 +- src/runtime/cli/test_command.rs | 25 ++++-- test/js/junit-reporter/junit.test.js | 120 +++++++++++++++++++++---- 4 files changed, 121 insertions(+), 30 deletions(-) diff --git a/docs/test/reporters.mdx b/docs/test/reporters.mdx index d046b3adf8ad..7b48a7c17746 100644 --- a/docs/test/reporters.mdx +++ b/docs/test/reporters.mdx @@ -85,7 +85,7 @@ junit = "path/to/junit.xml" # Output path for JUnit XML report #### Describe Blocks in JUnit Reports -By default, each `describe` block becomes a `` nested inside its file's ``. A block is one ``, in whatever order its tests end. A block with an empty title, or with no test in the report, has no ``. +By default, each `describe` block becomes a `` nested inside its file's ``. A block is one ``, in whatever order its tests end. Two blocks with the same name share one `` when nothing is between them in the report, for example the rows of a `describe.each` that have the same title. A block with an empty title, or with no test in the report, has no ``. Some tools only read the `` elements directly inside a file's ``. For those tools, use `--reporter-junit-suites=flat`: diff --git a/src/runtime/cli/test/parallel/Frame.rs b/src/runtime/cli/test/parallel/Frame.rs index f63f80345a4b..a441a4662493 100644 --- a/src/runtime/cli/test/parallel/Frame.rs +++ b/src/runtime/cli/test/parallel/Frame.rs @@ -99,10 +99,10 @@ impl Frame { // Never let a single frame exceed `MAX_PAYLOAD` — the receiver treats that // as a corrupt-channel signal and closes, which would surface as a spurious // worker crash. Truncate the string in place instead. Leave headroom - // so the fixed-size tail of the frame (u32s, short paths, and the + // so the fixed-size tail of the frame (u32s and u64s, short paths, and the // 4-byte prefixes of any further strings, which go empty) still fits. const TRUNC: &[u8] = b"\n... [output truncated: would exceed --parallel IPC frame limit]\n"; - const HEADROOM: usize = 4096; + const HEADROOM: usize = 8192; let used: usize = (self.buf.len() - 5) + 4; // current payload + str-len prefix let room: usize = ((MAX_PAYLOAD as usize) - HEADROOM).saturating_sub(used); let (keep, marker): (usize, &[u8]) = if s.len() <= room { diff --git a/src/runtime/cli/test_command.rs b/src/runtime/cli/test_command.rs index 319b44c89b51..717205168e12 100644 --- a/src/runtime/cli/test_command.rs +++ b/src/runtime/cli/test_command.rs @@ -2303,6 +2303,8 @@ struct DescribeBlocks { struct BlockSuite { name: Box<[u8]>, line: u32, + /// Of the run of the file in which the block first reported. + started_unix_ms: i64, parent: Option, /// 1 for a block at the top level of the file. depth: u32, @@ -2321,7 +2323,7 @@ enum Child { impl DescribeBlocks { /// The suite of the innermost named block around the test. `None` for a case of the file itself. - fn suite_of(&mut self, t: &TestCaseReport<'_>) -> Option { + fn suite_of(&mut self, t: &TestCaseReport<'_>, started_unix_ms: i64) -> Option { // Only the flat form includes describe blocks with an empty title, like Vitest. let named = |scope: &&ScopeReport<'_>| !scope.name.is_empty(); let innermost = t.scopes.iter().rfind(named)?; @@ -2345,15 +2347,16 @@ impl DescribeBlocks { first_without = index; } for scope in t.scopes[first_without..].iter().filter(named) { - // As in every release, a block shares the suite of a block with its name that reported just before it: - // the rows of a `describe.each` with one title, and the repeats of `--rerun-each`. + // As in every release, a block joins the suite before it in its parent when that suite has its name: + // the rows of a `describe.each` with one title, and the repeats of `--rerun-each`. So two suites with + // one name are never next to each other. let before = match self.children_of(suite).last() { Some(&Child::Suite(before)) => Some(before), _ => None, }; let inside = match before { Some(before) if strings::eql(&self.suites[before].name, scope.name) => before, - _ => self.add(suite, scope), + _ => self.add(suite, scope, started_unix_ms), }; self.by_key.insert(scope.block, inside); suite = Some(inside); @@ -2362,11 +2365,17 @@ impl DescribeBlocks { suite } - fn add(&mut self, parent: Option, scope: &ScopeReport<'_>) -> usize { + fn add( + &mut self, + parent: Option, + scope: &ScopeReport<'_>, + started_unix_ms: i64, + ) -> usize { let added = self.suites.len(); self.suites.push(BlockSuite { name: Box::from(scope.name), line: scope.line, + started_unix_ms, parent, depth: parent.map_or(1, |parent| self.suites[parent].depth + 1), metrics: Metrics::default(), @@ -2405,7 +2414,6 @@ impl DescribeBlocks { file: &[u8], file_metrics: &mut Metrics, hostname: &[u8], - started_unix_ms: i64, ) -> crate::Result<()> { if self.suites.is_empty() { return Ok(()); @@ -2449,7 +2457,7 @@ impl DescribeBlocks { &suite.metrics, suite.metrics.elapsed_time as f64 / bun::time::MS_PER_S as f64, hostname, - started_unix_ms, + suite.started_unix_ms, ); out.extend_from_slice(b">\n"); open.push((Some(suite), 0)); @@ -2881,7 +2889,6 @@ impl JunitReporter { &self.current_file, &mut suite.metrics, &hostname, - self.file_start_unix_ms, )?; let elapsed_ns = file_elapsed_ns.unwrap_or_else(|| { @@ -2947,7 +2954,7 @@ impl JunitReporter { class_name.extend_from_slice(scope.name); } - let suite = self.blocks.suite_of(t); + let suite = self.blocks.suite_of(t, self.file_start_unix_ms); if suite.is_none() && self.blocks.suites.is_empty() { // No block of the file has a suite yet, so this case comes before all of them. return CaseWriter { diff --git a/test/js/junit-reporter/junit.test.js b/test/js/junit-reporter/junit.test.js index 26915cb94008..5bf17172dfc0 100644 --- a/test/js/junit-reporter/junit.test.js +++ b/test/js/junit-reporter/junit.test.js @@ -1238,27 +1238,34 @@ describe("only block", () => { // Not concurrent: each test starts its `bun test` runs at once, and four tests at once make a run several times slower. describe("a describe block is one ", () => { - // Each test file says which process loaded it. - const sayPid = `require("node:fs").appendFileSync(process.env.PIDS, process.pid + "\\n");\n`; + // Each test file says which process loaded it, and when. + const sayLoaded = `require("node:fs").appendFileSync(process.env.LOADED, JSON.stringify({ file: import.meta.file, pid: process.pid, at: Date.now() }) + "\\n");\n`; async function report(files, args, env = bunEnv) { const fixtures = { "package.json": "{}" }; - for (const [name, source] of Object.entries(files)) fixtures[name] = source + sayPid; + for (const [name, source] of Object.entries(files)) fixtures[name] = source + sayLoaded; using dir = tempDir("junit-blocks", fixtures); - const pids = join(String(dir), "pids"); + const loadedPath = join(String(dir), "loaded"); await using proc = spawn([bunExe(), "test", "--reporter=junit", "--reporter-outfile=junit.xml", ...args], { cwd: String(dir), - env: { ...env, PIDS: pids }, + env: { ...env, LOADED: loadedPath }, stdout: "ignore", stderr: "ignore", }); const exitCode = await proc.exited; - const loadedIn = (await file(pids).text()).trim().split("\n").map(Number); + const loaded = (await file(loadedPath).text()).trim().split("\n").map(JSON.parse); + const raw = await file(join(String(dir), "junit.xml")).text(); + // Rejects for a report that is not well-formed. + await new Promise((resolve, reject) => { + xml2js.parseString(raw, { strict: true }, (err, r) => (err ? reject(err) : resolve(r))); + }); return { - xml: filterJunitXmlOutput(await file(join(String(dir), "junit.xml")).text()), + raw, + xml: filterJunitXmlOutput(raw), exitCode, + loaded, // With one test file, `bun test --parallel` loads the file itself and starts no worker. - usedWorkers: !loadedIn.includes(proc.pid), + usedWorkers: !loaded.some(({ pid }) => pid === proc.pid), }; } @@ -1370,6 +1377,22 @@ describe.each([0, 1])("row", row => { describe("third", () => { test.concurrent("in third", end("in third")); }); +`, + // "y" is between the two "x" in the report, although the case before the second "x" is in the first. + "adjacent.test.js": `import { describe, test } from "bun:test"; +${turns} +${endInOrder} +const end = endInOrder(["x1", "y1", "x2", "again"]); +describe("x", () => { + test.concurrent("x1", end("x1")); + test.concurrent("x2", end("x2")); +}); +describe("y", () => { + test.concurrent("y1", end("y1")); +}); +describe("x", () => { + test.concurrent("again", end("again")); +}); `, "same-name.test.js": `import { describe, test } from "bun:test"; describe.each([1, 2])("each", row => { @@ -1401,6 +1424,13 @@ helper("second call"); // The flat form has the cases in the order reported. expect(outline(flat.xml)).toEqual({ + "adjacent.test.js": [ + "suite adjacent.test.js tests=4 failures=0", + " case x > x1", + " case y > y1", + " case x > x2", + " case x > again", + ], "concurrent.test.js": [ "suite concurrent.test.js tests=3 failures=0", " case a > a1", @@ -1447,8 +1477,18 @@ helper("second call"); ], }); // An error outside of a test stays a case of the file, where it was reported among the file's blocks and cases. - // Two blocks with one name share a when one reports just after the other, as before. + // Two blocks with one name share a when nothing is between them in the report, as before. expect(outline(serial.xml)).toEqual({ + "adjacent.test.js": [ + "suite adjacent.test.js tests=4 failures=0", + " suite x line=16 tests=2 failures=0", + " case x1", + " case x2", + " suite y line=20 tests=1 failures=0", + " case y1", + " suite x line=23 tests=1 failures=0", + " case again", + ], "concurrent.test.js": [ "suite concurrent.test.js tests=3 failures=0", " suite a line=16 tests=2 failures=0", @@ -1509,7 +1549,7 @@ helper("second call"); }); expect(parallel.xml).toBe(serial.xml); expect(totals(serial.xml)).toEqual(totals(flat.xml)); - expect(totals(flat.xml)).toHaveLength(7); + expect(totals(flat.xml)).toHaveLength(8); expect([serial.usedWorkers, parallel.usedWorkers]).toEqual([false, true]); expect([serial.exitCode, parallel.exitCode, flat.exitCode]).toEqual([1, 1, 1]); }); @@ -1551,24 +1591,33 @@ for (const order of ${JSON.stringify(orders)}) { order.map(name => ` case ${order.join(" ")} > ${name === "i1" ? "a > inner" : name[0]} > ${name}`), ), ); - // Each with the names of those around it. Which block reports first decides the order of two blocks. + // Each and with the names of the suites around it. Which block reports first decides + // the order of two blocks. const around = []; - const suites = []; + const elements = []; for (const line of outline(nested.xml)["orders.test.js"].slice(1)) { - const [, indent, name, tests] = line.match(/^( *)suite (.*) line=\d+ (tests=\d+) /) ?? []; - if (name === undefined) continue; + const [, indent, element, name, tests] = line.match(/^( *)(suite|case) (.*?)(?: line=\d+ (tests=\d+) .*)?$/); around.length = indent.length / 2 - 1; - around.push(name); - suites.push(`${around.join(" > ")} ${tests}`); + if (element === "suite") around.push(name); + elements.push(element === "suite" ? `${around.join(" > ")} ${tests}` : `${around.join(" > ")} has ${name}`); } const once = orders .flatMap(order => { const block = order.join(" "); - return [`${block} tests=4`, `${block} > a tests=3`, `${block} > a > inner tests=1`, `${block} > b tests=1`]; + return [ + `${block} tests=4`, + `${block} > a tests=3`, + `${block} > a has a1`, + `${block} > a has a2`, + `${block} > a > inner tests=1`, + `${block} > a > inner has i1`, + `${block} > b tests=1`, + `${block} > b has b1`, + ]; }) .sort(); expect(orders).toHaveLength(24); - expect(suites.sort()).toEqual(once); + expect(elements.sort()).toEqual(once); expect([nested.exitCode, flat.exitCode]).toEqual([0, 0]); }); @@ -1592,6 +1641,18 @@ for (let block = 0; block < 50; block++) { test("in block " + block, () => {}); }); } +`, + "stamps.test.js": `import { describe, test } from "bun:test"; +describe("first", () => { + // The next run starts in a later millisecond than this one was loaded in. + test("until the clock moves", () => { + const now = Date.now(); + while (Date.now() === now); + }); +}); +describe("second", () => { + test("b", () => {}); +}); `, "wrapped.test.js": `import { describe, test } from "bun:test"; describe("outer", () => { @@ -1631,6 +1692,17 @@ describe("outer", () => { " case b1", ], "many.test.js": ["suite many.test.js tests=100 failures=0", ...many, ...many], + "stamps.test.js": [ + "suite stamps.test.js tests=4 failures=0", + " suite first line=2 tests=1 failures=0", + " case until the clock moves", + " suite second line=9 tests=1 failures=0", + " case b", + " suite first line=2 tests=1 failures=0", + " case until the clock moves", + " suite second line=9 tests=1 failures=0", + " case b", + ], "wrapped.test.js": [ "suite wrapped.test.js tests=4 failures=0", " suite outer line=2 tests=4 failures=0", @@ -1645,6 +1717,18 @@ describe("outer", () => { ], }); expect(parallel.xml).toBe(serial.xml); + // The timestamp of a block is the start of the run in which it reported. + const loads = serial.loaded.filter(({ file }) => file === "stamps.test.js").map(({ at }) => at); + const stamps = Array.from( + serial.raw.matchAll(/]* timestamp="([^"]*)">/g), + ([, name, timestamp]) => [name, loads.findIndex(loaded => Date.parse(timestamp) <= loaded)], + ); + expect(stamps).toEqual([ + ["first", 0], + ["second", 0], + ["first", 1], + ["second", 1], + ]); expect([serial.usedWorkers, parallel.usedWorkers]).toEqual([false, true]); expect([serial.exitCode, parallel.exitCode]).toEqual([0, 0]); }); From ea1128c5025168a533cb1739ff8c7a7744d9b631 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 7 Nov 2026 13:53:25 +0000 Subject: [PATCH 3/3] junit reporter: shorten the comments --- src/runtime/cli/test/parallel/Frame.rs | 4 +--- src/runtime/cli/test_command.rs | 22 +++++----------------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/src/runtime/cli/test/parallel/Frame.rs b/src/runtime/cli/test/parallel/Frame.rs index a441a4662493..ce8af2ed57b8 100644 --- a/src/runtime/cli/test/parallel/Frame.rs +++ b/src/runtime/cli/test/parallel/Frame.rs @@ -14,9 +14,7 @@ pub(crate) enum Kind { /// then, only when the worker runs with `--reporter`, the structured /// result (`runner::encode_test_case`): u32 status, u32 assertions, /// u64 elapsed_ns, u32 line, str name, u32 n × {str scope_name, - /// u32 scope_line, u32 scope_run, u64 scope_address}, u32 has_failure, - /// u32 earlier_failures, then for each earlier failure and for the last - /// one if has_failure: str name, str message, str body, u32 uncaught + /// u32 scope_line, u32 scope_run, u64 scope_address}, u32 has_failure, u32 earlier_failures, (earlier_failures + has_failure) × {str name, str message, str body, u32 uncaught} TestDone, /// 10 × u32: file_idx, pass, fail, skip, todo, expectations, skipped_label, /// files, shuffled, unhandled; u64 elapsed_ns; 4 × u32, 0 where nobody measures: the megabytes by which the worker grew at most, those of its largest child, diff --git a/src/runtime/cli/test_command.rs b/src/runtime/cli/test_command.rs index 717205168e12..eff673ec82cc 100644 --- a/src/runtime/cli/test_command.rs +++ b/src/runtime/cli/test_command.rs @@ -2133,9 +2133,7 @@ pub(crate) fn junit_file_name(path: &[u8]) -> &[u8] { } } -/// Names one `describe` block of one run of a test file. Its name and line do -/// not: the rows of a `describe.each` and the calls of a function that calls -/// `describe` have the same. +/// Names one `describe` block of one run of a test file. Its name and line do not: the rows of a `describe.each` share them. #[derive(Clone, Copy, PartialEq, Eq, Hash)] pub(crate) struct BlockKey { /// `BunTestRoot::file_generation`. Each `--rerun-each` repeat builds its blocks again, and can get the addresses of the repeat before. @@ -2152,7 +2150,6 @@ impl BlockKey { } } - /// What a `--parallel` worker sends on its `TestDone` frame. pub(crate) fn to_wire(self) -> (u32, u64) { (self.run, self.address) } @@ -2284,10 +2281,7 @@ impl Metrics { } } -/// The `describe` blocks of the file whose `` is open. A block's -/// `` is written when the file ends, because the cases of one block -/// don't arrive together: concurrent tests of two blocks end in any order, and -/// an error outside of a test is a case of the file. +/// The `describe` blocks of the open file, written when it ends because the cases of one block don't arrive together. #[derive(Default)] struct DescribeBlocks { suites: Vec, @@ -2324,7 +2318,6 @@ enum Child { impl DescribeBlocks { /// The suite of the innermost named block around the test. `None` for a case of the file itself. fn suite_of(&mut self, t: &TestCaseReport<'_>, started_unix_ms: i64) -> Option { - // Only the flat form includes describe blocks with an empty title, like Vitest. let named = |scope: &&ScopeReport<'_>| !scope.name.is_empty(); let innermost = t.scopes.iter().rfind(named)?; if let Some((block, suite)) = self.last @@ -2347,9 +2340,7 @@ impl DescribeBlocks { first_without = index; } for scope in t.scopes[first_without..].iter().filter(named) { - // As in every release, a block joins the suite before it in its parent when that suite has its name: - // the rows of a `describe.each` with one title, and the repeats of `--rerun-each`. So two suites with - // one name are never next to each other. + // As in every release, a block joins its parent's last suite when that suite has its name, like the rows of a `describe.each`. let before = match self.children_of(suite).last() { Some(&Child::Suite(before)) => Some(before), _ => None, @@ -2405,8 +2396,7 @@ impl DescribeBlocks { } } - /// Writes the `` of each block around what it holds, adds the - /// counts of the blocks to those of the file, and forgets the file. + /// Writes each block's `` around what it holds, adds the blocks' counts to the file's, and forgets the file. #[optimize(size)] fn write( &mut self, @@ -2811,7 +2801,6 @@ impl JunitReporter { hostname: &[u8], started_unix_ms: i64, ) { - // Std::io::Write removed; bun_io::Write (top-level) provides write_fmt. let _ = write!( out, "tests=\"{}\" assertions=\"{}\" failures=\"{}\" skipped=\"{}\" time=\"{}\" hostname=\"{}\" timestamp=\"", @@ -2919,8 +2908,7 @@ impl JunitReporter { Ok(()) } - /// Writes the `` into the suite of `t.file`. In the nested form - /// a test inside a named `describe` block goes into the suite of that block. + /// Writes the `` into the suite of `t.file`, or in the nested form into that of its innermost named `describe` block. #[optimize(size)] pub(crate) fn record_test_case(&mut self, t: &TestCaseReport<'_>) -> crate::Result<()> { if self.file_suite.is_none() || !strings::eql(&self.current_file, t.file) {