bun test: one location per test for TestReporter.found and JUnit

docs/test/reporters.mdx

@@ -105,6 +105,15 @@ The report then has one `<testsuite>` per file. Each test's name is prefixed wit
 </testsuite>
 ```
 
+#### Line Numbers in JUnit Reports
+
+The `line` of a `<testcase>`, and of the `<testsuite>` of a `describe` block, is the line of the call in the test file that registered it:
+
+- A test that a helper in another module registers has the line of the call to the helper.
+- A test that no call in the test file leads to has the line of the `describe` block it is in. An example is a test in a `describe` callback that another module defines.
+
+The attribute is absent when the test is in no `describe` block and no call in the test file leads to it, such as a test that an imported module registers while it loads.
+
 #### Retried tests
 
 A test with [`retry`](/test/writing-tests#retries-and-repeats) is one `<testcase>`, with its final result. The attempts before the last one are in the elements that Maven Surefire uses, which CI tools read to find flaky tests:
@@ -168,3 +177,7 @@ Custom reporters can listen for these events:
 - `TestReporter.end`: Emitted when a test completes
 - `Console.messageAdded`: Emitted when console output occurs during a test
 - `LifecycleReporter.error`: Emitted when an error or exception occurs
+
+A client gets every event of a run if it starts `bun test` with `--inspect-wait` and sends `TestReporter.enable` before `Inspector.initialized`. A client that enables the domain later gets `found` for the tests of the file that is running, and nothing for the files that have ended.
+
+`TestReporter.found` has the `url` of the test file and a `line`. They are the `file` and `line` of the [JUnit report](#line-numbers-in-junit-reports), not the place of the test's callback. `line` is `0` where the report has no `line`. It is also `0` for a test that was registered before `TestReporter.enable` arrived, if the process wasn't started with `--inspect`, `--inspect-wait`, `--inspect-brk`, `BUN_INSPECT` or `--reporter=junit`. An example is a client that connects through `BUN_INSPECT_CONNECT_TO`.

src/jsc/CallFrame.rs

@@ -1,5 +1,6 @@
 use alloc::borrow::Cow;
 use core::ffi::c_uint;
+use core::num::NonZeroU32;
 
 use crate::virtual_machine::VirtualMachine;
 use crate::{JSGlobalObject, JSValue};
@@ -47,6 +48,11 @@ impl JSGlobalObject {
         JSC__JSGlobalObject__getCallerSrcLoc(self, &mut str, &mut line, &mut column);
         CallerSrcLoc { str, line, column }
     }
+
+    /// The line of the innermost call that `file` makes on the stack, if it makes one.
+    pub fn get_caller_line_in_file(&self, file: &bun_core::String) -> Option<NonZeroU32> {
+        NonZeroU32::new(JSC__JSGlobalObject__getCallerLineInFile(self, file))
+    }
 }
 
 /// This is an advanced iterator struct which is used by various APIs. In
@@ -117,6 +123,10 @@ unsafe extern "C" {
         out_line: &mut c_uint,
         out_column: &mut c_uint,
     );
+    safe fn JSC__JSGlobalObject__getCallerLineInFile(
+        global: &JSGlobalObject,
+        file: &bun_core::String,
+    ) -> c_uint;
     #[cfg(debug_assertions)]
     fn Bun__CallFrame__describeFrame(cf: *const CallFrame) -> *const core::ffi::c_char;
 }

src/jsc/Debugger.rs

@@ -9,6 +9,7 @@
 
 use core::cell::Cell;
 use core::ffi::{c_int, c_void};
+use core::num::NonZeroU32;
 use core::sync::atomic::{AtomicBool, AtomicU32, Ordering};
 
 use bun_core::{String as BunString, StringView};
@@ -830,13 +831,6 @@ bun_opaque::opaque_ffi! { pub struct TestReporterHandle; }
 // in/out-param the C++ side reads/consumes in-place. Remaining args are
 // by-value scalars.
 unsafe extern "C" {
-    safe fn Bun__TestReporterAgentReportTestFound(
-        agent: &mut TestReporterHandle,
-        test_id: c_int,
-        name: &BunString,
-        item_type: TestType,
-        parent_id: c_int,
-    );
     safe fn Bun__TestReporterAgentReportTestFoundWithLocation(
         agent: &mut TestReporterHandle,
         test_id: c_int,
@@ -856,27 +850,24 @@ unsafe extern "C" {
 }
 
 impl TestReporterHandle {
-    pub(crate) fn report_test_found(
-        &mut self,
-        test_id: i32,
-        name: &BunString,
-        item_type: TestType,
-        parent_id: i32,
-    ) {
-        Bun__TestReporterAgentReportTestFound(self, test_id, name, item_type, parent_id);
-    }
-
+    /// The protocol cannot leave `line` out, so an unknown line is sent as 0.
     pub fn report_test_found_with_location(
         &mut self,
         test_id: i32,
         name: &BunString,
         item_type: TestType,
         parent_id: i32,
         source_url: &BunString,
-        line: i32,
+        line: Option<NonZeroU32>,
     ) {
         Bun__TestReporterAgentReportTestFoundWithLocation(
-            self, test_id, name, item_type, parent_id, source_url, line,
+            self,
+            test_id,
+            name,
+            item_type,
+            parent_id,
+            source_url,
+            line.map_or(0, |line| line.get() as c_int),
         );
     }
 
@@ -942,20 +933,18 @@ impl TestReporterAgent {
     }
 
     /// Caller must ensure that it is enabled first.
-    ///
-    /// Since we may have to call .deinit on the name string.
-    ///
-    /// The test's location is that of the caller of the host function that is running.
     pub fn report_test_found(
         &self,
         test_id: i32,
         name: &BunString,
         item_type: TestType,
         parent_id: i32,
+        source_url: &BunString,
+        line: Option<NonZeroU32>,
     ) {
         bun_core::scoped_log!(TestReporterAgent, "reportTestFound");
         self.handle_mut()
-            .report_test_found(test_id, name, item_type, parent_id);
+            .report_test_found_with_location(test_id, name, item_type, parent_id, source_url, line);
     }
 
     /// Caller must ensure that it is enabled first.

src/jsc/bindings/InspectorTestReporterAgent.cpp

@@ -7,7 +7,6 @@
 #include <JavaScriptCore/ScriptCallStack.h>
 #include <JavaScriptCore/JSGlobalObjectDebuggable.h>
 #include <JavaScriptCore/JSGlobalObjectInspectorController.h>
-#include "ErrorStackTrace.h"
 #include "ZigGlobalObject.h"
 
 #include "ModuleLoader.h"
@@ -31,25 +30,6 @@ enum class BunTestType : uint8_t {
     Describe,
 };
 
-void Bun__TestReporterAgentReportTestFound(Inspector::InspectorTestReporterAgent* agent, int testId, const BunString* name, BunTestType item_type, int parentId)
-{
-    auto str = name->toWTFString(BunString::ZeroCopy);
-
-    Protocol::TestReporter::TestType type;
-    switch (item_type) {
-    case BunTestType::Test:
-        type = Protocol::TestReporter::TestType::Test;
-        break;
-    case BunTestType::Describe:
-        type = Protocol::TestReporter::TestType::Describe;
-        break;
-    default:
-        ASSERT_NOT_REACHED();
-    }
-
-    agent->reportTestFound(testId, str, type, parentId);
-}
-
 void Bun__TestReporterAgentReportTestFoundWithLocation(Inspector::InspectorTestReporterAgent* agent, int testId, const BunString* name, BunTestType item_type, int parentId, const BunString* sourceURL, int line)
 {
     auto str = name->toWTFString(BunString::ZeroCopy);
@@ -159,82 +139,14 @@ Protocol::ErrorStringOr<void> InspectorTestReporterAgent::disable()
     return {};
 }
 
-void InspectorTestReporterAgent::reportTestFound(int testId, const String& name, Protocol::TestReporter::TestType type, int parentId)
-{
-    if (!m_enabled)
-        return;
-
-    JSC::LineColumn lineColumn;
-    JSC::SourceID sourceID = 0;
-    String sourceURL;
-
-    auto* globalObject = &m_globalObject;
-    auto& vm = JSC::getVM(globalObject);
-
-    JSC::StackVisitor::visit(vm.topCallFrame, vm, [&](JSC::StackVisitor& visitor) -> WTF::IterationStatus {
-        if (Zig::isImplementationVisibilityPrivate(visitor))
-            return WTF::IterationStatus::Continue;
-
-        if (visitor->hasLineAndColumnInfo()) {
-            lineColumn = visitor->computeLineAndColumn();
-
-            String sourceURLForFrame = Zig::sourceURL(visitor);
-
-            // Sometimes, the sourceURL is empty.
-            // For example, pages in Next.js.
-            if (auto* codeBlock = sourceURLForFrame.isEmpty() ? visitor->codeBlock() : nullptr) {
-                const auto& source = codeBlock->source();
-
-                // source.isNull() is true when the SourceProvider is a null pointer.
-                if (!source.isNull()) {
-                    auto* provider = source.provider();
-                    sourceID = provider->asID();
-                }
-            }
-
-            sourceURL = sourceURLForFrame;
-
-            return WTF::IterationStatus::Done;
-        }
-
-        return WTF::IterationStatus::Continue;
-    });
-
-    if (!sourceURL.isEmpty() and lineColumn.line > 0) {
-        OrdinalNumber originalLine = OrdinalNumber::fromOneBasedInt(lineColumn.line);
-        OrdinalNumber originalColumn = OrdinalNumber::fromOneBasedInt(lineColumn.column);
-
-        Bun::OwnedZigStackFrames remappedFrames(1);
-        ZigStackFrame& remappedFrame = remappedFrames[0];
-        remappedFrame.position.line_zero_based = originalLine.zeroBasedInt();
-        remappedFrame.position.column_zero_based = originalColumn.zeroBasedInt();
-        remappedFrame.source_url = Bun::toStringRef(sourceURL);
-
-        remappedFrames.remap(Bun::vm(globalObject));
-
-        sourceURL = remappedFrame.source_url.toWTFString();
-        lineColumn.line = OrdinalNumber::fromZeroBasedInt(remappedFrame.position.line_zero_based).oneBasedInt();
-        lineColumn.column = OrdinalNumber::fromZeroBasedInt(remappedFrame.position.column_zero_based).oneBasedInt();
-    }
-
-    m_frontendDispatcher->found(
-        testId,
-        sourceID > 0 ? String::number(sourceID) : String(),
-        sourceURL,
-        lineColumn.line,
-        name,
-        type,
-        parentId > 0 ? parentId : std::optional<int>());
-}
-
 void InspectorTestReporterAgent::reportTestFoundWithLocation(int testId, const String& name, Protocol::TestReporter::TestType type, int parentId, const String& sourceURL, int line)
 {
     if (!m_enabled)
         return;
 
     m_frontendDispatcher->found(
         testId,
-        String(), // sourceID - not available for retroactively reported tests
+        String(), // scriptId. The url is always sent.
         sourceURL,
         line,
         name,

src/jsc/bindings/InspectorTestReporterAgent.h

@@ -36,7 +36,6 @@ class InspectorTestReporterAgent final : public InspectorAgentBase, public Inspe
     virtual Protocol::ErrorStringOr<void> disable() final;
 
     // Public API for reporting test events
-    void reportTestFound(int testId, const String& name, Protocol::TestReporter::TestType type = Protocol::TestReporter::TestType::Test, int parentId = -1);
     void reportTestFoundWithLocation(int testId, const String& name, Protocol::TestReporter::TestType type, int parentId, const String& sourceURL, int line);
     void reportTestStart(int testId);
     void reportTestEnd(int testId, Protocol::TestReporter::TestStatus status, double elapsed);

src/jsc/bindings/bindings.cpp

@@ -7504,41 +7504,56 @@ CPP_DECL [[ZIG_EXPORT(check_slow)]] void Bun__JSValue__setPrototypeDirect(JSC::E
     return;
 }
 
-CPP_DECL [[ZIG_EXPORT(nothrow)]] unsigned int JSC__JSGlobalObject__getCallerLineNumber(JSC::JSGlobalObject* globalObject)
+// The line of the innermost call that `file` makes on the stack, or 0 if no frame of `file` is on it.
+CPP_DECL unsigned int JSC__JSGlobalObject__getCallerLineInFile(JSC::JSGlobalObject* globalObject, const BunString* file)
 {
     auto& vm = JSC::getVM(globalObject);
+    String sourceURL = file->toWTFString(BunString::ZeroCopy);
     JSC::LineColumn lineColumn;
-    String sourceURL;
+    bool calleeEndsInTemplate = false;
 
     JSC::StackVisitor::visit(vm.topCallFrame, vm, [&](JSC::StackVisitor& visitor) -> WTF::IterationStatus {
-        if (Zig::isImplementationVisibilityPrivate(visitor))
+        if (Zig::isImplementationVisibilityPrivate(visitor) || !visitor->hasLineAndColumnInfo())
             return WTF::IterationStatus::Continue;
 
-        if (visitor->hasLineAndColumnInfo()) {
-            String currentSourceURL = Zig::sourceURL(visitor);
+        // Not Zig::sourceURL(): it prefers the `//# sourceURL=` comment of the debugger, which is not the path if the path is not ASCII.
+        auto* codeBlock = visitor->codeBlock();
+        auto* provider = codeBlock && codeBlock->ownerExecutable() ? codeBlock->source().provider() : nullptr;
+        if ((provider ? provider->sourceURL() : Zig::sourceURL(visitor)) != sourceURL)
+            return WTF::IterationStatus::Continue;
 
-            if (!currentSourceURL.startsWith("builtin://"_s) && !currentSourceURL.startsWith("node:"_s)) {
-                lineColumn = visitor->computeLineAndColumn();
-                sourceURL = currentSourceURL;
-                return WTF::IterationStatus::Done;
-            }
+        lineColumn = visitor->computeLineAndColumn();
+        if (provider && lineColumn.line) {
+            StringView text = provider->source();
+            unsigned parenthesis = provider->offsetForPosition(OrdinalNumber::fromOneBasedInt(lineColumn.line).zeroBasedInt(), OrdinalNumber::fromOneBasedInt(lineColumn.column).zeroBasedInt());
+            calleeEndsInTemplate = parenthesis && parenthesis <= text.length() && text[parenthesis - 1] == '`';
         }
-        return WTF::IterationStatus::Continue;
+        return WTF::IterationStatus::Done;
     });
 
-    if (!sourceURL.isEmpty() && lineColumn.line > 0) {
+    if (!lineColumn.line)
+        return 0;
+
+    auto lineOfColumn = [&](int column) -> unsigned {
         Bun::OwnedZigStackFrames remappedFrames(1);
         ZigStackFrame& remappedFrame = remappedFrames[0];
-        remappedFrame.position.line_zero_based = lineColumn.line - 1;
-        remappedFrame.position.column_zero_based = lineColumn.column;
+        remappedFrame.position.line_zero_based = OrdinalNumber::fromOneBasedInt(lineColumn.line).zeroBasedInt();
+        remappedFrame.position.column_zero_based = column;
         remappedFrame.source_url = Bun::toStringRef(sourceURL);
 
         remappedFrames.remap(Bun::vm(globalObject));
 
-        return remappedFrame.position.line_zero_based + 1;
-    }
+        // The source map of a bundle names the files that the bundle was made from. The caller pairs the line with `file`.
+        if (remappedFrame.source_url.toWTFString(BunString::ZeroCopy) != sourceURL)
+            return lineColumn.line;
+
+        return OrdinalNumber::fromZeroBasedInt(remappedFrame.position.line_zero_based).oneBasedInt();
+    };
 
-    return lineColumn.line;
+    // After the tagged template of a `test.each`, the mapping before the `(` is a row of the table and the next one is the first argument.
+    int parenthesis = OrdinalNumber::fromOneBasedInt(lineColumn.column).zeroBasedInt();
+    unsigned line = lineOfColumn(parenthesis);
+    return calleeEndsInTemplate ? std::max(line, lineOfColumn(parenthesis + 1)) : line;
 }
 
 // REPL evaluation function - evaluates JavaScript code in the global scope

src/runtime/cli/test/parallel/aggregate.rs

@@ -40,7 +40,7 @@ pub(crate) fn replay_test_records(coord: &mut Coordinator) {
                 status: TestResult::Fail,
                 assertions: 0,
                 elapsed_ns: 0,
-                line_number: 0,
+                line_number: None,
                 failure: Some(TestFailure {
                     message: message.to_vec(),
                     ..Default::default()

src/runtime/cli/test/parallel/runner.rs

@@ -4,6 +4,7 @@
 //! from stdin, runs each file under isolation, and streams results to fd 3).
 
 use core::ffi::c_char;
+use core::num::NonZeroU32;
 use core::ptr::NonNull;
 use std::io::Write as _;
 
@@ -1029,12 +1030,12 @@ fn encode_test_case(wf: &mut Frame, t: &test_command::TestCaseReport<'_>) {
     wf.u32(t.status as u32);
     wf.u32(t.assertions);
     wf.u64(t.elapsed_ns);
-    wf.u32(t.line_number);
+    wf.u32(t.line_number.map_or(0, NonZeroU32::get));
     wf.str(t.name);
     wf.u32(u32::try_from(t.scopes.len()).expect("int cast"));
     for &(name, line) in &t.scopes {
         wf.str(name);
-        wf.u32(line);
+        wf.u32(line.map_or(0, NonZeroU32::get));
     }
     // Earlier attempts' failures first, then the last attempt's if it failed.
     wf.u32(u32::from(t.failure.is_some()));
@@ -1058,15 +1059,15 @@ pub(crate) fn decode_test_case<'a>(
         Result::from_repr(u8::try_from(rd.u32()).ok()?).filter(|s| *s != Result::Pending)?;
     let assertions = rd.u32();
     let elapsed_ns = rd.u64();
-    let line_number = rd.u32();
+    let line_number = NonZeroU32::new(rd.u32());
     let name = rd.str();
     let n = rd.u32() as usize;
     if n > 64 {
         return None;
     }
     let mut scopes = Vec::with_capacity(n);
     for _ in 0..n {
-        scopes.push((rd.str(), rd.u32()));
+        scopes.push((rd.str(), NonZeroU32::new(rd.u32())));
     }
     let has_failure = rd.u32() != 0;
     let earlier = rd.u32() as usize;