bun test --coverage: append a final newline to a text that lacks one

src/jsc/bindings/ZigSourceProvider.cpp

@@ -55,9 +55,17 @@ void addCodeCoverageSourceID(JSC::VM& vm, JSC::SourceProvider& provider)
 }
 
 extern "C" bool BunTest__shouldGenerateCodeCoverage(const BunString* sourceURL);
+extern "C" bool BunTest__writesCoverageReport();
 extern "C" void Bun__addSourceProviderSourceMap(void* bun_vm, SourceProvider* opaque_source_provider, const BunString* specifier);
 extern "C" void Bun__removeSourceProviderSourceMap(void* bun_vm, SourceProvider* opaque_source_provider, const BunString* specifier);
 
+// A text of one line needs none: an empty CommonJS file runs as one, and it reports no line.
+static bool needsFinalLineTerminator(const String& text)
+{
+    auto isLineTerminator = [](char16_t unit) { return JSC::isLineTerminator(unit); };
+    return !text.isEmpty() && !isLineTerminator(text[text.length() - 1]) && text.contains(isLineTerminator);
+}
+
 Ref<SourceProvider> SourceProvider::create(
     Zig::GlobalObject* globalObject,
     ResolvedSource& resolvedSource,
@@ -81,6 +89,9 @@ Ref<SourceProvider> SourceProvider::create(
     if (isCodeCoverageEnabled && !isBuiltin) {
         BunString sourceURLBunString = Bun::toString(sourceURLString);
         shouldGenerateCodeCoverage = BunTest__shouldGenerateCodeCoverage(&sourceURLBunString);
+        // The report of `bun test --coverage` reads a text as lines that end with a terminator.
+        if (shouldGenerateCodeCoverage && needsFinalLineTerminator(string) && BunTest__writesCoverageReport())
+            string = makeString(string, '\n');
     }
 
     const auto getSourceOrigin = [&]() -> SourceOrigin {

src/runtime/cli/test_command.rs

@@ -1730,6 +1730,12 @@ extern "C" fn BunTest__shouldGenerateCodeCoverage(test_name_str: &bun_core::Stri
     true
 }
 
+/// Whether this process is `bun test --coverage`, which writes the coverage report.
+#[unsafe(no_mangle)]
+extern "C" fn BunTest__writesCoverageReport() -> bool {
+    jest::Jest::runner().is_some_and(|runner| runner.test_options.coverage.enabled)
+}
+
 pub(crate) struct TestCommand;
 
 impl TestCommand {

src/sourcemap_jsc/CodeCoverage.rs

@@ -827,10 +827,10 @@ impl ByteRangeMapping {
                 // functions that have executed have non-executable lines in them and thats fine.
                 if !did_fn_execute {
                     let end = max_line.min(line_count);
-                    line_hits_slice[min_line as usize..end as usize].fill(0);
                     for line in min_line..end {
                         executable_lines.set(line as usize);
                         lines_which_have_executed.unset(line as usize);
+                        line_hits_slice[line as usize] = 0;
                     }
                 }
 

test/cli/test/coverage.test.ts

@@ -698,6 +698,32 @@ test("calls second", () => {
   expect(exitCode).toBe(0);
 });
 
+type Row = { functions: string; lines: string; uncovered: string };
+// The text reporter's row and the lcov record of every file in the report.
+async function run(fixture: Record<string, string>, args: string[], env: Record<string, string> = {}) {
+  using dir = tempDir("cov", fixture);
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "test", "--coverage", "--coverage-reporter=text", "--coverage-reporter=lcov", ...args],
+    env: { ...bunEnv, ...env },
+    cwd: String(dir),
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  if (exitCode !== 0) throw new Error(stderr);
+  const rows: Record<string, Row> = {};
+  for (const line of stderr.split("\n")) {
+    const [file, functions, lines, uncovered] = line.split("|").map(column => column.trim());
+    if (uncovered !== undefined) rows[file] = { functions, lines, uncovered };
+  }
+  const lcov: Record<string, string> = {};
+  for (const record of readFileSync(path.join(String(dir), "coverage", "lcov.info"), "utf-8").split("end_of_record")) {
+    const file = record.match(/^SF:(.+)$/m)?.[1];
+    if (file) lcov[file] = record;
+  }
+  return { rows, lcov, stdout };
+}
+
 // JSC records coverage per SourceProvider, and a file has one for each time
 // it is loaded. Each case has a subject file of its own. Where a case loads
 // its subject twice, one load runs the `if` branch and the other runs the
@@ -908,33 +934,6 @@ test("b", () => {
 `,
   };
 
-  type Row = { functions: string; lines: string; uncovered: string };
-  async function run(fixture: Record<string, string>, args: string[], env: Record<string, string> = {}) {
-    using dir = tempDir("cov-loaded-twice", fixture);
-    await using proc = Bun.spawn({
-      cmd: [bunExe(), "test", "--coverage", "--coverage-reporter=text", "--coverage-reporter=lcov", ...args],
-      env: { ...bunEnv, ...env },
-      cwd: String(dir),
-      stdout: "pipe",
-      stderr: "pipe",
-    });
-    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
-    if (exitCode !== 0) throw new Error(stderr);
-    const rows: Record<string, Row> = {};
-    for (const line of stderr.split("\n")) {
-      const [file, functions, lines, uncovered] = line.split("|").map(column => column.trim());
-      if (uncovered !== undefined) rows[file] = { functions, lines, uncovered };
-    }
-    const lcov: Record<string, string> = {};
-    for (const record of readFileSync(path.join(String(dir), "coverage", "lcov.info"), "utf-8").split(
-      "end_of_record",
-    )) {
-      const file = record.match(/^SF:(.+)$/m)?.[1];
-      if (file) lcov[file] = record;
-    }
-    return { rows, lcov, stdout };
-  }
-
   let loaded: Awaited<ReturnType<typeof run>>;
   let oneLoad: Awaited<ReturnType<typeof run>>;
   let pluginUnderIsolate: Awaited<ReturnType<typeof run>>;
@@ -1006,3 +1005,221 @@ test("b", () => {
     });
   });
 });
+
+// JSC runs a text that bun does not print as it is: a file whose first line starts with
+// `// @bun`, and CommonJS under a changed `Module.wrapper`. Such a text can end without a line
+// terminator. Under `bun test --coverage` it reports what the same text reports with a newline
+// after it.
+describe("a text that is not printed by bun", () => {
+  // [the text, FNF and FNH, whether lines at the end of the text ran]
+  // The report does not count the first byte of a line, so each last line here has more bytes.
+  const texts: Record<string, [string, string, Record<number, boolean>]> = {
+    "never-ran-alone-on-the-last-line.js": [
+      "// @bun\nexport function first() {\n  return 1;\n}\nexport const second = () => 2;",
+      "FNF:2\nFNH:1",
+      { 5: true },
+    ],
+    "all-on-the-last-line.js": [
+      "// @bun\nexport const first = () => 1; export const second = () => 2;",
+      "FNF:2\nFNH:1",
+      { 2: true },
+    ],
+    "branch-not-taken-on-the-last-line.js": [
+      "// @bun\nexport const first = () => 1;\nif (globalThis.neverSet)\n  first();",
+      "FNF:1\nFNH:1",
+      { 4: false },
+    ],
+    "last-line-of-two-bytes.js": [
+      "// @bun\nexport const first = () => {\n  return 1;\n};",
+      "FNF:1\nFNH:1",
+      { 4: true },
+    ],
+    "last-byte-closes-a-function-that-never-ran.js": [
+      "// @bun\nexport function first() {\n  return 1;\n}\nexport function second() {\n  return 2;\n  }",
+      "FNF:2\nFNH:1",
+      { 5: false, 6: false },
+    ],
+    "wrapper-on-the-last-line.cjs": [
+      "// @bun @bun-cjs\n(function(exports, require, module, __filename, __dirname) {exports.first = () => 1; exports.second = () => 2;})",
+      "FNF:3\nFNH:2",
+      { 2: true },
+    ],
+    // plugin.ts puts the `// @bun` line before this text, so the text that runs has five lines.
+    "from-a-plugin.js": [
+      "export function first() {\n  return 1;\n}\nexport const second = () => 2;",
+      "FNF:2\nFNH:1",
+      { 5: true },
+    ],
+  };
+  const terminated = (name: string) => `terminated-${name}`;
+  // The row of each of these names a line that did not run, so it shows that the last line is in the report.
+  const withRowFromBunJsc = ["branch-not-taken-on-the-last-line.js", "last-byte-closes-a-function-that-never-ran.js"];
+
+  let result: Awaited<ReturnType<typeof run>>;
+  beforeAll(async () => {
+    const files: Record<string, string> = {
+      // coverageIgnoreSourcemaps: the report of wrapped.cjs is then of the text that ran.
+      "bunfig.toml": `[test]\ncoverageSkipTestFiles = true\ncoverageIgnoreSourcemaps = true\npreload = ["./plugin.ts"]\n`,
+      "plugin.ts": `
+import { plugin } from "bun";
+
+plugin({
+  name: "pragma",
+  setup(build) {
+    build.onLoad({ filter: /from-a-plugin\\.js$/ }, async ({ path }) => {
+      return { contents: "// @bun\\n" + (await Bun.file(path).text()), loader: "js" };
+    });
+  },
+});
+`,
+      "empty.cjs": "",
+      // bun prints these two. The test puts a wrapper of its own around the printed text.
+      "wrapped.cjs": "exports.first = () => 1;\nexports.second = () => 2;\n",
+      [terminated("wrapped.cjs")]: "exports.first = () => 1;\nexports.second = () => 2;\n",
+      "ends-with-a-carriage-return.js": "// @bun\nexport const first = () => 1;\r",
+      "ends-with-a-newline.js": "// @bun\nexport const first = () => 1;\n",
+      "never-ran-on-no-line.js": "// @bun\n// \u00a9\u00a9\n\n\n\n\n()=>1;\nexport const first = () => 1;\n",
+    };
+    // Each text as it is, and with a final newline.
+    const names: string[] = [];
+    for (const [name, [text]] of Object.entries(texts)) {
+      files[name] = text;
+      files[terminated(name)] = text + "\n";
+      names.push(name, terminated(name));
+    }
+    files["first.test.ts"] = `
+import { expect, test } from "bun:test";
+import { codeCoverageForFile } from "bun:jsc";
+import Module from "node:module";
+import { join } from "node:path";
+${names.map((name, i) => (name.endsWith(".cjs") ? `const m${i} = require("./${name}");` : `import * as m${i} from "./${name}";`)).join("\n")}
+import * as carriageReturn from "./ends-with-a-carriage-return.js";
+import * as newline from "./ends-with-a-newline.js";
+import * as onNoLine from "./never-ran-on-no-line.js";
+require("./empty.cjs");
+
+test("calls first() and not second()", () => {
+  for (const module of [${names.map((_, i) => `m${i}`).join(", ")}, carriageReturn, newline, onNoLine]) {
+    expect(module.first()).toBe(1);
+  }
+  for (const name of ${JSON.stringify(withRowFromBunJsc.flatMap(name => [name, terminated(name)]))}) {
+    console.log(JSON.stringify({ name, row: codeCoverageForFile(join(import.meta.dir, name), true) }));
+  }
+});
+
+test("calls first() and not second() under a changed Module.wrapper", () => {
+  const defaultEnd = Module.wrapper[1];
+  const end = "\\n;var unused = function () {};})";
+  try {
+    Module.wrapper[1] = end;
+    expect(require("./wrapped.cjs").first()).toBe(1);
+    Module.wrapper[1] = end + "\\n";
+    expect(require("./${terminated("wrapped.cjs")}").first()).toBe(1);
+  } finally {
+    Module.wrapper[1] = defaultEnd;
+  }
+});
+`;
+    result = await run(files, []);
+  });
+
+  // The lcov record of a file without its name, and its table row.
+  const record = (file: string) => result.lcov[file].replace(`SF:${file}\n`, "").trim();
+  const report = (file: string) => ({ record: record(file), row: result.rows[file] });
+  // What codeCoverageForFile() returned in the test, without the name of the file.
+  const rowFromBunJsc = (file: string) =>
+    result.stdout
+      .split("\n")
+      .filter(line => line.startsWith("{"))
+      .map(line => JSON.parse(line))
+      .filter(({ name }) => name === file)
+      .map(({ row }) => row.slice(row.indexOf("|")))[0];
+
+  test.each(Object.entries(texts))("%s", (file, [, functions, lines]) => {
+    expect({
+      functions: record(file).match(/^FNF:\d+\nFNH:\d+$/m)?.[0],
+      ran: Object.fromEntries(
+        Array.from(record(file).matchAll(/^DA:(\d+),(\d+)$/gm), ([, line, hits]) => [line, hits !== "0"]),
+      ),
+      row: result.rows[file],
+    }).toEqual({
+      functions,
+      ran: expect.objectContaining(lines),
+      row: { functions: expect.any(String), lines: expect.any(String), uncovered: expect.any(String) },
+    });
+    expect(report(file)).toEqual(report(terminated(file)));
+  });
+
+  test.each(withRowFromBunJsc)("bun:jsc codeCoverageForFile() of %s", file => {
+    expect(rowFromBunJsc(file)).toMatch(/^\| +\d+\.\d+ \| +\d+\.\d+ \| \d/);
+    expect(rowFromBunJsc(file)).toBe(rowFromBunJsc(terminated(file)));
+  });
+
+  test("CommonJS under a changed Module.wrapper", () => {
+    // first(), second(), the wrapper, and the function in the end of the wrapper.
+    expect(result.rows["wrapped.cjs"]).toEqual({ functions: "50.00", lines: "100.00", uncovered: "" });
+    expect(report("wrapped.cjs")).toEqual(report(terminated("wrapped.cjs")));
+  });
+
+  test("the lcov record of a text with a final newline", () => {
+    expect(record(terminated("never-ran-alone-on-the-last-line.js"))).toContain(
+      "FNF:2\nFNH:1\nDA:2,10\nDA:3,9\nDA:4,1\nDA:5,21\nLF:4\nLH:4",
+    );
+  });
+
+  // A carriage return ends a line too, so this text runs as it is.
+  test("a text that ends with a carriage return", () => {
+    expect(record("ends-with-a-carriage-return.js")).toMatch(/^FNF:1\nFNH:1\nDA:2,\d+\nLF:1\nLH:1$/m);
+    expect(report("ends-with-a-carriage-return.js")).toEqual(report("ends-with-a-newline.js"));
+  });
+
+  // The text of an empty CommonJS file is a wrapper that bun writes. The file has no line to report.
+  test("an empty .cjs file reports no line", () => {
+    expect(record("empty.cjs")).toContain("FNF:1\nFNH:1\nLF:0\nLH:0");
+  });
+
+  // The line table has four more bytes than JSC has code units before the function on line 7. Each
+  // offset of that function is then the start of a blank line, and the report puts it on no line.
+  test("a function that never ran and that the line table puts on no line", () => {
+    expect(record("never-ran-on-no-line.js")).toContain("FNF:2\nFNH:1\n");
+  });
+
+  // Only `bun test --coverage` reads a text by line. A process that turns the profiler on with
+  // node:inspector runs the text as it is, as node does.
+  test("the text is as it is after Profiler.startPreciseCoverage", async () => {
+    const text = texts["never-ran-alone-on-the-last-line.js"][0];
+    using dir = tempDir("cov", {
+      "subject.js": text,
+      "main.mjs": `
+import { codeCoverageForFile } from "bun:jsc";
+import inspector from "node:inspector/promises";
+import { join } from "node:path";
+
+const session = new inspector.Session();
+session.connect();
+await session.post("Profiler.enable");
+await session.post("Profiler.startPreciseCoverage", { callCount: true, detailed: true });
+(await import("./subject.js")).first();
+const row = codeCoverageForFile(join(import.meta.dir, "subject.js"), true);
+const { result } = await session.post("Profiler.takePreciseCoverage");
+const { functions } = result.find(({ url }) => url.endsWith("subject.js"));
+const end = Math.max(...functions.flatMap(({ ranges }) => ranges.map(range => range.endOffset)));
+console.log(JSON.stringify({ row: row.slice(row.indexOf("|")), end }));
+`,
+    });
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "main.mjs"],
+      env: bunEnv,
+      cwd: String(dir),
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect(stderr).toBe("");
+    expect(JSON.parse(stdout)).toEqual({
+      row: expect.stringMatching(/^\| +50\.00 \| +100\.00 \| $/),
+      end: text.length,
+    });
+    expect(exitCode).toBe(0);
+  });
+});