-
Notifications
You must be signed in to change notification settings - Fork 764
Use go test -json in updateFailing, detect baseline errors
#2347
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this 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
execFileSynctospawnwith 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 |
|
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. |
|
No, this does seem to work, I just merged the two PRs together wrong. |
This does two things:
go test -jsonto stream the output in a parseable format.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.