Skip to content

Conversation

@jakebailey
Copy link
Member

This does two things:

  • Uses go test -json to stream the output in a parseable format.
  • With the parsed output, read each failing test's output to see if all of its failures are due to missing baselines.

We have no tests where this matters (I or others had fixed them previously), but I have a change where it does matter in the future.

Copilot AI review requested due to automatic review settings December 11, 2025 22:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR modernizes the updateFailing script by switching from synchronous test execution with regex parsing to streaming JSON output from go test -json. The key improvement is the ability to distinguish between baseline-only failures (which shouldn't mark tests as failing) and genuine test failures, enabling more accurate tracking of truly broken tests.

Key changes:

  • Migrated from execFileSync to spawn with streaming JSON output parsing
  • Added logic to differentiate baseline failures from real test errors by analyzing test output
  • Simplified the CI workflow by removing redundant test and baseline-accept steps

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
internal/fourslash/_scripts/updateFailing.mts Completely refactored from synchronous regex-based parsing to async streaming with JSON parsing, added baseline-only failure detection logic
.github/workflows/ci.yml Removed redundant baseline-accept and test steps that are now handled more efficiently by the updated script

@jakebailey jakebailey marked this pull request as draft December 12, 2025 00:28
@jakebailey
Copy link
Member Author

I need to think about this harder; I have to go manually merge this into one of my branches to get it to test so it's annoying to figure out.

@jakebailey jakebailey marked this pull request as ready for review December 12, 2025 05:41
@jakebailey
Copy link
Member Author

No, this does seem to work, I just merged the two PRs together wrong.

@jakebailey jakebailey requested a review from gabritto December 12, 2025 20:47
@jakebailey jakebailey requested a review from gabritto December 12, 2025 21:44
@jakebailey jakebailey enabled auto-merge December 12, 2025 21:48
@jakebailey jakebailey added this pull request to the merge queue Dec 12, 2025
Merged via the queue into main with commit 06abb77 Dec 12, 2025
22 checks passed
@jakebailey jakebailey deleted the jabaile/fourslash-failing-checker branch December 12, 2025 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants