Skip to content
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

Fix: verification of incremental correctness that was not working because of using wrote writeFile #48751

Merged
merged 12 commits into from
Apr 22, 2022

Conversation

sheetalkamat
Copy link
Member

@sheetalkamat sheetalkamat commented Apr 18, 2022

This ensures that we use sys.writeFile for baseline buildinfo so that buildInfo correctness is verified. It was missing when we started passing the writeFileCallback which didnt track written files.
This also baselines those differences so its easy to view, read and maintain.

This is on top of #48703. Actual change is only 386786c and f45e9ca

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 18, 2022
@sheetalkamat sheetalkamat changed the base branch from moreBaselines to main April 20, 2022 17:22
Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

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

So the tests used to call the unpatched writeFile and that was always wrong? They were, apparently, doing this because the assert was too strict?

src/testRunner/unittests/tsbuild/noEmitOnError.ts Outdated Show resolved Hide resolved
src/testRunner/unittests/tsbuild/helpers.ts Outdated Show resolved Hide resolved
src/testRunner/unittests/tsbuild/helpers.ts Outdated Show resolved Hide resolved
src/testRunner/unittests/tsbuild/helpers.ts Outdated Show resolved Hide resolved
src/testRunner/unittests/tsc/helpers.ts Outdated Show resolved Hide resolved
src/testRunner/unittests/tsc/incremental.ts Outdated Show resolved Hide resolved
src/testRunner/unittests/tsbuild/helpers.ts Outdated Show resolved Hide resolved
@amcasey
Copy link
Member

amcasey commented Apr 22, 2022

So the tests used to call the unpatched writeFile and that was always wrong? They were, apparently, doing this because the assert was too strict?

Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

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

LGTM, assuming I have correctly understood why calling the original writeFile was a mistake.

src/testRunner/unittests/tsbuild/helpers.ts Show resolved Hide resolved
@sheetalkamat sheetalkamat merged commit 94cb657 into main Apr 22, 2022
@sheetalkamat sheetalkamat deleted the incrementalCorrectness branch April 22, 2022 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants