Skip to content
Open
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
88 changes: 50 additions & 38 deletions src/runtime/cli/test/Scanner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use std::rc::Rc;
use bun_alloc::AllocError;
use bun_bundler::Transpiler;
use bun_bundler::options::BundleOptions;
use bun_collections::index_sort;
use bun_core::{StringOrTinyString, strings};
use bun_collections::{HashMap, index_sort};
use bun_core::{StringOrTinyString, UnwrapOrOom, strings};
use bun_output::{declare_scope, scoped_log};
use bun_paths::resolve_path::{join_abs_string_buf_checked, platform};
use bun_paths::{self, PathBuffer};
Expand All @@ -29,11 +29,14 @@ pub(crate) struct Scanner<'a> {
pub(crate) dirs_to_scan: Fifo,
/// Paths to test files found while scanning.
pub(crate) test_files: Vec<Interned>,
/// Every path in `test_files`, so a path two arguments select is listed once.
seen_test_files: HashMap<&'static [u8], ()>,
pub(crate) fs: *mut FileSystem,
pub(crate) open_dir_buf: PathBuffer,
pub(crate) options: &'a BundleOptions<'a>,
pub(crate) has_iterated: bool,
pub(crate) search_count: usize,
/// Set by `next`. False after `read_dir_with_name` means the listing came from the cache.
iterator_invoked: bool,
/// The directory being iterated; its fd closes once every child `ScanEntry` has been opened.
current_dir: Option<Rc<Dir>>,
}
Expand Down Expand Up @@ -80,9 +83,10 @@ impl<'a> Scanner<'a> {
options: &transpiler.options,
fs: transpiler.resolver.fs(),
test_files: results,
seen_test_files: HashMap::new(),
open_dir_buf: PathBuffer::ZEROED,
has_iterated: false,
search_count: 0,
iterator_invoked: false,
current_dir: None,
})
}
Expand Down Expand Up @@ -118,6 +122,19 @@ impl<'a> Scanner<'a> {
Ok(core::mem::take(&mut self.test_files).into_boxed_slice())
}

/// Append `path` to `test_files` unless an earlier scan already found it.
fn push_test_file(&mut self, path: Interned) -> Result<(), AllocError> {
if self
.seen_test_files
.get_or_put(path.as_bytes())?
Comment thread
robobun marked this conversation as resolved.
Resolved
.found_existing
{
return Ok(());
}
self.test_files.push(path);
Ok(())
}

pub(crate) fn scan(&mut self, path_literal: &[u8]) -> Result<(), ScanError> {
let mut scan_dir_buf = bun_paths::path_buffer_pool::get();
let parts: [&[u8]; 2] = [self.top_level_dir(), path_literal];
Expand All @@ -139,7 +156,7 @@ impl<'a> Scanner<'a> {
.append_slice(path)
.map_err(|_| ScanError::OutOfMemory)?;
let rel_path = Interned::from_static(stored);
self.test_files.push(rel_path);
self.push_test_file(rel_path)?;
}
} else if e == bun_resolver::Error::Sys(bun_errno::SystemErrno::ENOENT) {
return Err(ScanError::DoesNotExist);
Expand All @@ -153,34 +170,6 @@ impl<'a> Scanner<'a> {
}
}

// you typed "." and we already scanned it
if !self.has_iterated {
if let EntriesOption::Entries(entries) = root {
// Collect first so `self.next(…)` doesn't overlap the
// `entries.data` borrow.
// this branch is taken when the resolver already has
// `path` cached (e.g. `run_env_loader`/`read_dir_info` read the
// cwd before the scanner runs), so `read_directory_with_iterator`
// returned the cached `EntryMap` without invoking `iterator.next`.
// Hash-map iteration order is not stable. Sort by (lowercased)
// base name so test-file discovery order is deterministic —
// regression/issue/26851 relies on `a_*.test` running before
// `b_*.test` under `--bail`.
let mut entry_ptrs: Vec<*mut fs::Entry> = entries.data.values().copied().collect();
index_sort::sort_slice_by(&mut entry_ptrs, |a, b| {
// SAFETY: `EntryMap` stores `*mut Entry` into the
// process-static `EntryStore`; valid for `'static`.
let (an, bn) = unsafe { ((**a).base_lowercase(), (**b).base_lowercase()) };
an.cmp(bn)
});
for entry_ptr in entry_ptrs {
// SAFETY: `EntryMap` stores `*mut Entry` into the
// process-static `EntryStore`; valid for `'static`.
self.next(&self.fs().fs, unsafe { &*entry_ptr });
}
}
}

