From 8d29704d469c18a9ac3a04772c068677ffb0d646 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Thu, 5 Nov 2026 00:33:44 +0800 Subject: [PATCH] fix(run): merge Gradle console results into the reported test classes (#1323) Gradle's console names a test class by its simple name and a JUnit 5 method by its display name (`method()`), while its XML reports use the qualified class and the method name. After a run the test tree showed each class twice, counted the failure twice, and the report's copy of the failing test had no source location. Console results are now moved under the reported class (when the simple name is unambiguous), Gradle method names drop the empty `()`, and the report's stack frame is matched for Kotlin names with spaces and frames behind a class loader prefix (`app//`). --- src/features/run/java.test.ts | 18 ++++++++++++++ src/features/run/store.ts | 17 ++++++++++---- src/features/run/testing/jvm.ts | 20 +++++++++++++++- src/features/run/testing/parsers.test.ts | 30 +++++++++++++++++++++--- src/features/run/testing/results.ts | 20 ++++++++++++++++ 5 files changed, 96 insertions(+), 9 deletions(-) diff --git a/src/features/run/java.test.ts b/src/features/run/java.test.ts index d48d0151f..c486d7007 100644 --- a/src/features/run/java.test.ts +++ b/src/features/run/java.test.ts @@ -367,6 +367,24 @@ describe("JUnit XML reports", () => { location: { path: "a/B.java", line: 42, resolve: "name", className: "a.B$Inner" }, }); }); + + it("locate Kotlin tests with spaces in their names, behind a class loader prefix", () => { + const events = testReportEvents([ + { + className: "a.LibraryTest", + name: "cannot lend twice", + status: "failed", + durationMs: null, + message: null, + output: "AssertionFailedError\n\tat app//a.LibraryTest.cannot lend twice(LibraryTest.kt:43)", + }, + ]); + expect(events).toContainEqual({ + type: "location", + path: ["a.LibraryTest", "cannot lend twice"], + location: { path: "a/LibraryTest.kt", line: 43, resolve: "name", className: "a.LibraryTest" }, + }); + }); }); describe("temporary configurations", () => { diff --git a/src/features/run/store.ts b/src/features/run/store.ts index e91b56297..431b09c2b 100644 --- a/src/features/run/store.ts +++ b/src/features/run/store.ts @@ -42,6 +42,7 @@ import { type TestOutputParser, } from "./testing"; import { GoTestOutput } from "./testing/go"; +import { moveToReportedClasses } from "./testing/jvm"; import { TeamcityOutput } from "./testing/phpunit"; import type { TestStdout } from "./testing/types"; import { recordTestStates } from "./testing/states"; @@ -918,7 +919,14 @@ function finishSession(sessionId: string, code: number | null) { if (rt.tests && rt.parser) { rt.tests.apply(rt.parser.end?.() ?? []); // Surefire / Gradle print only failures and class totals: per-test results come from the XML reports. - if (frameworkFor(session.config) === "jvm") rt.tests.apply(await jvmReportEvents(session)); + if (frameworkFor(session.config) === "jvm") { + const reports = await jvmReports(session); + rt.tests.apply(testReportEvents(reports)); + moveToReportedClasses( + rt.tests, + reports.map((r) => r.className), + ); + } if (runtimes.get(sessionId) !== rt) return; rt.tests.finish(code); recordTestStates(rt.tests); @@ -935,12 +943,11 @@ function finishSession(sessionId: string, code: number | null) { }, 30); } -async function jvmReportEvents(session: RunSession): Promise { +async function jvmReports(session: RunSession): Promise { if (!isTauri()) return []; try { // Report files are written during the run; allow for coarse file-system timestamps. - const reports = await call(commands.runTestReports(session.config.cwd ?? null, session.startedAt - 2000)); - return testReportEvents(reports); + return await call(commands.runTestReports(session.config.cwd ?? null, session.startedAt - 2000)); } catch { return []; } @@ -958,7 +965,7 @@ export function testReportEvents(reports: readonly TestCaseReport[]): TestEvent[ for (const line of text.split("\n").slice(0, 200)) events.push({ type: "output", path, line }); const outer = r.className.split("$")[0]!; const frame = new RegExp( - `at (${outer.replace(/[.$]/g, "\\$&")}(?:\\$[\\w$]+)?)\\.[\\w$<>]+\\(([\\w$-]+\\.(?:java|kt|groovy|scala)):(\\d+)\\)`, + `at (?:\\S*/)?(${outer.replace(/[.$]/g, "\\$&")}(?:\\$[\\w$]+)?)\\.[^()\\n]+\\(([\\w$-]+\\.(?:java|kt|groovy|scala)):(\\d+)\\)`, ).exec(r.output ?? ""); if (frame) events.push({ type: "location", path, location: jvmFrameTarget(frame[1], frame[2]!, Number(frame[3])) }); } diff --git a/src/features/run/testing/jvm.ts b/src/features/run/testing/jvm.ts index b0344419d..71c6cf11e 100644 --- a/src/features/run/testing/jvm.ts +++ b/src/features/run/testing/jvm.ts @@ -1,5 +1,6 @@ /** Gradle test logging (`Class > method() PASSED`) and Maven Surefire reports. */ import { jvmFrameTarget } from "../links"; +import type { TestResults } from "./results"; import { parseDuration, type TestEvent, type TestOutputParser } from "./types"; const GRADLE_RE = /^(\S[^>]*?(?:\s>\s[^>]+?)+)\s+(PASSED|FAILED|SKIPPED)(?:\s+\(([\d.]+\s*m?s)\))?$/; @@ -29,7 +30,8 @@ export function jvmParser(): TestOutputParser { const events: TestEvent[] = []; let m: RegExpExecArray | null; if ((m = GRADLE_RE.exec(line.trim())) && !line.startsWith(" ")) { - const path = m[1]!.split(/\s+>\s+/); + // `method()` is a JUnit 5 display name; the reports name the method. + const path = m[1]!.split(/\s+>\s+/).map((p, i, all) => (i === all.length - 1 ? p.replace(/\(\)$/, "") : p)); const status = m[2] === "PASSED" ? "passed" : m[2] === "FAILED" ? "failed" : "skipped"; events.push({ type: "result", path, status, durationMs: parseDuration(m[3]) }); if (status === "failed") startFailure(path); @@ -99,3 +101,19 @@ export function jvmParser(): TestOutputParser { }, }; } + +/** + * Gradle's console names a test class by its simple name and its XML reports by the + * qualified one: moves the console's results to the reported class. + */ +export function moveToReportedClasses(tests: TestResults, classNames: readonly string[]) { + const bySimpleName = new Map(); + for (const c of new Set(classNames)) { + const simple = c.slice(c.lastIndexOf(".") + 1); + if (simple !== c) bySimpleName.set(simple, bySimpleName.has(simple) ? null : c); + } + for (const root of [...tests.roots]) { + const qualified = bySimpleName.get(root.name); + if (qualified) tests.move([root.name], [qualified]); + } +} diff --git a/src/features/run/testing/parsers.test.ts b/src/features/run/testing/parsers.test.ts index 95e15fbc4..db5e1f30c 100644 --- a/src/features/run/testing/parsers.test.ts +++ b/src/features/run/testing/parsers.test.ts @@ -13,6 +13,7 @@ import { TestResults, type TestOutputParser, } from "./index"; +import { moveToReportedClasses } from "./jvm"; function results(parser: TestOutputParser, text: string) { const r = new TestResults(); @@ -365,16 +366,39 @@ OrderControllerTest > Paging > firstPage() SKIPPED 3 tests completed, 1 failed, 1 skipped `, ); - expect(leaf(r, "OrderControllerTest", "shouldListOrders()")?.own).toBe("passed"); - expect(leaf(r, "OrderControllerTest", "shouldCreateOrder()")?.location).toEqual({ + expect(leaf(r, "OrderControllerTest", "shouldListOrders")?.own).toBe("passed"); + expect(leaf(r, "OrderControllerTest", "shouldCreateOrder")?.location).toEqual({ path: "OrderControllerTest.kt", line: 22, resolve: "name", }); - expect(leaf(r, "OrderControllerTest", "Paging", "firstPage()")?.own).toBe("skipped"); + expect(leaf(r, "OrderControllerTest", "Paging", "firstPage")?.own).toBe("skipped"); expect(r.summary).toMatchObject({ passed: 1, failed: 1, skipped: 1 }); }); + it("moves gradle's console results to the reported classes", () => { + const r = results( + jvmParser(), + `OrderTest > creates order() FAILED + org.opentest4j.AssertionFailedError at OrderTest.kt:22 +`, + ); + r.apply([ + { type: "result", path: ["com.acme.OrderTest", "lists"], status: "passed" }, + { type: "result", path: ["com.acme.OrderTest", "creates order"], status: "failed" }, + { type: "result", path: ["com.acme.UserTest", "loads"], status: "passed" }, + { type: "result", path: ["com.other.UserTest", "loads"], status: "passed" }, + ]); + moveToReportedClasses(r, ["com.acme.OrderTest", "com.acme.UserTest", "com.other.UserTest"]); + expect(r.roots.map((n) => n.name)).toEqual(["com.acme.OrderTest", "com.acme.UserTest", "com.other.UserTest"]); + const failed = leaf(r, "com.acme.OrderTest", "creates order")!; + expect(failed.own).toBe("failed"); + expect(failed.location).toMatchObject({ path: "OrderTest.kt", line: 22 }); + expect(failed.output).not.toEqual([]); + expect(r.counts()).toMatchObject({ passed: 3, failed: 1 }); + expect(r.get("OrderTest")).toBeUndefined(); + }); + it("parses surefire reports", () => { const r = results( jvmParser(), diff --git a/src/features/run/testing/results.ts b/src/features/run/testing/results.ts index 2e31412b5..468ff6364 100644 --- a/src/features/run/testing/results.ts +++ b/src/features/run/testing/results.ts @@ -81,6 +81,26 @@ export class TestResults { return best; } + /** Moves the subtree at `from` to `into`, merging it with what is already there. */ + move(from: string[], into: string[]) { + const source = this.byId.get(from.join(SEP)); + if (!source || from.join(SEP) === into.join(SEP)) return; + this.version++; + const parent = from.length > 1 ? this.byId.get(from.slice(0, -1).join(SEP))?.children : this.roots; + parent?.splice(parent.indexOf(source), 1); + this.mergeInto(source, into); + } + + private mergeInto(source: TestNode, into: string[]) { + this.byId.delete(source.id); + const target = this.node(into); + if (source.own && (!target.own || source.own === "failed")) target.own = source.own; + target.durationMs ??= source.durationMs; + target.location ??= source.location; + if (target.output.length === 0) target.output = source.output; + for (const child of source.children) this.mergeInto(child, [...into, child.name]); + } + apply(events: readonly TestEvent[]) { if (events.length === 0) return; this.version++;