-
Notifications
You must be signed in to change notification settings - Fork 4
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
test: Use assert_matches! less #1004
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1004 +/- ##
==========================================
+ Coverage 85.72% 85.89% +0.17%
==========================================
Files 79 79
Lines 14531 14558 +27
Branches 14531 14558 +27
==========================================
+ Hits 12456 12505 +49
+ Misses 1428 1393 -35
- Partials 647 660 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
756bbb2
to
33f8515
Compare
The value of let x = None;
assert!(matches!(x, Some(42)));
// thread 'test::test_assert' panicked at src/lib.rs:14:9:
// assertion failed: matches!(x, Some(42))
// note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace with let x = None;
assert_matches!(x, Some(42));
// thread 'test::test_matches' panicked at src/lib.rs:8:9:
// assertion failed at src/lib.rs:8: `(value does not match pattern)`
// value: None
// pattern: Some(42) In this case, matching against a pattern instead of doing I agree that adding statements on the matched branches is error-prone. I'm happy with banning those. |
I'm closing this PR since seem to have agreed on the status quo. |
There are downsides to
assert_matches!
:assert!(matches!(...))
lead toclippy
noticing that the match could be converted to.is_ok()
or.is_some()
Hence, this PR adopts the policy of "use
assert_matches
only when it's useful". Specifically, that means only when we can use(and while we're at it, where
EXPR
is an equality, useassert_eq!
), rather thanbecause the former gives better error messages in case of failure.
(This is quite a small advantage, and noting the increased chance of accidents, one could also say - let's not use
assert_matches
at all....I'd be happy to do that, too.)