while let Some(entry) = self.dirs_to_scan.pop_front() {
let parts2: [&[u8]; 2] = [entry.dir_path, entry.name.slice()];
let Some(path2) = self.fs().abs_buf_checked(&parts2, &mut scan_dir_buf) else {
Expand Down Expand Up @@ -214,17 +203,40 @@ impl<'a> Scanner<'a> {
Ok(())
}

/// A cached listing is returned without invoking the iterator. Sorted so the run order is stable.
fn walk_cached_entries(&mut self, listing: &EntriesOption) {
let EntriesOption::Entries(entries) = listing else {
return;
};
let mut entry_ptrs: Vec<*mut fs::Entry> = entries.data.values().copied().collect();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
index_sort::sort_slice_by(&mut entry_ptrs, |a, b| {
// SAFETY: `EntryMap` stores `*mut Entry` into the
// process-static `EntryStore`; valid for `'static`.
let (an, bn) = unsafe { ((**a).base_lowercase(), (**b).base_lowercase()) };
an.cmp(bn)
});
for entry_ptr in entry_ptrs {
// SAFETY: `EntryMap` stores `*mut Entry` into the
// process-static `EntryStore`; valid for `'static`.
self.next(&self.fs().fs, unsafe { &*entry_ptr });
}
}

/// `handle` stays owned by the caller; the resolver caches the listing but not the fd.
fn read_dir_with_name(
&mut self,
name: &[u8],
handle: Option<Fd>,
) -> crate::Result<&'static mut EntriesOption> {
let fs_ptr = self.fs;
self.iterator_invoked = false;
// SAFETY: borrows only the `fs` field; re-entrant access is serialised by `RealFS.entries_mutex`.
unsafe { &mut (*fs_ptr).fs }
.read_directory_with_iterator(name, handle, 0, false, self)
.map_err(Into::into)
let listing = unsafe { &mut (*fs_ptr).fs }
.read_directory_with_iterator(name, handle, 0, false, self)?;
if !self.iterator_invoked {
self.walk_cached_entries(listing);
}
Ok(listing)
}

pub(crate) fn could_be_test_file<const NEEDS_TEST_SUFFIX: bool>(&self, name: &[u8]) -> bool {
Expand Down Expand Up @@ -321,7 +333,7 @@ impl<'a> Scanner<'a> {

fn next<R: EntryKindResolver>(&mut self, rfs: &R, entry: &fs::Entry) {
let name = entry.base_lowercase();
self.has_iterated = true;
self.iterator_invoked = true;
match entry.kind(rfs, false) {
fs::EntryKind::Dir => {
if (!name.is_empty() && name[0] == b'.') || name == b"node_modules" {
Expand Down Expand Up @@ -403,7 +415,7 @@ impl<'a> Scanner<'a> {
Err(_) => bun_core::out_of_memory(),
};
let abs_path = entry.abs_path_or_fill(|| Interned::from_static(stored));
self.test_files.push(abs_path);
self.push_test_file(abs_path).unwrap_or_oom();
}
fs::EntryKind::Dangling => {}
}
Expand Down
45 changes: 45 additions & 0 deletions test/cli/test/bun-test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2061,6 +2061,51 @@ describe.concurrent("test file discovery (scanner)", () => {
expect(exitCode).toBe(0);
});

// Two path arguments can select the same file: a directory and a file
// inside it, or a directory and ".". With --isolate or --parallel each entry
// in the run list is a separate load, so the file runs once only if the
// scanner lists it once. In the default mode a repeat only prints the file
// header twice and counts two files. The cwd and its ancestors are listed
// by the resolver before the scanner runs, so those arguments are served
// from the cache and must still be walked.
describe.each([
["directory then file", ["./sub", "./sub/b.test.ts"], "Ran 2 tests across 2 files."],
["file then directory", ["./sub/b.test.ts", "./sub"], "Ran 2 tests across 2 files."],
["the same file twice", ["./sub/b.test.ts", "./sub/b.test.ts"], "Ran 1 test across 1 file."],
["subdirectory then cwd", ["./sub", "."], "Ran 3 tests across 3 files."],
["cwd then subdirectory", [".", "./sub"], "Ran 3 tests across 3 files."],
["parent of the cwd", ["../"], "Ran 4 tests across 4 files."],
])("a file selected by two path arguments runs once (%s)", (_name, args, summary) => {
test.each([
["default", []],
["--isolate", ["--isolate"]],
["--parallel", ["--parallel"]],
])("%s", async (_mode, flags) => {
using dir = tempDir("scanner-duplicate-args", {
"outer.test.ts": `import { test } from "bun:test"; test("outer", () => { console.log("RAN outer"); });`,
"cwd/top.test.ts": `import { test } from "bun:test"; test("top", () => { console.log("RAN top"); });`,
"cwd/sub/a.test.ts": `import { test } from "bun:test"; test("a", () => { console.log("RAN a"); });`,
"cwd/sub/b.test.ts": `import { test } from "bun:test"; test("b", () => { console.log("RAN b"); });`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "test", ...flags, ...args],
env: bunEnv,
cwd: join(String(dir), "cwd"),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

// --parallel forwards the console output of a worker through stderr.
const out = stdout + stderr;
expect(out.split("RAN b").length - 1).toBe(1);
expect(out.split(`sub${sep}b.test.ts:`).length - 1).toBe(1);
expect(stderr).toContain(summary);
expect(exitCode).toBe(0);
});
});

// The scanner builds every absolute path in a PathBuffer of MAX_PATH_BYTES:
// 4096 on Linux, 1024 on every other POSIX (src/bun_core/util.rs). On Windows
// it is 32767*3+1 bytes, more than a command line or an NT path can hold, so
Expand Down
Loading