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

Cleanup test helpers #432

Merged
merged 6 commits into from
Dec 29, 2023
Merged

Cleanup test helpers #432

merged 6 commits into from
Dec 29, 2023

Conversation

ratmice
Copy link
Collaborator

@ratmice ratmice commented Dec 28, 2023

This is a cleanup for the ErrorsHelper test functions, it is primarily intended to improve the output.
I.e. it should now print both the error, and the expected, when they don't match now.
Previously this was difficult to do because the expected value was used in the if guard of an Err(e) if ... it could no longer be used in plain old the Err(e) branch of the match. As such, I just reorganized the implementation to make that branch unneeded.

Doing the same thing in lrlex required we add a Debug derive for LrNonStreamingLexerDef.

@ratmice
Copy link
Collaborator Author

ratmice commented Dec 29, 2023

It occurred to me that it would be good to add #[track_caller] to these helper functions, so that panics in the helper refer to the location in the test where the helper is called, rather than some line within the helper itself. Pretty sure that is everything for this series.

@ratmice ratmice changed the title Cleanup test Cleanup test helpers Dec 29, 2023
@ltratt
Copy link
Member

ltratt commented Dec 29, 2023

It occurred to me that it would be good to add #[track_caller] to these helper functions

I did not know about #[track_caller]! Good idea.

@ltratt ltratt added this pull request to the merge queue Dec 29, 2023
Merged via the queue into softdevteam:master with commit 13e217c Dec 29, 2023
2 checks passed
@ratmice ratmice deleted the cleanup_test branch December 29, 2023 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants