fix(cli/test): improve filtering DX (#18847)

Co-authored-by: DonIsaac <22823424+DonIsaac@users.noreply.github.com>
This commit is contained in:
Don Isaac
2025-04-09 16:41:32 -07:00
committed by GitHub
parent 9f023d7471
commit 1d6bdf745b
6 changed files with 322 additions and 220 deletions

226
src/cli/test/Scanner.zig Normal file
View File

@@ -0,0 +1,226 @@
const Scanner = @This();
const std = @import("std");
const bun = @import("root").bun;
const BundleOptions = @import("../../options.zig").BundleOptions;
const Allocator = std.mem.Allocator;
const FileSystem = bun.fs.FileSystem;
const Transpiler = bun.Transpiler;
const strings = bun.strings;
const StringOrTinyString = strings.StringOrTinyString;
const JSC = bun.JSC;
const jest = JSC.Jest;
/// Memory is borrowed.
exclusion_names: []const []const u8 = &.{},
/// When this list is empty, no filters are applied.
/// "test" suffixes (e.g. .spec.*) are always applied when traversing directories.
filter_names: []const []const u8 = &.{},
dirs_to_scan: Fifo,
/// Paths to test files found while scanning.
test_files: std.ArrayListUnmanaged(bun.PathString),
fs: *FileSystem,
open_dir_buf: bun.PathBuffer = undefined,
scan_dir_buf: bun.PathBuffer = undefined,
options: *BundleOptions,
has_iterated: bool = false,
search_count: usize = 0,
const log = bun.Output.scoped(.jest, true);
const Fifo = std.fifo.LinearFifo(ScanEntry, .Dynamic);
const ScanEntry = struct {
relative_dir: bun.StoredFileDescriptorType,
dir_path: []const u8,
name: StringOrTinyString,
};
const Error = error{
/// Scan entrypoint file/directory does not exist. Not returned when
/// a subdirectory is scanned but does not exist.
DoesNotExist,
} || Allocator.Error;
pub fn init(
alloc: Allocator,
transpiler: *Transpiler,
initial_results_capacity: usize,
) Allocator.Error!Scanner {
const results = try std.ArrayListUnmanaged(bun.PathString).initCapacity(
alloc,
initial_results_capacity,
);
return Scanner{
.dirs_to_scan = Fifo.init(alloc),
.options = &transpiler.options,
.fs = transpiler.fs,
.test_files = results,
};
}
pub fn deinit(this: *Scanner) void {
this.test_files.deinit(this.allocator());
this.dirs_to_scan.deinit();
this.* = undefined;
}
/// Take the list of test files out of this scanner. Caller owns the returned
/// allocation.
pub fn takeFoundTestFiles(this: *Scanner) Allocator.Error![]bun.PathString {
return this.test_files.toOwnedSlice(this.allocator());
}
pub fn scan(this: *Scanner, path_literal: []const u8) Error!void {
const parts = &[_][]const u8{ this.fs.top_level_dir, path_literal };
const path = this.fs.absBuf(parts, &this.scan_dir_buf);
var root = this.readDirWithName(path, null) catch |err| {
switch (err) {
error.NotDir, error.ENOTDIR => {
if (this.isTestFile(path)) {
const rel_path = bun.PathString.init(this.fs.filename_store.append([]const u8, path) catch bun.outOfMemory());
this.test_files.append(this.allocator(), rel_path) catch bun.outOfMemory();
}
},
error.ENOENT => return error.DoesNotExist,
error.OutOfMemory => return error.OutOfMemory,
else => log("Scanner.readDirWithName('{s}') -> {s}", .{ path, @errorName(err) }),
}
return;
};
// you typed "." and we already scanned it
if (!this.has_iterated) {
if (@as(FileSystem.RealFS.EntriesOption.Tag, root.*) == .entries) {
var iter = root.entries.data.iterator();
const fd = root.entries.fd;
bun.assert(fd != bun.invalid_fd);
while (iter.next()) |entry| {
this.next(entry.value_ptr.*, fd);
}
}
}
while (this.dirs_to_scan.readItem()) |entry| {
if (!bun.Environment.isWindows) {
const dir = entry.relative_dir.asDir();
bun.assert(bun.toFD(dir.fd) != bun.invalid_fd);
const parts2 = &[_][]const u8{ entry.dir_path, entry.name.slice() };
var path2 = this.fs.absBuf(parts2, &this.open_dir_buf);
this.open_dir_buf[path2.len] = 0;
const pathZ = this.open_dir_buf[path2.len - entry.name.slice().len .. path2.len :0];
const child_dir = bun.openDir(dir, pathZ) catch continue;
path2 = try this.fs.dirname_store.append([]const u8, path2);
FileSystem.setMaxFd(child_dir.fd);
_ = this.readDirWithName(path2, child_dir) catch return error.OutOfMemory;
} else {
const dir = entry.relative_dir.asDir();
bun.assert(bun.toFD(dir.fd) != bun.invalid_fd);
const parts2 = &[_][]const u8{ entry.dir_path, entry.name.slice() };
const path2 = this.fs.absBufZ(parts2, &this.open_dir_buf);
const child_dir = bun.openDirNoRenamingOrDeletingWindows(bun.invalid_fd, path2) catch continue;
_ = this.readDirWithName(
try this.fs.dirname_store.append([]const u8, path2),
child_dir,
) catch return error.OutOfMemory;
}
}
}
fn readDirWithName(this: *Scanner, name: []const u8, handle: ?std.fs.Dir) !*FileSystem.RealFS.EntriesOption {
return try this.fs.fs.readDirectoryWithIterator(name, handle, 0, true, *Scanner, this);
}
pub const test_name_suffixes = [_][]const u8{
".test",
"_test",
".spec",
"_spec",
};
pub fn couldBeTestFile(this: *Scanner, name: []const u8, comptime needs_test_suffix: bool) bool {
const extname = std.fs.path.extension(name);
if (extname.len == 0 or !this.options.loader(extname).isJavaScriptLike()) return false;
if (comptime !needs_test_suffix) return true;
const name_without_extension = name[0 .. name.len - extname.len];
inline for (test_name_suffixes) |suffix| {
if (strings.endsWithComptime(name_without_extension, suffix)) return true;
}
return false;
}
pub fn doesAbsolutePathMatchFilter(this: *Scanner, name: []const u8) bool {
if (this.filter_names.len == 0) return true;
for (this.filter_names) |filter_name| {
if (strings.startsWith(name, filter_name)) return true;
}
return false;
}
pub fn doesPathMatchFilter(this: *Scanner, name: []const u8) bool {
if (this.filter_names.len == 0) return true;
for (this.filter_names) |filter_name| {
if (strings.contains(name, filter_name)) return true;
}
return false;
}
pub fn isTestFile(this: *Scanner, name: []const u8) bool {
return this.couldBeTestFile(name, false) and this.doesPathMatchFilter(name);
}
pub fn next(this: *Scanner, entry: *FileSystem.Entry, fd: bun.StoredFileDescriptorType) void {
const name = entry.base_lowercase();
this.has_iterated = true;
switch (entry.kind(&this.fs.fs, true)) {
.dir => {
if ((name.len > 0 and name[0] == '.') or strings.eqlComptime(name, "node_modules")) {
return;
}
if (comptime bun.Environment.allow_assert)
bun.assert(!strings.contains(name, std.fs.path.sep_str ++ "node_modules" ++ std.fs.path.sep_str));
for (this.exclusion_names) |exclude_name| {
if (strings.eql(exclude_name, name)) return;
}
this.search_count += 1;
this.dirs_to_scan.writeItem(.{
.relative_dir = fd,
.name = entry.base_,
.dir_path = entry.dir,
}) catch unreachable;
},
.file => {
// already seen it!
if (!entry.abs_path.isEmpty()) return;
this.search_count += 1;
if (!this.couldBeTestFile(name, true)) return;
const parts = &[_][]const u8{ entry.dir, entry.base() };
const path = this.fs.absBuf(parts, &this.open_dir_buf);
if (!this.doesAbsolutePathMatchFilter(path)) {
const rel_path = bun.path.relative(this.fs.top_level_dir, path);
if (!this.doesPathMatchFilter(rel_path)) return;
}
entry.abs_path = bun.PathString.init(this.fs.filename_store.append(@TypeOf(path), path) catch unreachable);
this.test_files.append(this.allocator(), entry.abs_path) catch unreachable;
},
}
}
inline fn allocator(self: *const Scanner) Allocator {
return self.dirs_to_scan.allocator;
}

View File

@@ -46,6 +46,8 @@ const coverage = bun.sourcemap.coverage;
const CodeCoverageReport = coverage.Report;
const uws = bun.uws;
const Scanner = @import("test/Scanner.zig");
fn escapeXml(str: string, writer: anytype) !void {
var last: usize = 0;
var i: usize = 0;
@@ -958,208 +960,43 @@ pub const CommandLineReporter = struct {
}
};
const Scanner = struct {
const Fifo = std.fifo.LinearFifo(ScanEntry, .Dynamic);
exclusion_names: []const []const u8 = &.{},
filter_names: []const []const u8 = &.{},
dirs_to_scan: Fifo,
results: *std.ArrayList(bun.PathString),
fs: *FileSystem,
open_dir_buf: bun.PathBuffer = undefined,
scan_dir_buf: bun.PathBuffer = undefined,
options: *options.BundleOptions,
has_iterated: bool = false,
search_count: usize = 0,
export fn BunTest__shouldGenerateCodeCoverage(test_name_str: bun.String) callconv(.C) bool {
var zig_slice: bun.JSC.ZigString.Slice = .{};
defer zig_slice.deinit();
const ScanEntry = struct {
relative_dir: bun.StoredFileDescriptorType,
dir_path: string,
name: strings.StringOrTinyString,
// In this particular case, we don't actually care about non-ascii latin1 characters.
// so we skip the ascii check
const slice = brk: {
zig_slice = test_name_str.toUTF8(bun.default_allocator);
break :brk zig_slice.slice();
};
fn readDirWithName(this: *Scanner, name: string, handle: ?std.fs.Dir) !*FileSystem.RealFS.EntriesOption {
return try this.fs.fs.readDirectoryWithIterator(name, handle, 0, true, *Scanner, this);
}
pub fn scan(this: *Scanner, path_literal: string) void {
const parts = &[_]string{ this.fs.top_level_dir, path_literal };
const path = this.fs.absBuf(parts, &this.scan_dir_buf);
var root = this.readDirWithName(path, null) catch |err| {
if (err == error.NotDir) {
if (this.isTestFile(path)) {
this.results.append(bun.PathString.init(this.fs.filename_store.append(@TypeOf(path), path) catch bun.outOfMemory())) catch bun.outOfMemory();
}
}
return;
};
// you typed "." and we already scanned it
if (!this.has_iterated) {
if (@as(FileSystem.RealFS.EntriesOption.Tag, root.*) == .entries) {
var iter = root.entries.data.iterator();
const fd = root.entries.fd;
bun.assert(fd != bun.invalid_fd);
while (iter.next()) |entry| {
this.next(entry.value_ptr.*, fd);
}
}
}
while (this.dirs_to_scan.readItem()) |entry| {
if (!Environment.isWindows) {
const dir = entry.relative_dir.asDir();
bun.assert(bun.toFD(dir.fd) != bun.invalid_fd);
const parts2 = &[_]string{ entry.dir_path, entry.name.slice() };
var path2 = this.fs.absBuf(parts2, &this.open_dir_buf);
this.open_dir_buf[path2.len] = 0;
const pathZ = this.open_dir_buf[path2.len - entry.name.slice().len .. path2.len :0];
const child_dir = bun.openDir(dir, pathZ) catch continue;
path2 = this.fs.dirname_store.append(string, path2) catch bun.outOfMemory();
FileSystem.setMaxFd(child_dir.fd);
_ = this.readDirWithName(path2, child_dir) catch continue;
} else {
const dir = entry.relative_dir.asDir();
bun.assert(bun.toFD(dir.fd) != bun.invalid_fd);
const parts2 = &[_]string{ entry.dir_path, entry.name.slice() };
const path2 = this.fs.absBufZ(parts2, &this.open_dir_buf);
const child_dir = bun.openDirNoRenamingOrDeletingWindows(bun.invalid_fd, path2) catch continue;
_ = this.readDirWithName(
this.fs.dirname_store.append(string, path2) catch bun.outOfMemory(),
child_dir,
) catch bun.outOfMemory();
}
}
}
const test_name_suffixes = [_]string{
".test",
"_test",
".spec",
"_spec",
};
export fn BunTest__shouldGenerateCodeCoverage(test_name_str: bun.String) callconv(.C) bool {
var zig_slice: bun.JSC.ZigString.Slice = .{};
defer zig_slice.deinit();
// In this particular case, we don't actually care about non-ascii latin1 characters.
// so we skip the ascii check
const slice = brk: {
zig_slice = test_name_str.toUTF8(bun.default_allocator);
break :brk zig_slice.slice();
};
// always ignore node_modules.
if (strings.contains(slice, "/node_modules/") or strings.contains(slice, "\\node_modules\\")) {
return false;
}
const ext = std.fs.path.extension(slice);
const loader_by_ext = JSC.VirtualMachine.get().transpiler.options.loader(ext);
// allow file loader just incase they use a custom loader with a non-standard extension
if (!(loader_by_ext.isJavaScriptLike() or loader_by_ext == .file)) {
return false;
}
if (jest.Jest.runner) |runner| {
if (runner.test_options.coverage.skip_test_files) {
const name_without_extension = slice[0 .. slice.len - ext.len];
inline for (test_name_suffixes) |suffix| {
if (strings.endsWithComptime(name_without_extension, suffix)) {
return false;
}
}
}
}
return true;
}
pub fn couldBeTestFile(this: *Scanner, name: string) bool {
const extname = std.fs.path.extension(name);
if (!this.options.loader(extname).isJavaScriptLike()) return false;
const name_without_extension = name[0 .. name.len - extname.len];
inline for (test_name_suffixes) |suffix| {
if (strings.endsWithComptime(name_without_extension, suffix)) return true;
}
// always ignore node_modules.
if (bun.strings.contains(slice, "/node_modules/") or bun.strings.contains(slice, "\\node_modules\\")) {
return false;
}
pub fn doesAbsolutePathMatchFilter(this: *Scanner, name: string) bool {
if (this.filter_names.len == 0) return true;
for (this.filter_names) |filter_name| {
if (strings.startsWith(name, filter_name)) return true;
}
const ext = std.fs.path.extension(slice);
const loader_by_ext = JSC.VirtualMachine.get().transpiler.options.loader(ext);
// allow file loader just incase they use a custom loader with a non-standard extension
if (!(loader_by_ext.isJavaScriptLike() or loader_by_ext == .file)) {
return false;
}
pub fn doesPathMatchFilter(this: *Scanner, name: string) bool {
if (this.filter_names.len == 0) return true;
for (this.filter_names) |filter_name| {
if (strings.contains(name, filter_name)) return true;
}
return false;
}
pub fn isTestFile(this: *Scanner, name: string) bool {
return this.couldBeTestFile(name) and this.doesPathMatchFilter(name);
}
pub fn next(this: *Scanner, entry: *FileSystem.Entry, fd: bun.StoredFileDescriptorType) void {
const name = entry.base_lowercase();
this.has_iterated = true;
switch (entry.kind(&this.fs.fs, true)) {
.dir => {
if ((name.len > 0 and name[0] == '.') or strings.eqlComptime(name, "node_modules")) {
return;
if (jest.Jest.runner) |runner| {
if (runner.test_options.coverage.skip_test_files) {
const name_without_extension = slice[0 .. slice.len - ext.len];
inline for (Scanner.test_name_suffixes) |suffix| {
if (bun.strings.endsWithComptime(name_without_extension, suffix)) {
return false;
}
if (comptime Environment.allow_assert)
bun.assert(!strings.contains(name, std.fs.path.sep_str ++ "node_modules" ++ std.fs.path.sep_str));
for (this.exclusion_names) |exclude_name| {
if (strings.eql(exclude_name, name)) return;
}
this.search_count += 1;
this.dirs_to_scan.writeItem(.{
.relative_dir = fd,
.name = entry.base_,
.dir_path = entry.dir,
}) catch unreachable;
},
.file => {
// already seen it!
if (!entry.abs_path.isEmpty()) return;
this.search_count += 1;
if (!this.couldBeTestFile(name)) return;
const parts = &[_]string{ entry.dir, entry.base() };
const path = this.fs.absBuf(parts, &this.open_dir_buf);
if (!this.doesAbsolutePathMatchFilter(path)) {
const rel_path = bun.path.relative(this.fs.top_level_dir, path);
if (!this.doesPathMatchFilter(rel_path)) return;
}
entry.abs_path = bun.PathString.init(this.fs.filename_store.append(@TypeOf(path), path) catch unreachable);
this.results.append(entry.abs_path) catch unreachable;
},
}
}
}
};
return true;
}
pub const TestCommand = struct {
pub const name = "test";
@@ -1314,29 +1151,36 @@ pub const TestCommand = struct {
_ = vm.global.setTimeZone(&JSC.ZigString.init(TZ_NAME));
}
var results = try std.ArrayList(PathString).initCapacity(ctx.allocator, ctx.positionals.len);
defer results.deinit();
// Start the debugger before we scan for files
// But, don't block the main thread waiting if they used --inspect-wait.
//
try vm.ensureDebugger(false);
const test_files, const search_count = scan: {
if (for (ctx.positionals) |arg| {
if (std.fs.path.isAbsolute(arg) or
strings.startsWith(arg, "./") or
strings.startsWith(arg, "../") or
(Environment.isWindows and (strings.startsWith(arg, ".\\") or
strings.startsWith(arg, "..\\")))) break true;
} else false) {
// One of the files is a filepath. Instead of treating the arguments as filters, treat them as filepaths
for (ctx.positionals[1..]) |arg| {
results.appendAssumeCapacity(PathString.init(arg));
}
break :scan .{ results.items, 0 };
var scanner = Scanner.init(ctx.allocator, &vm.transpiler, ctx.positionals.len) catch bun.outOfMemory();
defer scanner.deinit();
const has_relative_path = for (ctx.positionals) |arg| {
if (std.fs.path.isAbsolute(arg) or
strings.startsWith(arg, "./") or
strings.startsWith(arg, "../") or
(Environment.isWindows and (strings.startsWith(arg, ".\\") or
strings.startsWith(arg, "..\\")))) break true;
} else false;
if (has_relative_path) {
// One of the files is a filepath. Instead of treating the
// arguments as filters, treat them as filepaths
const file_or_dirnames = ctx.positionals[1..];
for (file_or_dirnames) |arg| {
scanner.scan(arg) catch |err| switch (err) {
error.OutOfMemory => bun.outOfMemory(),
// don't error if multiple are passed; one might fail
// but the others may not
error.DoesNotExist => if (file_or_dirnames.len == 1) {
Output.prettyErrorln("Test filter <b>{}<r> had no matches", .{bun.fmt.quote(arg)});
Global.exit(1);
},
};
}
} else {
// Treat arguments as filters and scan the codebase
const filter_names = if (ctx.positionals.len == 0) &[0][]const u8{} else ctx.positionals[1..];
@@ -1356,14 +1200,8 @@ pub const TestCommand = struct {
ctx.allocator.free(i);
ctx.allocator.free(filter_names_normalized);
};
scanner.filter_names = filter_names_normalized;
var scanner = Scanner{
.dirs_to_scan = Scanner.Fifo.init(ctx.allocator),
.options = &vm.transpiler.options,
.fs = vm.transpiler.fs,
.filter_names = filter_names_normalized,
.results = &results,
};
const dir_to_scan = brk: {
if (ctx.debug.test_directory.len > 0) {
break :brk try vm.allocator.dupe(u8, resolve_path.joinAbs(scanner.fs.top_level_dir, .auto, ctx.debug.test_directory));
@@ -1372,11 +1210,18 @@ pub const TestCommand = struct {
break :brk scanner.fs.top_level_dir;
};
scanner.scan(dir_to_scan);
scanner.dirs_to_scan.deinit();
scanner.scan(dir_to_scan) catch |err| switch (err) {
error.OutOfMemory => bun.outOfMemory(),
error.DoesNotExist => {
Output.prettyErrorln("<red>Failed to scan non-existent root directory for tests:<r> {s}", .{dir_to_scan});
Global.exit(1);
},
};
}
break :scan .{ scanner.results.items, scanner.search_count };
};
const test_files = scanner.takeFoundTestFiles() catch bun.outOfMemory();
defer ctx.allocator.free(test_files);
const search_count = scanner.search_count;
if (test_files.len > 0) {
vm.hot_reload = ctx.debug.hot_reload;
@@ -1387,7 +1232,6 @@ pub const TestCommand = struct {
else => {},
}
// vm.transpiler.fs.fs.readDirectory(_dir: string, _handle: ?std.fs.Dir)
runAllTests(reporter, vm, test_files, ctx.allocator);
}

View File

@@ -1058,6 +1058,8 @@ pub const FileSystem = struct {
// https://twitter.com/jarredsumner/status/1655787337027309568
// https://twitter.com/jarredsumner/status/1655714084569120770
// https://twitter.com/jarredsumner/status/1655464485245845506
/// Caller borrows the returned EntriesOption. When `FeatureFlags.enable_entry_cache` is `false`,
/// it is not safe to store this pointer past the current function call.
pub fn readDirectoryWithIterator(
fs: *RealFS,
dir_maybe_trail_slash: string,

3
src/js/private.d.ts vendored
View File

@@ -113,6 +113,9 @@ declare module "bun" {
var fetch: typeof globalThis.fetch;
}
/**
* `JSC::JSModuleLoader`
*/
declare var Loader: {
registry: Map<string, LoaderEntry>;

View File

@@ -28,6 +28,7 @@ pub const PathString = packed struct {
return @as([*:0]u8, @ptrFromInt(@as(usize, @intCast(this.ptr))))[0..this.len :0];
}
/// Create a PathString from a borrowed slice. No allocation occurs.
pub inline fn init(str: []const u8) @This() {
@setRuntimeSafety(false); // "cast causes pointer to be null" is fine here. if it is null, the len will be 0.

View File

@@ -1,6 +1,6 @@
import { spawnSync } from "bun";
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, tmpdirSync } from "harness";
import { describe, beforeAll, expect, test, it } from "bun:test";
import { bunEnv, bunExe, tempDirWithFiles, tmpdirSync } from "harness";
import { mkdirSync, rmSync, writeFileSync } from "node:fs";
import { dirname, join, resolve } from "node:path";
@@ -113,6 +113,32 @@ describe("bun test", () => {
});
expect(stderr).toContain(path);
});
describe("when filters are provided", () => {
let dir: string;
beforeAll(() => {
const makeTest = (name: string, pass = true) => `
import { test, expect } from "bun:test";
test("${name}", () => {
expect(1).toBe(${pass ? 1 : 0});
});
`;
dir = tempDirWithFiles("bun-test-filtering", {
"foo.test.js": makeTest("foo"),
bar: {
"bar1.spec.tsx": makeTest("bar1"),
"bar2.spec.ts": makeTest("bar2"),
},
});
});
it("if that filter is a path to a directory, will run all tests in that directory", () => {
const stderr = runTest({ cwd: dir, args: ["./bar"] });
expect(stderr).toContain("2 pass");
expect(stderr).not.toContain("foo");
});
});
test("works with require", () => {
const stderr = runTest({
args: [],