-
Couldn't load subscription status.
- Fork 724
Use a different set of commands to detect fourslash test updates #1923
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
base: main
Are you sure you want to change the base?
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 updates the CI workflow to use a more comprehensive process for detecting and updating fourslash test changes. The new approach addresses an issue where failing tests leave behind baselines, which the existing updatefailing command doesn't properly handle.
- Replaces single
updatefailingcommand with a multi-step process - Clears failing test list, regenerates fourslash tests, and iteratively updates baselines
- Ensures all test state is properly synchronized by removing and regenerating fourslash baselines
.github/workflows/ci.yml
Outdated
| run: | | ||
| echo "" > ./internal/fourslash/_scripts/failingTests.txt | ||
| npm run convertfourslash | ||
| npx hereby test | ||
| npx hereby baseline-accept | ||
| npm run updatefailing | ||
| npx hereby baseline-accept | ||
| rm -rf testdata/baselines/reference/fourslash | ||
| npx hereby test |
Copilot
AI
Oct 22, 2025
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.
This complex multi-step process lacks inline comments explaining why each step is necessary. Add comments documenting the purpose of each phase (e.g., why tests are run multiple times, why fourslash baselines are removed and regenerated) to help maintainers understand this workaround.
| run: | | |
| echo "" > ./internal/fourslash/_scripts/failingTests.txt | |
| npm run convertfourslash | |
| npx hereby test | |
| npx hereby baseline-accept | |
| npm run updatefailing | |
| npx hereby baseline-accept | |
| rm -rf testdata/baselines/reference/fourslash | |
| npx hereby test | |
| run: | | |
| # Clear the list of failing fourslash tests to start fresh. | |
| echo "" > ./internal/fourslash/_scripts/failingTests.txt | |
| # Convert fourslash test sources to the latest format. | |
| npm run convertfourslash | |
| # Run all tests to generate new baselines. | |
| npx hereby test | |
| # Accept all generated baselines to update reference outputs. | |
| npx hereby baseline-accept | |
| # Update the list of failing tests after baseline update. | |
| npm run updatefailing | |
| # Accept any new baselines generated by updatefailing. | |
| npx hereby baseline-accept | |
| # Remove existing fourslash baselines to force full regeneration. | |
| rm -rf testdata/baselines/reference/fourslash | |
| # Run tests again to regenerate all fourslash baselines from scratch. | |
| npx hereby test | |
| # Accept the freshly regenerated baselines. |
This is the weird set of commands I have been using locally to keep things up to date while working around the problem of "failing tests" leaving baselines behind. It seems like
updatefailingshould do something like this, but I think it might be too annoying.(Sending first without the baseline update to show that it works.)