-
Notifications
You must be signed in to change notification settings - Fork 847
Tests/syntax: produce tmp file when result doesn't match the baseline #15089
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
tests/service/SyntaxTreeTests.fs
Outdated
| elif not equals then | ||
| File.WriteAllText(tmpPath, actual) | ||
| else | ||
| File.Delete(tmpPath) |
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.
Would be good to avoid IO-attempt in the happy path
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.
Do you think it is something that would make a measurable difference in these tests?
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.
These things tend to accummulate over time with more tests and more functinalities in the suite.
If it can be easily avoided, we shouldn't be doing unnecessary IO when tests are running.
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.
It looks like these tests access IO enough already for it not to matter much here. It simplifies the work with the parser, as it's easy to see what got changed without requiring to clean this files manually later.
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.
Or I could, for example, check some environment variable indicating a CI run if you share a pointer
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.
If the TEST_UPDATE_BSL is not appropriate, see here what get's configured for CI runs:
https://dev.azure.com/dnceng-public/public/_build/results?buildId=241480&view=logs&jobId=b4ab8557-6e24-5e79-3313-9df489728e07&j=b4ab8557-6e24-5e79-3313-9df489728e07&t=cdc24b7f-1874-5a89-2ab2-501a708f5d36
e.g. "BUILD_REASON" is set for CI runs.
36e1d8a to
40579fa
Compare
Simplifies development loop when making changes to the parser and want to see what files parsing is changed compared to the baseline and what has changed.