diff --git a/src/jsc/hot_reloader.rs b/src/jsc/hot_reloader.rs index a5b258768dc6..d7211aa981c5 100644 --- a/src/jsc/hot_reloader.rs +++ b/src/jsc/hot_reloader.rs @@ -295,6 +295,9 @@ unsafe impl Sync for WatchChangedPaths {} pub static WATCH_CHANGED_TRIGGER_FILE: std::sync::OnceLock<&'static ZStr> = std::sync::OnceLock::new(); +/// Ends each path in the trigger file. A path can hold a line break, but not a NUL. +pub const WATCH_CHANGED_PATHS_TERMINATOR: &[u8] = b"\0"; + fn record_changed_path(path: &[u8]) { let Some(set) = WATCH_CHANGED_PATHS.get() else { return; @@ -331,11 +334,14 @@ fn flush_changed_paths_for_reload() { let mut buf: Vec = Vec::new(); for p in set.keys() { - if buf.try_reserve(p.len() + 1).is_err() { + if buf + .try_reserve(p.len() + WATCH_CHANGED_PATHS_TERMINATOR.len()) + .is_err() + { return; } buf.extend_from_slice(p); - buf.push(b'\n'); + buf.extend_from_slice(WATCH_CHANGED_PATHS_TERMINATOR); } let _ = bun_sys::File::write_file(Fd::cwd(), dest, &buf); } diff --git a/src/runtime/cli/test/ChangedFilesFilter.rs b/src/runtime/cli/test/ChangedFilesFilter.rs index 09a9907bf271..e81feec06f41 100644 --- a/src/runtime/cli/test/ChangedFilesFilter.rs +++ b/src/runtime/cli/test/ChangedFilesFilter.rs @@ -380,7 +380,7 @@ pub(crate) fn init_watch_trigger() { } /// If the previous process's watcher recorded which files triggered -/// this restart, read the newline-separated absolute-path list out of +/// this restart, read the NUL-terminated absolute-path list out of /// the trigger file, delete the file, and return the set. Returns null /// if the file is absent, empty, or every path no longer exists (in /// which case the caller falls back to querying git). @@ -408,7 +408,8 @@ fn consume_watch_trigger() -> Option { let _ = sys::unlink(&trigger_path); let mut set = StringSet::new(); - for path in strings::tokenize_any(&contents, b"\r\n") { + for path in strings::tokenize(&contents, jsc::hot_reloader::WATCH_CHANGED_PATHS_TERMINATOR) + { // The watcher may see a file disappear (delete/rename). A path // that no longer exists cannot appear in the module graph this // run, so drop it; its importers will still be picked up if the @@ -460,22 +461,28 @@ fn get_changed_files( // Find the git repository root so we can make the paths git prints // absolute (git prints paths relative to the repo toplevel with these // commands). - let git_root: Box<[u8]> = - match run_git(git_path, top_level_dir, &[b"rev-parse", b"--show-toplevel"]) { - GitResult::SpawnFailed => return Err(GitError::GitFailed), - GitResult::ExitError { stderr } => { - if !stderr.is_empty() { - Output::err_generic( - "--changed: {s}", - (BStr::new(strings::trim(&stderr, b" \r\n\t")),), - ); - } else { - Output::err_generic("--changed requires running inside a git repository", ()); - } + let tree = match run_git( + git_path, + top_level_dir, + &[b"rev-parse", b"--show-cdup", b"--show-toplevel"], + ) { + GitResult::SpawnFailed => return Err(GitError::GitFailed), + GitResult::ExitError { stderr } => { + return Err(git_failed(&stderr, || { + Output::err_generic("--changed requires running inside a git repository", ()); + })); + } + GitResult::Ok { stdout } => match WorkTree::parse(top_level_dir, &stdout) { + Some(tree) => tree, + None => { + Output::err_generic( + "--changed: `git rev-parse --show-cdup` printed a path that is too long", + (), + ); return Err(GitError::GitFailed); } - GitResult::Ok { stdout } => Box::<[u8]>::from(strings::trim(&stdout, b" \r\n\t")), - }; + }, + }; let mut set = StringSet::new(); @@ -483,53 +490,40 @@ fn get_changed_files( // Uncommitted (unstaged + staged). `git diff HEAD` covers both. // On a repo with no commits, `HEAD` is unresolved; fall back to just // `git diff` (unstaged) + staged. - match run_git( + match list_names( git_path, top_level_dir, - &[b"diff", b"--name-only", b"HEAD", b"--"], + b"diff", + &[b"--name-only", b"HEAD", b"--"], ) { GitResult::SpawnFailed => return Err(GitError::GitFailed), - GitResult::Ok { stdout } => append_paths(&mut set, &git_root, &stdout), + GitResult::Ok { stdout } => append_paths(&mut set, &tree.root, &stdout), GitResult::ExitError { .. } => { - match run_git(git_path, top_level_dir, &[b"diff", b"--name-only", b"--"]) { - GitResult::SpawnFailed => return Err(GitError::GitFailed), - GitResult::Ok { stdout } => append_paths(&mut set, &git_root, &stdout), - GitResult::ExitError { .. } => {} - } - - match run_git( - git_path, - top_level_dir, - &[b"diff", b"--name-only", b"--cached", b"--"], - ) { - GitResult::SpawnFailed => return Err(GitError::GitFailed), - GitResult::Ok { stdout } => append_paths(&mut set, &git_root, &stdout), - GitResult::ExitError { .. } => {} + let unstaged: &[&[u8]] = &[b"--name-only", b"--"]; + let staged: &[&[u8]] = &[b"--name-only", b"--cached", b"--"]; + for args in [unstaged, staged] { + let stdout = list_names_or_fail(git_path, top_level_dir, b"diff", args)?; + append_paths(&mut set, &tree.root, &stdout); } } } } else { - match run_git( + match list_names( git_path, top_level_dir, - &[b"diff", b"--name-only", since, b"--"], + b"diff", + &[b"--name-only", since, b"--"], ) { GitResult::SpawnFailed => return Err(GitError::GitFailed), GitResult::ExitError { stderr } => { - if !stderr.is_empty() { - Output::err_generic( - "--changed: {s}", - (BStr::new(strings::trim(&stderr, b" \r\n\t")),), - ); - } else { + return Err(git_failed(&stderr, || { Output::err_generic( "--changed: git diff against {f} failed", (bun_fmt::quote(since),), ); - } - return Err(GitError::GitFailed); + })); } - GitResult::Ok { stdout } => append_paths(&mut set, &git_root, &stdout), + GitResult::Ok { stdout } => append_paths(&mut set, &tree.root, &stdout), } } @@ -539,24 +533,64 @@ fn get_changed_files( // supplement with ls-files in both branches above. `--full-name` // forces repo-root-relative output regardless of our cwd, matching // `git diff --name-only`. - match run_git( + let stdout = list_names_or_fail( git_path, top_level_dir, + b"ls-files", &[ - b"ls-files", b"--others", b"--exclude-standard", b"--full-name", + b"--", + // Without a pathspec `ls-files` lists only what is below the cwd. + &tree.pathspec, ], - ) { - GitResult::SpawnFailed => return Err(GitError::GitFailed), - GitResult::Ok { stdout } => append_paths(&mut set, &git_root, &stdout), - GitResult::ExitError { .. } => {} - } + )?; + append_paths(&mut set, &tree.root, &stdout); Ok(set) } +/// The work tree that bun's cwd is in. +struct WorkTree { + /// Absolute path of the root. Git lists names relative to it. + root: Box<[u8]>, + /// The root, as a pathspec for a git command that runs in bun's cwd. + pathspec: Box<[u8]>, +} + +impl WorkTree { + /// Reads what `git rev-parse --show-cdup --show-toplevel` printed in `cwd`. + fn parse(cwd: &[u8], printed: &[u8]) -> Option { + // The cdup line is only `../` segments. The toplevel line can hold a line break. + if printed.starts_with(b"../") || printed.starts_with(b"\n") { + let up = strings::split_once_char(printed, b'\n').map_or(printed, |(up, _)| up); + if up.is_empty() { + return Some(WorkTree { + root: Box::from(cwd), + pathspec: Box::from(&b"."[..]), + }); + } + let mut buf = bun_paths::path_buffer_pool::get(); + let root = resolve_path::join_abs_string_buf_checked::( + cwd, + &mut buf.0, + &[up], + )?; + return Some(WorkTree { + root: Box::from(root), + pathspec: Box::from(up), + }); + } + // Outside the work tree (`GIT_WORK_TREE`) git prints the root twice and runs at the root. + let root = &printed[..printed.len() / 2]; + Some(WorkTree { + root: Box::from(root.strip_suffix(b"\n").unwrap_or(root)), + pathspec: Box::from(&b"."[..]), + }) + } +} + pub(crate) enum GitResult { /// `run_git` has already reported the spawn error. SpawnFailed, @@ -569,13 +603,14 @@ pub(crate) enum GitResult { } fn run_git(git_path: &[u8], cwd: &[u8], args: &[&[u8]]) -> GitResult { - let mut argv: Vec<&[u8]> = Vec::with_capacity(args.len() + 3); + let mut argv: Vec<&[u8]> = Vec::with_capacity(args.len() + 5); argv.push(git_path); - // `core.quotePath` (on by default) wraps non-ASCII filenames in quotes - // and emits octal escapes. We want raw UTF-8 paths so they match the - // bundler's resolved paths byte-for-byte. + // A user's `diff.relative=true` lists only the files below the cwd, relative to it. + argv.push(b"-c"); + argv.push(b"diff.relative=false"); + // A rename is listed by its new name only, and the old name can still be on disk. argv.push(b"-c"); - argv.push(b"core.quotePath=off"); + argv.push(b"diff.renames=false"); argv.extend_from_slice(args); let proc = match spawn_sync::spawn(&spawn_sync::Options { @@ -625,16 +660,55 @@ fn run_git(git_path: &[u8], cwd: &[u8], args: &[&[u8]]) -> GitResult { } } -/// Parse newline-delimited repo-relative paths from git output, join each +/// Runs `git -z `: git ends each name with a NUL and does not quote it. +fn list_names(git_path: &[u8], cwd: &[u8], command: &[u8], args: &[&[u8]]) -> GitResult { + run_git(git_path, cwd, &[&[command, b"-z"], args].concat()) +} + +/// [`list_names`] for a listing whose failure would otherwise read as "no changed files". +fn list_names_or_fail( + git_path: &[u8], + cwd: &[u8], + command: &[u8], + args: &[&[u8]], +) -> core::result::Result, GitError> { + match list_names(git_path, cwd, command, args) { + GitResult::SpawnFailed => Err(GitError::GitFailed), + GitResult::ExitError { stderr } => Err(git_failed(&stderr, || { + Output::err_generic("--changed: git {s} failed", (BStr::new(command),)); + })), + GitResult::Ok { stdout } => Ok(stdout), + } +} + +/// Reports a git command that failed: git's own message, or `otherwise` when git printed none. +fn git_failed(stderr: &[u8], otherwise: impl FnOnce()) -> GitError { + let message = strings::trim(stderr, b" \r\n\t"); + if message.is_empty() { + otherwise(); + } else { + Output::err_generic("--changed: {s}", (BStr::new(message),)); + } + GitError::GitFailed +} + +/// Parse NUL-terminated repo-relative paths from git output, join each /// with the repository root, and insert existing files into `set`. fn append_paths(set: &mut StringSet, git_root: &[u8], stdout: &[u8]) { let mut buf = bun_paths::path_buffer_pool::get(); - for line in strings::tokenize_any(stdout, b"\r\n") { - let rel = strings::trim(line, b" \t"); - if rel.is_empty() { + for rel in strings::tokenize(stdout, b"\0") { + // `ls-files` names an untracked nested repository as `dir/` and lists no file in it. + if rel.ends_with(b"/") { continue; } - let abs = resolve_path::join_abs_string_buf::(git_root, &mut buf.0, &[rel]); + // A path that does not fit the buffer is too long to open, so no source has it. + let Some(abs) = resolve_path::join_abs_string_buf_checked::( + git_root, + &mut buf.0, + &[rel], + ) else { + continue; + }; // Skip deletions; the bundler can only parse files that exist. if !sys::exists(abs) { continue; diff --git a/test/cli/test/test-changed.test.ts b/test/cli/test/test-changed.test.ts index 09a80ce39ef1..33a2abdbf7e1 100644 --- a/test/cli/test/test-changed.test.ts +++ b/test/cli/test/test-changed.test.ts @@ -1,8 +1,8 @@ import { spawnSync } from "bun"; import { describe, expect, setDefaultTimeout, test } from "bun:test"; -import { bunEnv, bunExe, isASAN, isWindows, tempDir, tmpdirSync } from "harness"; -import { appendFileSync, writeFileSync } from "node:fs"; -import { basename, join } from "node:path"; +import { bunEnv, bunExe, isASAN, isWindows, MAX_PATH_BYTES, tempDir, tmpdirSync } from "harness"; +import { appendFileSync, copyFileSync, existsSync, mkdirSync, truncateSync, writeFileSync } from "node:fs"; +import { basename, delimiter, join } from "node:path"; // Each case spawns a full `bun test` process; give the concurrent group // headroom on slow ASAN/CI machines. @@ -49,11 +49,13 @@ function initRepo(cwd: string) { async function runTestChanged( cwd: string, extra: string[] = [], + flag = "--changed", + env: Record = gitEnv, ): Promise<{ stdout: string; stderr: string; exitCode: number }> { await using proc = Bun.spawn({ - cmd: [bunExe(), "test", "--changed", ...extra], + cmd: [bunExe(), "test", flag, ...extra], cwd, - env: gitEnv, + env, stdout: "pipe", stderr: "pipe", stdin: "ignore", @@ -68,6 +70,35 @@ function ranFiles(stderr: string, names: string[]): string[] { return names.filter(n => stderr.includes(n + ":")).sort(); } +// Stands in for git where no state of a real repository gives the answer. It +// fails the command named by STAND_IN_FAILS the way git reports a fatal error, +// and it answers the command named by STAND_IN_ANSWERS with STAND_IN_ANSWER. +// It is a shell script, so the cases that use it skip Windows. +const gitStandIn = `#!/bin/sh +for arg in "$@"; do + if [ "$arg" = "$STAND_IN_FAILS" ]; then + echo "fatal: the stand-in for git does not run $arg" >&2 + exit 128 + fi + if [ "$arg" = "$STAND_IN_ANSWERS" ]; then + printf '%s\\n' "$STAND_IN_ANSWER" + exit 0 + fi +done +exec "$STAND_IN_GIT" "$@" +`; + +/** Writes the stand-in for git into `dir` and returns the environment that finds it before git. */ +function standInEnv( + dir: string, + vars: { STAND_IN_FAILS?: string; STAND_IN_ANSWERS?: string; STAND_IN_ANSWER?: string }, +) { + const bin = join(dir, "stand-in"); + mkdirSync(bin); + writeFileSync(join(bin, "git"), gitStandIn, { mode: 0o755 }); + return { ...gitEnv, ...vars, PATH: bin + delimiter + gitEnv.PATH, STAND_IN_GIT: Bun.which("git")! }; +} + // The --watch test at the end is the slow one; everything else is independent // git repos so run them concurrently. describe.concurrent("bun test --changed", () => { @@ -260,6 +291,319 @@ describe.concurrent("bun test --changed", () => { expect(exitCode).toBe(0); }); + const passingTest = (name: string, dependency?: string) => + `import { test, expect } from "bun:test";\n` + + (dependency ? `import { value } from ${JSON.stringify(dependency)};\n` : `const value = 1;\n`) + + `test(${JSON.stringify(name)}, () => expect(value).toBe(1));\n`; + + const monorepo = { + "package.json": JSON.stringify({ name: "root", private: true }), + "packages/a/package.json": JSON.stringify({ name: "a", type: "module" }), + "packages/a/a.test.ts": passingTest("a", "../b/value"), + "packages/a/local.ts": `export const value = 1;\n`, + "packages/a/local.test.ts": passingTest("local", "./local"), + "packages/a/plain.test.ts": passingTest("plain"), + "packages/b/package.json": JSON.stringify({ name: "b", type: "module" }), + "packages/b/value.js": `export const value = 1;\n`, + }; + const monorepoTests = ["a.test.ts", "fresh.test.ts", "local.test.ts", "plain.test.ts"]; + + // `git ls-files` lists only what is below its cwd unless a pathspec names + // more. The pathspec keeps its meaning under the pathspec variables that a + // git hook or a wrapper can export. + test.each([ + { flag: "--changed", ran: ["a.test.ts"] }, + { flag: "--changed=HEAD", ran: ["a.test.ts"] }, + { flag: "--changed", fromRoot: true, ran: ["a.test.ts"] }, + { flag: "--changed", args: ["./", "../b"], ran: ["a.test.ts", "fresh.test.ts"] }, + { flag: "--changed=HEAD", args: ["./", "../b"], ran: ["a.test.ts", "fresh.test.ts"] }, + { flag: "--changed", env: { GIT_GLOB_PATHSPECS: "1" }, ran: ["a.test.ts"] }, + { flag: "--changed", env: { GIT_LITERAL_PATHSPECS: "1" }, ran: ["a.test.ts"] }, + ])("untracked file outside the cwd is a changed file: %j", async ({ flag, fromRoot, args = [], env, ran }) => { + using dir = tempDir("test-changed-untracked-outside-cwd", monorepo); + initRepo(String(dir)); + // A new file that `../b/value` now resolves to in place of value.js, and a new test. + writeFileSync(join(String(dir), "packages", "b", "value.ts"), `export const value = 1;\n`); + writeFileSync(join(String(dir), "packages", "b", "fresh.test.ts"), passingTest("fresh")); + + const { stderr, exitCode } = fromRoot + ? await runTestChanged(String(dir), ["--cwd", join("packages", "a")], flag) + : await runTestChanged(join(String(dir), "packages", "a"), args, flag, { ...gitEnv, ...env }); + expect(ranFiles(stderr, monorepoTests)).toEqual(ran); + expect(exitCode).toBe(0); + }); + + // The other side of the wider list: an untracked file that no test here + // imports counts as a change, and a file that .gitignore names does not. + test.each(["--changed", "--changed=HEAD"])("%s counts an untracked file that no test imports", async flag => { + using dir = tempDir("test-changed-untracked-unrelated", { + ...monorepo, + "packages/b/.gitignore": "generated.ts\n", + "packages/a/generated.test.ts": passingTest("generated", "../b/generated"), + }); + initRepo(String(dir)); + writeFileSync(join(String(dir), "packages", "b", "generated.ts"), `export const value = 1;\n`); + writeFileSync(join(String(dir), "packages", "b", "notes.txt"), "notes\n"); + + const { stderr, exitCode } = await runTestChanged(join(String(dir), "packages", "a"), [], flag); + expect(stderr).toContain("--changed: 1 changed file, but no test files are affected\n"); + expect(ranFiles(stderr, [...monorepoTests, "generated.test.ts"])).toEqual([]); + expect(exitCode).toBe(0); + }); + + // Git lists a repository inside the work tree (a clone, or the target of + // `git worktree add`) as `nested/` and names no file in it. + test.each([".", join("packages", "a")])("untracked nested repository is not a changed file (cwd %s)", async cwd => { + using dir = tempDir("test-changed-nested-repository", monorepo); + initRepo(String(dir)); + const nested = join(String(dir), "nested"); + mkdirSync(nested); + writeFileSync(join(nested, "lib.ts"), `export const value = 1;\n`); + initRepo(nested); + + const { stderr, exitCode } = await runTestChanged(join(String(dir), cwd)); + expect(stderr).toContain("--changed: no changed files, nothing to run\n"); + expect(ranFiles(stderr, monorepoTests)).toEqual([]); + expect(exitCode).toBe(0); + }); + + // With `diff.relative` set, `git diff` lists only the files below the cwd, by + // names relative to the cwd. + test.each(["--changed", "--changed=HEAD"])("diff.relative does not change what %s lists", async flag => { + using dir = tempDir("test-changed-diff-relative", monorepo); + initRepo(String(dir)); + git(String(dir), "config", "diff.relative", "true"); + appendFileSync(join(String(dir), "packages", "b", "value.js"), "// touched\n"); + appendFileSync(join(String(dir), "packages", "a", "local.ts"), "// touched\n"); + + const { stderr, exitCode } = await runTestChanged(join(String(dir), "packages", "a"), [], flag); + expect(ranFiles(stderr, monorepoTests)).toEqual(["a.test.ts", "local.test.ts"]); + expect(exitCode).toBe(0); + }); + + // With rename detection `git diff` lists a rename by its new name only. Here + // the old name is still on disk: the file is taken out of the index and a + // copy is added under a new name. + test.each(["--changed", "--changed=HEAD"])( + "%s lists a file that is untracked and copied to a new name", + async flag => { + using dir = tempDir("test-changed-untrack-and-copy", { + "package.json": JSON.stringify({ name: "untrack-and-copy", type: "module" }), + "config.ts": `export const host = "localhost";\nexport const port = 8080;\nexport const value = 1;\n`, + "config.test.ts": passingTest("config", "./config.ts"), + "plain.test.ts": passingTest("plain"), + }); + initRepo(String(dir)); + git(String(dir), "rm", "-q", "--cached", "config.ts"); + writeFileSync(join(String(dir), ".gitignore"), "config.ts\n"); + copyFileSync(join(String(dir), "config.ts"), join(String(dir), "config.example.ts")); + git(String(dir), "add", ".gitignore", "config.example.ts"); + appendFileSync(join(String(dir), "config.ts"), "// a local edit\n"); + + const { stderr, exitCode } = await runTestChanged(String(dir), [], flag); + expect(ranFiles(stderr, ["config.test.ts", "plain.test.ts"])).toEqual(["config.test.ts"]); + expect(exitCode).toBe(0); + }, + ); + + // Git prints a name in C quotes when it holds a quote, a control character + // or a byte above 0x7f, unless it is asked for NUL-terminated names. A reader + // that splits on line breaks and trims blanks loses the other names. Windows + // cannot hold a quote, a tab or a line break in a file name. The accented + // name is precomposed, so that row is about the quotes and not about the + // form that macOS stores. + const oddNames = [ + " dep.ts", + "d\u00e9p-\u65e5\u672c.ts", + ...(isWindows ? [] : ['de"p.ts', "de\tp.ts", "de\np.ts", "de\rp.ts"]), + ]; + test.each(oddNames)("change to a dependency named %j selects the importing test", async name => { + using dir = tempDir("test-changed-odd-name", { + "package.json": JSON.stringify({ name: "odd-name", type: "module" }), + [name]: `export const value = 1;\n`, + "odd.test.ts": passingTest("odd", "./" + name), + "plain.test.ts": passingTest("plain"), + }); + initRepo(String(dir)); + appendFileSync(join(String(dir), name), "// touched\n"); + + for (const flag of ["--changed", "--changed=HEAD"]) { + const { stderr, exitCode } = await runTestChanged(String(dir), [], flag); + expect(ranFiles(stderr, ["odd.test.ts", "plain.test.ts"])).toEqual(["odd.test.ts"]); + expect(exitCode).toBe(0); + } + }); + + // Windows removes a trailing blank from a name and cannot hold a line break in one. + test.skipIf(isWindows).each(["repo ", "repo\n"])("works in a repository directory named %j", async repo => { + using dir = tempDir("test-changed-odd-root", { + [repo + "/package.json"]: JSON.stringify({ name: "odd-root", type: "module" }), + [repo + "/dep.ts"]: `export const value = 1;\n`, + [repo + "/dep.test.ts"]: passingTest("dep", "./dep"), + [repo + "/plain.test.ts"]: passingTest("plain"), + }); + const root = join(String(dir), repo); + initRepo(root); + appendFileSync(join(root, "dep.ts"), "// touched\n"); + + const { stderr, exitCode } = await runTestChanged(root); + expect(ranFiles(stderr, ["dep.test.ts", "plain.test.ts"])).toEqual(["dep.test.ts"]); + expect(exitCode).toBe(0); + }); + + // Git then prints the work tree where it prints `../` segments otherwise, and + // it runs each command at the root of the work tree. The work tree has a line + // break in its name where the system can hold one. + test.each(["--changed", "--changed=HEAD"])("cwd outside the work tree (GIT_WORK_TREE): %s", async flag => { + const repo = isWindows ? "repo" : "re\npo"; + using dir = tempDir("test-changed-work-tree", { + [repo + "/package.json"]: JSON.stringify({ name: "work-tree", type: "module" }), + [repo + "/ dep.ts"]: `export const value = 1;\n`, + "tests/package.json": JSON.stringify({ name: "tests", type: "module" }), + "tests/edited.test.ts": passingTest("edited", `../${repo}/ dep.ts`), + "tests/added.test.ts": passingTest("added", `../${repo}/new.ts`), + "tests/plain.test.ts": passingTest("plain"), + }); + const root = join(String(dir), repo); + initRepo(root); + appendFileSync(join(root, " dep.ts"), "// touched\n"); + writeFileSync(join(root, "new.ts"), `export const value = 1;\n`); + + const { stderr, exitCode } = await runTestChanged(join(String(dir), "tests"), [], flag, { + ...gitEnv, + GIT_DIR: join(root, ".git"), + GIT_WORK_TREE: root, + }); + expect(ranFiles(stderr, ["added.test.ts", "edited.test.ts", "plain.test.ts"])).toEqual([ + "added.test.ts", + "edited.test.ts", + ]); + expect(exitCode).toBe(0); + }); + + // Without a commit `git diff HEAD` fails, and the list comes from `git diff`, + // `git diff --cached` and `git ls-files`. The leading blank shows that each + // of the three is read as git wrote it. + test("repository with no commits lists staged, unstaged and untracked files", async () => { + const names = [" fresh.test.ts", " intent.test.ts", " staged.test.ts", "ignored.test.ts"]; + using dir = tempDir("test-changed-no-commits", { + "package.json": JSON.stringify({ name: "no-commits", type: "module" }), + ".gitignore": "ignored.test.ts\n", + ...Object.fromEntries(names.map(name => [name, passingTest(name)])), + }); + git(String(dir), "init", "-q"); + git(String(dir), "add", "--", "package.json", ".gitignore", " staged.test.ts"); + git(String(dir), "add", "--intent-to-add", "--", " intent.test.ts"); + + const { stderr, exitCode } = await runTestChanged(String(dir)); + expect(ranFiles(stderr, names)).toEqual([" fresh.test.ts", " intent.test.ts", " staged.test.ts"]); + expect(exitCode).toBe(0); + }); + + test("git failure is an error, not an empty list of changes", async () => { + using dir = tempDir("test-changed-git-fails", fixture); + initRepo(String(dir)); + appendFileSync(join(String(dir), "c.test.ts"), "// touched\n"); + // Every `git diff` and `git ls-files` fails on an index that is cut short. + truncateSync(join(String(dir), ".git", "index"), 20); + + const { stderr, exitCode } = await runTestChanged(String(dir)); + expect(stderr).toContain("error: --changed: fatal:"); + expect(ranFiles(stderr, names)).toEqual([]); + expect(exitCode).toBe(1); + }); + + // A damaged index fails every listing at once. The stand-in fails one command + // alone. It fails each `git diff`, because a bare --changed reads a failure + // of `git diff HEAD` alone as a repository with no commits. + test.skipIf(isWindows).each([ + { flag: "--changed", fails: "diff" }, + { flag: "--changed", fails: "ls-files" }, + { flag: "--changed=HEAD", fails: "ls-files" }, + ])("failing git listing is an error, not an empty list: %j", async ({ flag, fails }) => { + using dir = tempDir("test-changed-listing-fails", fixture); + initRepo(String(dir)); + // One change for `git diff` to list and one for `git ls-files`. + appendFileSync(join(String(dir), "src", "other.ts"), "// touched\n"); + writeFileSync(join(String(dir), "new.test.ts"), passingTest("new")); + + const env = standInEnv(String(dir), { STAND_IN_FAILS: fails }); + const { stderr, exitCode } = await runTestChanged(String(dir), [], flag, env); + expect(stderr).toContain(`error: --changed: fatal: the stand-in for git does not run ${fails}\n`); + expect(ranFiles(stderr, [...names, "new.test.ts"])).toEqual([]); + expect(exitCode).toBe(1); + }); + + // Git prints one `../` for each directory between the cwd and the root. An + // answer that no path buffer holds must not be joined to the cwd. + test.skipIf(isWindows)("root that does not fit a path buffer is an error", async () => { + using dir = tempDir("test-changed-long-root", fixture); + initRepo(String(dir)); + appendFileSync(join(String(dir), "c.test.ts"), "// touched\n"); + + const env = standInEnv(String(dir), { + STAND_IN_ANSWERS: "rev-parse", + STAND_IN_ANSWER: "../" + Buffer.alloc(MAX_PATH_BYTES, "d/").toString(), + }); + const { stderr, exitCode } = await runTestChanged(String(dir), [], "--changed", env); + expect(stderr).toContain("error: --changed: `git rev-parse --show-cdup` printed a path that is too long\n"); + expect(ranFiles(stderr, names)).toEqual([]); + expect(exitCode).toBe(1); + }); + + // Git can list a path that is longer than any path the system opens: here a + // file that is only in a commit. Windows path buffers hold any name git lists. + test.skipIf(isWindows)("listed path that is too long to open is not a changed file", async () => { + using dir = tempDir("test-changed-long-path", fixture); + initRepo(String(dir)); + // The name alone is below the limit, so git lists it. Joined to the root it is above. + const length = MAX_PATH_BYTES - 12; + const directories = Math.floor(length / 101); + const name = + Array.from({ length: directories }, () => Buffer.alloc(100, "d").toString() + "/").join("") + + Buffer.alloc(length - directories * 101, "f").toString(); + const blob = git(String(dir), "hash-object", "-w", "README.md").trim(); + git(String(dir), "update-index", "--add", "--cacheinfo", `100644,${blob},${name}`); + git(String(dir), "commit", "-q", "-m", "long"); + expect(git(String(dir), "diff", "--name-only", "HEAD", "--")).toBe(name + "\n"); + appendFileSync(join(String(dir), "src", "other.ts"), "// touched\n"); + + const { stderr, exitCode } = await runTestChanged(String(dir)); + expect(ranFiles(stderr, names)).toEqual(["b.test.ts"]); + expect(exitCode).toBe(0); + }); + + // A `--changed --watch` restart reads the paths that the watcher saw change + // from a list, and does not ask git. Here there is no repository and no git, + // so a run that asks git fails. A path can hold a line break. + test.skipIf(isWindows).each([ + { paths: ["de\np.ts", "dep-b.ts"], ran: ["wa.test.ts", "wb.test.ts"] }, + { paths: ["dep-b.ts", "de\np.ts"], ran: ["wa.test.ts", "wb.test.ts"] }, + { paths: ["dep-b.ts", "deleted.ts", "de\np.ts"], ran: ["wa.test.ts", "wb.test.ts"] }, + ])("restart list %j selects the tests that import its paths", async ({ paths, ran }) => { + using dir = tempDir("test-changed-restart-list", { + "package.json": JSON.stringify({ name: "restart-list", type: "module" }), + "de\np.ts": `export const value = 1;\n`, + "dep-b.ts": `export const value = 1;\n`, + "dep-c.ts": `export const value = 1;\n`, + "wa.test.ts": passingTest("wa", "./de\np.ts"), + "wb.test.ts": passingTest("wb", "./dep-b"), + "wc.test.ts": passingTest("wc", "./dep-c"), + "no-git/.keep": "", + }); + const list = join(String(dir), "restart-list"); + writeFileSync(list, paths.map(path => join(String(dir), path) + "\0").join("")); + + const { stderr, exitCode } = await runTestChanged(String(dir), [], "--changed", { + ...bunEnv, + PATH: join(String(dir), "no-git"), + BUN_INTERNAL_TEST_CHANGED_TRIGGER_FILE: list, + }); + expect(ranFiles(stderr, ["wa.test.ts", "wb.test.ts", "wc.test.ts"])).toEqual(ran); + expect(existsSync(list)).toBe(false); + expect(exitCode).toBe(0); + }); + test("change inside node_modules does not select any test", async () => { using dir = tempDir("test-changed-nm", { "package.json": JSON.stringify({ name: "nm", type: "module" }), @@ -531,6 +875,61 @@ describe.skipIf(isWindows)("bun test --changed --watch", () => { reader.releaseLock(); }, 60_000); + // The watcher hands the restarted process the paths it saw change. A path + // can hold a line break. + test("editing a dirty file with a line break in its name reruns only its test", async () => { + using dir = tempDir("test-changed-watch-line-break", { + "package.json": JSON.stringify({ name: "watch", type: "module" }), + "de\np.ts": `export const A = 1;\n`, + "dep-b.ts": `export const B = 2;\n`, + "wa.test.ts": `import { test, expect } from "bun:test";\nimport { A } from "./de\\np.ts";\ntest("wa", () => expect(A).toBe(1));\n`, + "wb.test.ts": `import { test, expect } from "bun:test";\nimport { B } from "./dep-b";\ntest("wb", () => expect(B).toBe(2));\n`, + }); + initRepo(String(dir)); + // Both dependencies are dirty, so a restart that asks git reruns both tests. + appendFileSync(join(String(dir), "de\np.ts"), "// dirty\n"); + appendFileSync(join(String(dir), "dep-b.ts"), "// dirty\n"); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "--changed", "--watch", "--no-clear-screen"], + cwd: String(dir), + env: gitEnv, + stdout: "ignore", + stderr: "pipe", + stdin: "ignore", + }); + + const reader = proc.stderr.getReader(); + const decoder = new TextDecoder(); + let buf = ""; + let seen = 0; + + /** The output of the next run, up to its summary line. */ + async function nextRun(): Promise { + const summary = /Ran \d+ tests? across \d+ files?\./g; + while (true) { + summary.lastIndex = seen; + if (summary.test(buf)) { + const run = buf.slice(seen, summary.lastIndex); + seen = summary.lastIndex; + return run; + } + const { value, done } = await reader.read(); + if (done) throw new Error(`stream closed before the next run ended\n${buf}`); + buf += decoder.decode(value, { stream: true }); + } + } + + // Initial run: git lists both dependencies. + expect(ranFiles(await nextRun(), ["wa.test.ts", "wb.test.ts"])).toEqual(["wa.test.ts", "wb.test.ts"]); + + appendFileSync(join(String(dir), "de\np.ts"), "// touched again\n"); + expect(ranFiles(await nextRun(), ["wa.test.ts", "wb.test.ts"])).toEqual(["wa.test.ts"]); + + proc.kill(); + reader.releaseLock(); + }); + test("trigger file path handed to restarted runs has a 128-bit random hex suffix", async () => { using dir = tempDir("test-changed-watch-trigger-name", { "package.json": JSON.stringify({ name: "watch", type: "module" }),