Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 35 additions & 43 deletions src/runtime/cli/test/parallel/Coordinator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ use bun_ptr::Interned;

use super::frame::{self, Frame};
use super::worker::{Worker, WorkerPipe};
use crate::test_command::{CommandLineReporter, FileFailureRecord, TestFailure};
use crate::test_command::{CommandLineReporter, FailureSite, FileFailureRecord, TestFailure};
use crate::test_runner::jest::FileFailure;

// `Status` lives in `crate::api::bun::process`
// (not the lower-tier `bun_spawn` crate). Worker.exit_status is this type.
Expand Down Expand Up @@ -192,10 +193,7 @@ impl<'a> Coordinator<'a> {
.filter_map(|w| w.inflight.map(|idx| (idx, now - w.dispatched_at)))
.collect();
for (idx, ms) in &running {
self.reporter.summary().fail += 1;
self.reporter.summary().files += 1;
self.mark_crashed(*idx, *ms);
self.files_done += 1;
self.fail_crashed_file(*idx, *ms);
}
if !running.is_empty() {
bun_core::pretty_errorln!("<r>\n<red>Interrupted<r> while still running:");
Expand Down Expand Up @@ -487,26 +485,19 @@ impl<'a> Coordinator<'a> {
return;
}

// Reshaped for borrowck — `summary()` mutably borrows
// `self.reporter`, so the unhandled-errors counter (also on
// `self.reporter.jest`) and `bail_out()` must run after the
// summary borrow is released.
{
let summary = self.reporter.summary();
summary.pass += pass;
summary.fail += fail;
summary.skip += skip;
summary.todo += todo;
summary.expectations += expectations;
summary.skipped_because_label += skipped_label;
summary.files += files;
}
self.reporter.jest.unhandled_errors_between_tests += unhandled;
let summary = self.reporter.summary();
summary.pass += pass;
summary.merge_worker_counts(fail, unhandled);
summary.skip += skip;
summary.todo += todo;
summary.expectations += expectations;
summary.skipped_because_label += skipped_label;
summary.files += files;
self.record_timing(idx, w.dispatched_at);

w.inflight = None;
self.files_done += 1;
let fail_now = self.reporter.summary().fail;
let fail_now = self.reporter.summary().fail();
if self.bail > 0 && fail_now >= self.bail {
self.bail_out();
}
Expand Down Expand Up @@ -735,34 +726,34 @@ impl<'a> Coordinator<'a> {
bstr::BStr::new(self.rel_path(file_idx)),
bstr::BStr::new(reason),
);
self.reporter.summary().fail += 1;
self.reporter.summary().files += 1;
self.files_done += 1;
self.record_file_failure(file_idx, b"(aborted)", reason);
self.fail_file(file_idx, FileFailure::Aborted, reason);
}

fn mark_crashed(&mut self, file_idx: u32, elapsed_ms: i64) {
fn fail_crashed_file(&mut self, file_idx: u32, elapsed_ms: i64) {
if let Some(file) = self.test_records.get_mut(file_idx as usize) {
file.elapsed_ns = u64::try_from(elapsed_ms).unwrap_or(0) * bun_core::time::NS_PER_MS;
}
self.record_file_failure(
self.fail_file(
file_idx,
b"(worker crashed)",
FileFailure::WorkerCrashed,
b"worker process crashed before reporting results",
);
}

/// The failed testcase a structured reporter gets for a file that the coordinator fails itself.
fn record_file_failure(&mut self, file_idx: u32, name: &'static [u8], message: &[u8]) {
if let Some(file) = self.test_records.get_mut(file_idx as usize) {
file.failure = Some(Box::new(FileFailureRecord {
name,
failure: Some(TestFailure {
/// Counts a file that the coordinator fails itself. `message` goes into its record.
fn fail_file(&mut self, file_idx: u32, kind: FileFailure, message: &[u8]) {
self.reporter.fail_file(
kind,
FailureSite::Coordinator(self.test_records.get_mut(file_idx as usize)),
|| {
Some(TestFailure {
message: message.to_vec(),
..Default::default()
}),
}));
}
})
},
);
self.reporter.summary().files += 1;
self.files_done += 1;
}

fn account_crash(
Expand All @@ -777,11 +768,8 @@ impl<'a> Coordinator<'a> {
bstr::BStr::new(self.rel_path(file_idx)),
detail,
);
self.reporter.summary().fail += 1;
self.reporter.summary().files += 1;
self.mark_crashed(file_idx, bun_core::time::milli_timestamp() - dispatched_at);
self.files_done += 1;
if self.bail > 0 && self.reporter.summary().fail >= self.bail {
self.fail_crashed_file(file_idx, bun_core::time::milli_timestamp() - dispatched_at);
if self.bail > 0 && self.reporter.summary().fail() >= self.bail {
self.bail_out();
}
}
Expand Down Expand Up @@ -898,7 +886,11 @@ impl<'a> Coordinator<'a> {
)),
bstr::BStr::new(reason),
);
self.reporter.summary().fail += 1;
self.reporter.fail_file(
FileFailure::NotDispatched,
FailureSite::Coordinator(self.test_records.get_mut(idx as usize)),
|| None,
);
self.reporter.summary().files += 1;
self.files_done += 1;
}
Expand Down
5 changes: 2 additions & 3 deletions src/runtime/cli/test/parallel/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,6 @@ impl<'a> WorkerLoop<'a> {
self.cmds.send(wf.finish());

let before = *self.reporter.summary();
let before_unhandled = self.reporter.jest.unhandled_errors_between_tests;
let started_ns =
bun_core::Timespec::now(bun_core::TimespecMockMode::ForceRealTime).ns();

Expand Down Expand Up @@ -611,13 +610,13 @@ impl<'a> WorkerLoop<'a> {
for v in [
idx,
after.pass - before.pass,
after.fail - before.fail,
after.fail() - before.fail(),
after.skip - before.skip,
after.todo - before.todo,
after.expectations - before.expectations,
after.skipped_because_label - before.skipped_because_label,
after.files - before.files,
self.reporter.jest.unhandled_errors_between_tests - before_unhandled,
after.unhandled_errors() - before.unhandled_errors(),
] {
wf.u32(v);
}
Expand Down
84 changes: 57 additions & 27 deletions src/runtime/cli/test_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use bun_io::Write as _;

use crate::cli::Command;
use crate::cli::test::changed_files_filter as ChangedFilesFilter;
use crate::cli::test::parallel::coordinator::FileTestRecords;
use crate::cli::test::parallel_runner as ParallelRunner;
use crate::cli::test::scanner::{self, Scanner};
use crate::cli::test::timings::Timings;
Expand Down Expand Up @@ -59,7 +60,7 @@ use coverage::{ByteRangeMapping, CodeCoverageReport, Fraction};
// `crate::test_runner::*`; the façade below adapts the body's nested-path
// usage (`bun_test::Execution::Result`, `bun_test::BasicResult`, …) without a
// 2k-line body rewrite.
use crate::test_runner::jest::{self, FileColumns as _, Summary, TestRunner};
use crate::test_runner::jest::{self, FileColumns as _, FileFailure, Summary, TestRunner};
use crate::test_runner::snapshot::Snapshots;
use bun_collections::index_sort;

Expand Down Expand Up @@ -208,14 +209,16 @@ pub(crate) fn describe_chain<'a>(child: &'a bun_test::BaseScope) -> Vec<(&'a [u8
scopes
}

/// Where the testcase of a failure that is not a finished test goes. Each variant starts with the file, which this process runs.
/// Where the testcase of a failure that is not a finished test goes. The first three variants start with the file, which this process runs.
pub(crate) enum FailureSite<'a> {
/// Under no `describe` block.
File(&'a [u8]),
/// In the suite of the `describe` block whose callback failed, at its line.
Describe(&'a [u8], &'a bun_test::DescribeScope),
/// Beside the one test that runs now, which keeps the suite of its `describe` block in one piece.
BesideTest(&'a [u8], &'a bun_test::ExecutionEntry),
/// A file of the `--parallel` coordinator: the records of its worker, `None` without a structured reporter.
Coordinator(Option<&'a mut FileTestRecords>),
}

/// The testcase the `--parallel` coordinator records for a file itself.
Expand All @@ -224,6 +227,20 @@ pub(crate) struct FileFailureRecord {
pub failure: Option<TestFailure>,
}

impl FileFailure {
/// `None`: the failure is counted and gets no testcase.
fn testcase_name(self) -> Option<&'static [u8]> {
match self {
FileFailure::Load { .. } => Some(b"(load error)"),
FileFailure::DescribeCallback => Some(b"(describe callback)"),
FileFailure::Unhandled => Some(b"(unhandled error)"),
FileFailure::WorkerCrashed => Some(b"(worker crashed)"),
FileFailure::Aborted => Some(b"(aborted)"),
FileFailure::NotDispatched => None,
}
}
}

/// 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]) {
Expand Down Expand Up @@ -1284,18 +1301,33 @@ impl CommandLineReporter {
&mut self.jest.summary
}

/// Records a failure that is not a finished test as a failed testcase, when a structured reporter is on.
#[cold]
/// The name of the testcase `fail_file` records for `kind` at `site`, `None` when it records nothing.
pub(crate) fn file_failure_testcase(
&self,
kind: FileFailure,
site: &FailureSite<'_>,
) -> Option<&'static [u8]> {
let recorded = match site {
FailureSite::Coordinator(records) => records.is_some(),
_ => self.jest.test_options.reporters.junit,
};
if recorded { kind.testcase_name() } else { None }
}

/// The record half of `fail_file`, which counts the failure first.
pub(crate) fn record_file_failure(
&mut self,
name: &'static [u8],
site: FailureSite<'_>,
failure: Option<TestFailure>,
) {
if !self.jest.test_options.reporters.junit {
return;
}
let (file, scopes, line_number) = match site {
FailureSite::Coordinator(records) => {
if let Some(records) = records {
records.failure = Some(Box::new(FileFailureRecord { name, failure }));
}
return;
}
FailureSite::File(file) => (file, Vec::new(), 0),
FailureSite::Describe(file, scope) => {
let mut scopes = describe_chain(&scope.base);
Expand Down Expand Up @@ -1466,9 +1498,9 @@ impl CommandLineReporter {
| R::FailBecauseTimeoutWithDoneCallback
| R::FailBecauseHookTimeout
| R::FailBecauseHookTimeoutWithDoneCallback => {
this.summary().fail += 1;
this.summary().count_failed_test();

if this.summary().fail == this.jest.bail {
if this.summary().fail() == this.jest.bail {
this.print_summary();
pretty_error!(
"\nBailed out after {} failure{}<r>\n",
Expand All @@ -1490,7 +1522,7 @@ impl CommandLineReporter {

pub(crate) fn print_summary(&mut self) {
let summary_ = self.summary();
let tests = summary_.fail + summary_.pass + summary_.skip + summary_.todo;
let tests = summary_.fail() + summary_.pass + summary_.skip + summary_.todo;
let files = summary_.files;

pretty_error!(
Expand Down Expand Up @@ -1934,7 +1966,6 @@ impl TestCommand {
// SAFETY: lifetime-erase to `'static`; `ctx` is the
// process-lifetime CLI context and `exec()` never returns.
test_options: unsafe { bun_ptr::detach_lifetime_ref(&ctx.test_options) },
unhandled_errors_between_tests: 0,
summary: Summary::default(),
node_test_used: false,
},
Expand Down Expand Up @@ -2479,12 +2510,12 @@ impl TestCommand {
let _ = error_writer.write_all(&reporter.todos_to_repeat_buf);
}

if reporter.summary().fail > 0 {
if reporter.summary().fail() > 0 {
if reporter.summary().skip > 0 || reporter.summary().todo > 0 {
pretty_error!("\n");
}

pretty_error!("\n<r><d>{} tests failed:<r>\n", reporter.summary().fail);
pretty_error!("\n<r><d>{} tests failed:<r>\n", reporter.summary().fail());
Output::flush();

let error_writer = Output::error_writer();
Expand Down Expand Up @@ -2576,8 +2607,8 @@ impl TestCommand {
// `reporter.summary()` doesn't span the whole printing block and
// conflict with the `reporter.jest.*` reads below.
let summary: Summary = *reporter.summary();
let did_label_filter_out_all_tests = summary.did_label_filter_out_all_tests()
&& reporter.jest.unhandled_errors_between_tests == 0;
let did_label_filter_out_all_tests =
summary.did_label_filter_out_all_tests() && summary.unhandled_errors() == 0;

if !did_label_filter_out_all_tests {
struct DotIndenter {
Expand Down Expand Up @@ -2625,19 +2656,19 @@ impl TestCommand {
pretty_error!("{}<r><magenta>{:5>} todo<r>\n", &indenter, summary.todo);
}

if summary.fail > 0 {
if summary.fail() > 0 {
pretty_error!("<r><red>");
} else {
pretty_error!("<r><d>");
}

pretty_error!("{}{:5>} fail<r>\n", &indenter, summary.fail);
if reporter.jest.unhandled_errors_between_tests > 0 {
pretty_error!("{}{:5>} fail<r>\n", &indenter, summary.fail());
if summary.unhandled_errors() > 0 {
pretty_error!(
"{}<r><red>{:5>} error{}<r>\n",
&indenter,
reporter.jest.unhandled_errors_between_tests,
if reporter.jest.unhandled_errors_between_tests > 1 {
summary.unhandled_errors(),
if summary.unhandled_errors() > 1 {
"s"
} else {
""
Expand Down Expand Up @@ -2731,12 +2762,12 @@ impl TestCommand {
let should_fail_on_no_tests = !ctx.test_options.pass_with_no_tests
&& (failed_to_find_any_tests || summary.did_label_filter_out_all_tests());
if should_fail_on_no_tests
|| summary.fail > 0
|| summary.fail() > 0
|| (coverage_options.enabled
&& coverage_options.fractions.failing
&& coverage_options.fail_on_low_coverage)
|| !write_snapshots_success
|| reporter.jest.unhandled_errors_between_tests > 0
|| summary.unhandled_errors() > 0
{
vm.exit_handler.exit_code = 1;
}
Expand Down Expand Up @@ -3002,19 +3033,18 @@ impl TestCommand {
let (result, promise_js) = (p.result(global.vm()), p.to_js());
reporter.load_failure_pending = true;
vm.unhandled_rejection(global, result, promise_js);
reporter.summary().fail += 1;
if core::mem::take(&mut reporter.load_failure_pending) {
reporter.record_file_failure(
b"(load error)",
reporter.fail_file(
FileFailure::Load { reported: false },
FailureSite::File(file_path),
None,
|| None,
);
}
if let Some(junit) = reporter.reporters.junit.as_mut() {
let _ = junit.end_file(None);
}

if reporter.jest.bail == reporter.summary().fail {
if reporter.jest.bail == reporter.summary().fail() {
reporter.print_summary();
pretty_error!(
"\nBailed out after {} failure{}<r>\n",
Expand Down
Loading
Loading