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
14 changes: 13 additions & 1 deletion src/resolver/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ pub mod dir_entry {
}
}

/// Per-entry hook invoked by `add_entry`/`readdir`.
/// Called under `entries_mutex` for each entry a read returns, from disk or (sorted) from the cache.
pub trait DirEntryIterator {
const IS_VOID: bool = false;
fn next(&self, entry: &mut Entry, fd: Fd);
Expand Down Expand Up @@ -622,6 +622,18 @@ impl DirEntry {
})
}

/// Every entry, sorted by lowercased base name (`data` iterates in hash order).
pub(crate) fn sorted_entries(&self) -> Vec<*mut Entry> {
Self::debug_assert_entries_mutex_held();
let mut entries: Vec<*mut Entry> = self.data.values().copied().collect();
bun_collections::index_sort::sort_slice_by(&mut entries, |a, b| {
// SAFETY: every value is a slot in the process-lifetime `EntryStore`.
let (a, b) = unsafe { (&**a, &**b) };
a.base_lowercase().cmp(b.base_lowercase())
});
entries
}

/// True if a cached entry exists for the given already-lowercase name.
pub fn has_comptime_query(&self, query_lower: &'static [u8]) -> bool {
Self::debug_assert_entries_mutex_held();
Expand Down
13 changes: 12 additions & 1 deletion src/resolver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1114,7 +1114,18 @@ pub mod fs {
EntriesOption::Entries(e) if e.generation < generation => {
in_place = Some(std::ptr::from_mut::<DirEntry>(*e));
}
cached => return Ok(cached),
cached => {
if !I::IS_VOID {
if let EntriesOption::Entries(entries) = &*cached {
// Sorted, because hash order is not stable (#26851).
for entry in entries.sorted_entries() {
// SAFETY: a live slot of the process-lifetime `EntryStore`.
iterator.next(unsafe { &mut *entry }, entries.fd);
}
}
}
return Ok(cached);
}
}
} else if cache_result.status == bun_alloc::ItemStatus::NotFound && generation == 0
{
Expand Down
36 changes: 2 additions & 34 deletions src/runtime/cli/test/Scanner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ 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_output::{declare_scope, scoped_log};
use bun_paths::resolve_path::{join_abs_string_buf_checked, platform};
Expand All @@ -30,7 +29,6 @@ pub(crate) struct Scanner<'a> {
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,
/// The directory being iterated; its fd closes once every child `ScanEntry` has been opened.
current_dir: Option<Rc<Dir>>,
Expand Down Expand Up @@ -86,7 +84,6 @@ impl<'a> Scanner<'a> {
fs: transpiler.fs,
test_files: results,
open_dir_buf: PathBuffer::ZEROED,
has_iterated: false,
search_count: 0,
current_dir: None,
})
Expand Down Expand Up @@ -161,34 +158,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(unsafe { &mut *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 @@ -328,12 +297,11 @@ impl<'a> Scanner<'a> {
&& !self.matches_path_ignore_pattern(name)
}

pub(crate) fn next(&mut self, entry: &mut fs::Entry) {
fn next(&mut self, entry: &mut fs::Entry) {
let name = entry.base_lowercase();
self.has_iterated = true;
// SAFETY: `self.fs` is the process singleton.
let real_fs = unsafe { &raw mut (*self.fs).fs };
// SAFETY: caller holds `entries_mutex`; the direct path is single-threaded.
// SAFETY: `real_fs` is the singleton; the resolver calls `next` with `entries_mutex` held.
match unsafe { entry.kind(real_fs, false) } {
fs::EntryKind::Dir => {
if (!name.is_empty() && name[0] == b'.') || name == b"node_modules" {
Expand Down
129 changes: 129 additions & 0 deletions test/cli/test/bun-test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1973,6 +1973,135 @@ describe.concurrent("test file discovery (scanner)", () => {
expect(exitCode).toBe(0);
});

// The resolver lists the cwd and its parents before the scanner runs, and it
// keeps the listing of every directory an earlier path argument walked. The
// scanner has to find the tests in those directories too.
const ranTest = (name: string) =>
`import { test } from "bun:test"; test("${name}", () => { console.log("RAN ${name}"); });`;
const monorepo = {
"top.test.ts": ranTest("top"),
"sibling/sibling.test.ts": ranTest("sibling"),
"pkg/pkg.test.ts": ranTest("pkg"),
"pkg/lib/lib.test.ts": ranTest("lib"),
"pkg/app/app.test.ts": ranTest("app"),
"pkg/app/deeper/deeper.test.ts": ranTest("deeper"),
};
const everyTest = ["RAN app", "RAN deeper", "RAN lib", "RAN pkg", "RAN sibling", "RAN top"];
async function discover(cwd: string, args: readonly string[], { inRunOrder = false } = {}) {
await using proc = Bun.spawn({
cmd: [bunExe(), "test", ...args],
env: bunEnv,
cwd,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
// With --parallel the stdout of a worker arrives on stderr.
const ran = `${stdout}\n${stderr}`.split(/\r?\n/).filter(line => line.startsWith("RAN "));
return {
ran: inRunOrder ? ran : ran.sort(),
summary: stderr.match(/^Ran \d+ tests? across \d+ files?\./m)?.[0] ?? stderr,
exitCode,
};
}

test.each(["./nested ./", "./ ./nested"])('path arguments "%s" run every test below both, once', async order => {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
using dir = tempDir("scanner-two-paths", {
"root_only.test.ts": ranTest("root"),
"nested/inner.test.ts": ranTest("inner"),
"nested/deeper/most.test.ts": ranTest("most"),
"other/other.test.ts": ranTest("other"),
});

expect(await discover(String(dir), order.split(" "))).toEqual({
ran: ["RAN inner", "RAN most", "RAN other", "RAN root"],
summary: "Ran 4 tests across 4 files.",
exitCode: 0,
});
});

describe.each([
{ how: "a path argument", args: ["../../"], bunfig: {} },
{ how: "the bunfig test root", args: [], bunfig: { "pkg/app/bunfig.toml": `[test]\nroot = "../../"\n` } },
])("$how that is a parent of the cwd", ({ args, bunfig }) => {
test("runs the tests in every directory", async () => {
using dir = tempDir("scanner-parent-of-cwd", { ...bunfig, ...monorepo });

expect(await discover(join(String(dir), "pkg", "app"), args)).toEqual({
ran: everyTest,
summary: "Ran 6 tests across 6 files.",
exitCode: 0,
});
});

// A scanner that skips the cwd finds no test file here and exits 1.
test("finds a test that is only below the cwd", async () => {
using dir = tempDir("scanner-parent-of-cwd-only", {
...bunfig,
"pkg/app/app.test.ts": ranTest("app"),
});

expect(await discover(join(String(dir), "pkg", "app"), args)).toEqual({
ran: ["RAN app"],
summary: "Ran 1 test across 1 file.",
exitCode: 0,
});
});
});

const belowPkg = ["RAN app", "RAN deeper", "RAN lib", "RAN pkg"];
test.each([
{ cmd: "../..", cwd: "pkg/app", args: () => ["../.."], ran: everyTest },
{ cmd: "<root>", cwd: "pkg/app", args: (root: string) => [root], ran: everyTest },
{ cmd: "<root>/pkg", cwd: "pkg/app", args: (root: string) => [join(root, "pkg")], ran: belowPkg },
{ cmd: "../../ ./", cwd: "pkg/app", args: () => ["../../", "./"], ran: everyTest },
{ cmd: "--parallel=2 ../../", cwd: "pkg/app", args: () => ["--parallel=2", "../../"], ran: everyTest },
{
cmd: "--cwd=<root>/pkg/app ../../",
cwd: ".",
args: (root: string) => [`--cwd=${join(root, "pkg", "app")}`, "../../"],
ran: everyTest,
},
{ cmd: "./pkg/app ./pkg/lib ./", cwd: ".", args: () => ["./pkg/app", "./pkg/lib", "./"], ran: everyTest },
])('"bun test $cmd" in $cwd runs every test below its path arguments, once', async ({ cwd, args, ran }) => {
using dir = tempDir("scanner-cached-dirs", monorepo);
const root = String(dir);

expect(await discover(join(root, cwd), args(root))).toEqual({
ran,
summary: `Ran ${ran.length} tests across ${ran.length} files.`,
exitCode: 0,
});
});

// `pkg` is on the path to the cwd, so its listing comes from the resolver's cache.
test("a directory the resolver already listed is scanned in name order", async () => {
const names = ["B", "a", "D", "c", "F", "e", "H", "g"];
using dir = tempDir("scanner-cached-order", {
...Object.fromEntries(names.map(name => [`pkg/${name}.test.ts`, ranTest(name)])),
"pkg/app/notes.txt": "",
});

expect(await discover(join(String(dir), "pkg", "app"), ["../../"], { inRunOrder: true })).toEqual({
ran: ["RAN a", "RAN B", "RAN c", "RAN D", "RAN e", "RAN F", "RAN g", "RAN H"],
summary: "Ran 8 tests across 8 files.",
exitCode: 0,
});
});

test("a failing test below the cwd fails a run whose root is a parent of the cwd", async () => {
using dir = tempDir("scanner-parent-of-cwd-fails", {
...monorepo,
"pkg/app/app.test.ts": `import { expect, test } from "bun:test"; test("app", () => { console.log("RAN app"); expect(1).toBe(2); });`,
});

expect(await discover(join(String(dir), "pkg", "app"), ["../../"])).toEqual({
ran: everyTest,
summary: "Ran 6 tests across 6 files.",
exitCode: 1,
});
});

// 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