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

Custom panic messages in snapbox #223

Open
not-my-profile opened this issue Aug 18, 2023 · 3 comments
Open

Custom panic messages in snapbox #223

not-my-profile opened this issue Aug 18, 2023 · 3 comments
Labels
A-snapbox Area: snapbox package

Comments

@not-my-profile
Copy link

not-my-profile commented Aug 18, 2023

std::assert_eq supports providing a custom panic message in a third argument (optionally followed by further parameters for the message arguments). pretty_assertions and similar-asserts both provide assert_eq! macros that are compatible.

snapbox however currently appears to hard-code it's panic message:

https://github.com/assert-rs/trycmd/blob/d73c84cda48aebda8a4b3fd404745c8bff0ab649/crates/snapbox/src/assert.rs#L55-L61

I think custom panic messages are a nice way to provide more context to a test failure, so I think it would be great if snapbox supported them.

not-my-profile added a commit to not-my-profile/trycmd that referenced this issue Aug 18, 2023
not-my-profile added a commit to not-my-profile/trycmd that referenced this issue Aug 18, 2023
@epage epage added the A-snapbox Area: snapbox package label Aug 18, 2023
@epage
Copy link
Contributor

epage commented Aug 18, 2023

Yeah, I've recently gotten requests for this from others. When creating snapbox, I overlooked this because

  • I tend to use filesystem snapshots which provides enough extra context that I wasn't needing it
  • A lot of times, I'm needing to use Assert, meaning the macros don't get as much attention

The main care about I had was then handled with track_caller.

If we move forward with this, I would expect we'd try to match the new assert format that is coming out soon, where relevant.

My main concern is what the API should look like for something like this. Assert is a first class citizen in the API and should be able to be reused across calls (something which the current solution in #224 does not handle). I also don't like creating a bunch of variations of functions for different use cases. Maybe smaller builders off of Assert?

@not-my-profile
Copy link
Author

not-my-profile commented Aug 18, 2023

the new assert format that is coming out soon

Do you have a link for or more context about that?

@epage
Copy link
Contributor

epage commented Aug 19, 2023

rust-lang/rust#111071

epage added a commit to epage/snapbox that referenced this issue Apr 23, 2024
This leaves us room to evaluate assert-rs#223

Fixes assert-rs#226
epage added a commit to epage/snapbox that referenced this issue Apr 23, 2024
This leaves us room to evaluate assert-rs#223

Fixes assert-rs#226
epage added a commit to epage/snapbox that referenced this issue May 17, 2024
This leaves us room to evaluate assert-rs#223

Fixes assert-rs#226

Cherry pick 2a1a25f (assert-rs#296)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-snapbox Area: snapbox package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants