From f3705f809b248f618c614b8b3b39b07232ba2ba0 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Fri, 21 Nov 2025 13:21:19 -0800 Subject: [PATCH 1/5] Better updateFailing checker --- internal/fourslash/_scripts/updateFailing.mts | 130 ++++++++++++++++-- 1 file changed, 115 insertions(+), 15 deletions(-) diff --git a/internal/fourslash/_scripts/updateFailing.mts b/internal/fourslash/_scripts/updateFailing.mts index 98a20d8a05..855a35ae60 100644 --- a/internal/fourslash/_scripts/updateFailing.mts +++ b/internal/fourslash/_scripts/updateFailing.mts @@ -1,36 +1,136 @@ import * as cp from "child_process"; import * as fs from "fs"; import path from "path"; +import * as readline from "readline"; import which from "which"; const failingTestsPath = path.join(import.meta.dirname, "failingTests.txt"); -function main() { +interface TestEvent { + Time?: string; + Action: string; + Package?: string; + Test?: string; + Output?: string; + Elapsed?: number; +} + +async function main() { + const oldFailingTests = fs.readFileSync(failingTestsPath, "utf-8"); const go = which.sync("go"); - let testOutput: string; + + let testProcess: cp.ChildProcess; try { // Run tests with TSGO_FOURSLASH_IGNORE_FAILING=1 to run all tests including those in failingTests.txt - testOutput = cp.execFileSync(go, ["test", "-v", "./internal/fourslash/tests/gen"], { - encoding: "utf-8", + testProcess = cp.spawn(go, ["test", "-json", "./internal/fourslash/tests/gen"], { + stdio: ["ignore", "pipe", "pipe"], env: { ...process.env, TSGO_FOURSLASH_IGNORE_FAILING: "1" }, }); } catch (error) { - testOutput = (error as { stdout: string; }).stdout as string; + fs.writeFileSync(failingTestsPath, oldFailingTests, "utf-8"); + throw new Error("Failed to spawn test process: " + error); } - const panicRegex = /^panic/m; - if (panicRegex.test(testOutput)) { - throw new Error("Unrecovered panic detected in tests\n" + testOutput); + + if (!testProcess.stdout || !testProcess.stderr) { + throw new Error("Test process stdout or stderr is null"); } - const failRegex = /--- FAIL: ([\S]+)/gm; + const failingTests: string[] = []; - let match; + const testOutputs = new Map(); + const allOutputs: string[] = []; + let hadPanic = false; - while ((match = failRegex.exec(testOutput)) !== null) { - failingTests.push(match[1]); - } + const rl = readline.createInterface({ + input: testProcess.stdout, + crlfDelay: Infinity, + }); + + rl.on("line", line => { + try { + const event: TestEvent = JSON.parse(line); + + // Collect output for each test + if (event.Action === "output" && event.Output) { + allOutputs.push(event.Output); + if (event.Test) { + if (!testOutputs.has(event.Test)) { + testOutputs.set(event.Test, []); + } + testOutputs.get(event.Test)!.push(event.Output); + } + + // Check for panics + if (/^panic/m.test(event.Output)) { + hadPanic = true; + } + } + + // Process failed tests + if (event.Action === "fail" && event.Test) { + const outputs = testOutputs.get(event.Test) || []; + const fullOutput = outputs.join(""); - fs.writeFileSync(failingTestsPath, failingTests.sort((a, b) => a.localeCompare(b, "en-US")).join("\n") + "\n", "utf-8"); + // Check if this is a baseline-only failure + // Baseline failures contain specific messages from baseline.go + const hasBaselineMessage = /new baseline created at/.test(fullOutput) || + /the baseline file .* has changed/.test(fullOutput); + + // Check for non-baseline errors + // Look for patterns that indicate real test failures + // We need to filter out baseline messages when checking for errors + const outputWithoutBaseline = fullOutput + .replace(/the baseline file .* has changed\. \(Run `hereby baseline-accept` if the new baseline is correct\.\)/g, "") + .replace(/new baseline created at .*\./g, "") + .replace(/the baseline file .* does not exist in the TypeScript submodule/g, "") + .replace(/the baseline file .* does not match the reference in the TypeScript submodule/g, ""); + + const hasNonBaselineError = /^panic/m.test(outputWithoutBaseline) || + /Error|error/i.test(outputWithoutBaseline) || + /fatal|Fatal/.test(outputWithoutBaseline) || + /Unexpected/.test(outputWithoutBaseline); + + // Only mark as failing if it's not a baseline-only failure + // (i.e., if there's no baseline message, or if there are other errors besides baseline) + if (!hasBaselineMessage || hasNonBaselineError) { + failingTests.push(event.Test); + } + } + } + catch (e) { + // Not JSON, possibly stderr or other output - ignore + } + }); + + testProcess.stderr.on("data", data => { + // Check stderr for panics too + const output = data.toString(); + allOutputs.push(output); + if (/^panic/m.test(output)) { + hadPanic = true; + } + }); + + await new Promise((resolve, reject) => { + testProcess.on("close", code => { + if (hadPanic) { + fs.writeFileSync(failingTestsPath, oldFailingTests, "utf-8"); + reject(new Error("Unrecovered panic detected in tests\n" + allOutputs.join(""))); + return; + } + + fs.writeFileSync(failingTestsPath, failingTests.sort((a, b) => a.localeCompare(b, "en-US")).join("\n") + "\n", "utf-8"); + resolve(); + }); + + testProcess.on("error", error => { + fs.writeFileSync(failingTestsPath, oldFailingTests, "utf-8"); + reject(error); + }); + }); } -main(); +main().catch(error => { + console.error("Error:", error); + process.exit(1); +}); From c9e663812b193c60c3adf8149246bd4bd8e2b427 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 11 Dec 2025 14:26:04 -0800 Subject: [PATCH 2/5] Do less in CI --- .github/workflows/ci.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 571ac187e5..80d46786f3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -264,12 +264,8 @@ jobs: - name: Regenerate fourslash tests and update failing test list run: | set -x - echo "" > ./internal/fourslash/_scripts/failingTests.txt npm run convertfourslash >/dev/null 2>&1 || true - npx hereby test >/dev/null || true - npx hereby baseline-accept || true npm run updatefailing >/dev/null 2>&1 || true - npx hereby baseline-accept || true rm -rf testdata/baselines/reference/fourslash || true npx hereby test >/dev/null || true npx hereby baseline-accept || true From b5e386cb4c1229059647ad9a5005131f5f4f2938 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 11 Dec 2025 16:18:42 -0800 Subject: [PATCH 3/5] Stricter --- internal/fourslash/_scripts/updateFailing.mts | 44 +++++++++---------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/internal/fourslash/_scripts/updateFailing.mts b/internal/fourslash/_scripts/updateFailing.mts index 855a35ae60..38bf62e89e 100644 --- a/internal/fourslash/_scripts/updateFailing.mts +++ b/internal/fourslash/_scripts/updateFailing.mts @@ -69,30 +69,26 @@ async function main() { // Process failed tests if (event.Action === "fail" && event.Test) { const outputs = testOutputs.get(event.Test) || []; - const fullOutput = outputs.join(""); - - // Check if this is a baseline-only failure - // Baseline failures contain specific messages from baseline.go - const hasBaselineMessage = /new baseline created at/.test(fullOutput) || - /the baseline file .* has changed/.test(fullOutput); - - // Check for non-baseline errors - // Look for patterns that indicate real test failures - // We need to filter out baseline messages when checking for errors - const outputWithoutBaseline = fullOutput - .replace(/the baseline file .* has changed\. \(Run `hereby baseline-accept` if the new baseline is correct\.\)/g, "") - .replace(/new baseline created at .*\./g, "") - .replace(/the baseline file .* does not exist in the TypeScript submodule/g, "") - .replace(/the baseline file .* does not match the reference in the TypeScript submodule/g, ""); - - const hasNonBaselineError = /^panic/m.test(outputWithoutBaseline) || - /Error|error/i.test(outputWithoutBaseline) || - /fatal|Fatal/.test(outputWithoutBaseline) || - /Unexpected/.test(outputWithoutBaseline); - - // Only mark as failing if it's not a baseline-only failure - // (i.e., if there's no baseline message, or if there are other errors besides baseline) - if (!hasBaselineMessage || hasNonBaselineError) { + + // A test is only considered a baseline-only failure if ALL error messages + // are baseline-related. Any non-baseline error message means it's a real failure. + const baselineMessagePatterns = [ + /^\s*baseline\.go:\d+: the baseline file .* has changed\./, + /^\s*baseline\.go:\d+: new baseline created at /, + /^\s*baseline\.go:\d+: the baseline file .* does not exist in the TypeScript submodule/, + /^\s*baseline\.go:\d+: the baseline file .* does not match the reference in the TypeScript submodule/, + ]; + + // Check each output line that looks like an error message + // Error messages from Go tests typically contain ".go:" with a line number + const errorLines = outputs.filter(line => /^\s*\w+\.go:\d+:/.test(line)); + + // If there are no error lines, it's a real failure. + // If all error lines match baseline patterns, it's a baseline-only failure + const isBaselineOnlyFailure = errorLines.length > 0 && + errorLines.every(line => baselineMessagePatterns.some(pattern => pattern.test(line))); + + if (!isBaselineOnlyFailure) { failingTests.push(event.Test); } } From 3e3d17a5f4e7203779955b0e3376b88d9775dc46 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Fri, 12 Dec 2025 10:14:33 -0800 Subject: [PATCH 4/5] includes --- internal/fourslash/_scripts/updateFailing.mts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/fourslash/_scripts/updateFailing.mts b/internal/fourslash/_scripts/updateFailing.mts index df4942d5a3..931ac68c94 100644 --- a/internal/fourslash/_scripts/updateFailing.mts +++ b/internal/fourslash/_scripts/updateFailing.mts @@ -73,7 +73,7 @@ async function main() { const outputs = testOutputs.get(event.Test) || []; // Check if this is a crashing test (contains InternalError) - const hasCrash = outputs.some(line => /InternalError/.test(line)); + const hasCrash = outputs.some(line => line.includes("InternalError")); if (hasCrash) { crashingTests.push(event.Test); } From 8ea5b685cda1cc1cbc335afbc57339b54e0190c8 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Fri, 12 Dec 2025 13:44:29 -0800 Subject: [PATCH 5/5] Remove oops --- internal/fourslash/_scripts/updateFailing.mts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/fourslash/_scripts/updateFailing.mts b/internal/fourslash/_scripts/updateFailing.mts index 931ac68c94..1deee36182 100644 --- a/internal/fourslash/_scripts/updateFailing.mts +++ b/internal/fourslash/_scripts/updateFailing.mts @@ -17,7 +17,6 @@ interface TestEvent { } async function main() { - const oldFailingTests = fs.readFileSync(failingTestsPath, "utf-8"); const go = which.sync("go"); let testProcess: cp.ChildProcess; @@ -29,7 +28,6 @@ async function main() { }); } catch (error) { - fs.writeFileSync(failingTestsPath, oldFailingTests, "utf-8"); throw new Error("Failed to spawn test process: " + error); } @@ -118,7 +116,6 @@ async function main() { await new Promise((resolve, reject) => { testProcess.on("close", code => { if (hadPanic) { - fs.writeFileSync(failingTestsPath, oldFailingTests, "utf-8"); reject(new Error("Unrecovered panic detected in tests\n" + allOutputs.join(""))); return; } @@ -129,7 +126,6 @@ async function main() { }); testProcess.on("error", error => { - fs.writeFileSync(failingTestsPath, oldFailingTests, "utf-8"); reject(error); }); });