- Notifications
You must be signed in to change notification settings - Fork1.1k
test: add golden file test for ConvertState#20832
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Emyrk commentedNov 19, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This stack of pull requests is managed byGraphite. Learn more aboutstacking. |
golden file test is to prevent regressions in behavior. It is goingto be used as a benchmark for a refactor
d879360 to18fa1ffCompare6849ee9 to7cbbc20Compare57ea109 to2ee6b40Compare| // If the directory is missing these files, we cannot run ConvertState | ||
| // on it. So it's skipped. | ||
| ifsrcIdc==-1||dotIdx==-1 { | ||
| continue |
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 should log indicating so.
dannykopping left a comment
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.
LGTM, with a couple small nits
I don't need to review again
Thanks alot for adding these!
Uh oh!
There was an error while loading.Please reload this page.
a61b8bc intomainUh oh!
There was an error while loading.Please reload this page.

Uh oh!
There was an error while loading.Please reload this page.
Refactoring
ConvertStateis something we should eventually do. This PR adds a golden file unit test for the output ofConvertState(even errors).That way if a refactor occurs, we can verify the output is unchanged for our test cases.