-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
More useful test error messages on should_panic(expected=...) mismatch #66503
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
eb63306
to
16bf4f5
Compare
Looks reasonable to me! @bors r+ |
📋 Looks like this PR is still in progress, ignoring approval. Hint: Remove [WIP] from this PR's title when it is ready for review. |
@bors r=joshtriplett |
📌 Commit 16bf4f5 has been approved by |
⌛ Testing commit 16bf4f5 with merge 446a51bafa9f6bfdc812024c948acf4842b4e2e3... |
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-azure |
Seems to be an independent error, could someone with sufficient privileges retry? @bors retry |
@thomasetter: 🔑 Insufficient privileges: not in try users |
1 similar comment
@thomasetter: 🔑 Insufficient privileges: not in try users |
@bors retry spurious network |
⌛ Testing commit 16bf4f5 with merge 2363a5241004ed3ae5f4ef0cf3d16713f0c38392... |
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-azure |
@bors retry |
@hdhoang: 🔑 Insufficient privileges: not in try users |
@bors retry spurious |
…triplett More useful test error messages on should_panic(expected=...) mismatch Fixes rust-lang#66304 r? @gilescope Shows both the actual as well as the expected panic value when a test with `should_panic(expected=...)` fails. This makes `should_panic` more consistent with `assert_eq`. I am not sure whether printing the `Any::type_id()` is useful, is there something better that we could print for non-string panic values?
Rollup of 9 pull requests Successful merges: - #66503 (More useful test error messages on should_panic(expected=...) mismatch) - #66662 (Miri: run panic-catching tests in liballoc) - #66679 (Improve lifetime errors with implicit trait object lifetimes) - #66726 (Use recursion_limit for const eval stack limit) - #66790 (Do `min_const_fn` checks for `SetDiscriminant`s target) - #66832 (const_prop: detect and avoid catching Miri errors that require allocation) - #66880 (Add long error code explanation message for E0203) - #66890 (Format liballoc with rustfmt) - #66896 (pass Queries to compiler callbacks) Failed merges: r? @ghost
☔ The latest upstream changes (presumably #66917) made this pull request unmergeable. Please resolve the merge conflicts. |
Fixes #66304
r? @gilescope
Shows both the actual as well as the expected panic value when a test with
should_panic(expected=...)
fails.This makes
should_panic
more consistent withassert_eq
.I am not sure whether printing the
Any::type_id()
is useful, is there something better that we could print for non-string panic values?