On Sat, Oct 1, 2022 at 8:26 AM 'Daniel Latypov' via KUnit Development wrote: > > Context: > Currently this macro's name, KUNIT_ASSERTION conflicts with the name of > an enum whose values are {KUNIT_EXPECTATION, KUNIT_ASSERTION}. > > It's hard to think of a better name for the enum, so rename this macro. > It's also a bit strange that the macro might do nothing depending on the > boolean argument `pass`. Why not have callers check themselves? > > This patch: > Moves the pass/fail checking into the callers of KUNIT_ASSERTION, so now > we only call it when the check has failed. > Then we rename the macro the _KUNIT_FAILED() to reflect the new > semantics. > > Signed-off-by: Daniel Latypov > --- Looks good to me. I can't say the name _KUNIT_FAILED() feels perfect to me, but I can't think of anything better, either. We've not used a leading underscore for internal macros much thus far, as well, though I've no personal objections to starting. Regardless, let's get this in. Reviewed-by: David Gow Cheers, -- David > include/kunit/test.h | 123 +++++++++++++++++++++++-------------------- > 1 file changed, 65 insertions(+), 58 deletions(-) > > diff --git a/include/kunit/test.h b/include/kunit/test.h > index 3476549106f7..fec437c8a2b7 100644 > --- a/include/kunit/test.h > +++ b/include/kunit/test.h > @@ -475,30 +475,27 @@ void kunit_do_failed_assertion(struct kunit *test, > assert_format_t assert_format, > const char *fmt, ...); > > -#define KUNIT_ASSERTION(test, assert_type, pass, assert_class, assert_format, INITIALIZER, fmt, ...) do { \ > - if (unlikely(!(pass))) { \ > - static const struct kunit_loc __loc = KUNIT_CURRENT_LOC; \ > - struct assert_class __assertion = INITIALIZER; \ > - kunit_do_failed_assertion(test, \ > - &__loc, \ > - assert_type, \ > - &__assertion.assert, \ > - assert_format, \ > - fmt, \ > - ##__VA_ARGS__); \ > - } \ > +#define _KUNIT_FAILED(test, assert_type, assert_class, assert_format, INITIALIZER, fmt, ...) do { \ > + static const struct kunit_loc __loc = KUNIT_CURRENT_LOC; \ > + struct assert_class __assertion = INITIALIZER; \ > + kunit_do_failed_assertion(test, \ > + &__loc, \ > + assert_type, \ > + &__assertion.assert, \ > + assert_format, \ > + fmt, \ > + ##__VA_ARGS__); \ > } while (0) > > > #define KUNIT_FAIL_ASSERTION(test, assert_type, fmt, ...) \ > - KUNIT_ASSERTION(test, \ > - assert_type, \ > - false, \ > - kunit_fail_assert, \ > - kunit_fail_assert_format, \ > - {}, \ > - fmt, \ > - ##__VA_ARGS__) > + _KUNIT_FAILED(test, \ > + assert_type, \ > + kunit_fail_assert, \ > + kunit_fail_assert_format, \ > + {}, \ > + fmt, \ > + ##__VA_ARGS__) > > /** > * KUNIT_FAIL() - Always causes a test to fail when evaluated. > @@ -523,15 +520,19 @@ void kunit_do_failed_assertion(struct kunit *test, > expected_true, \ > fmt, \ > ...) \ > - KUNIT_ASSERTION(test, \ > - assert_type, \ > - !!(condition) == !!expected_true, \ > - kunit_unary_assert, \ > - kunit_unary_assert_format, \ > - KUNIT_INIT_UNARY_ASSERT_STRUCT(#condition, \ > - expected_true), \ > - fmt, \ > - ##__VA_ARGS__) > +do { \ > + if (likely(!!(condition) == !!expected_true)) \ > + break; \ > + \ > + _KUNIT_FAILED(test, \ > + assert_type, \ > + kunit_unary_assert, \ > + kunit_unary_assert_format, \ > + KUNIT_INIT_UNARY_ASSERT_STRUCT(#condition, \ > + expected_true), \ > + fmt, \ > + ##__VA_ARGS__); \ > +} while (0) > > #define KUNIT_TRUE_MSG_ASSERTION(test, assert_type, condition, fmt, ...) \ > KUNIT_UNARY_ASSERTION(test, \ > @@ -581,16 +582,18 @@ do { \ > .right_text = #right, \ > }; \ > \ > - KUNIT_ASSERTION(test, \ > - assert_type, \ > - __left op __right, \ > - assert_class, \ > - format_func, \ > - KUNIT_INIT_BINARY_ASSERT_STRUCT(&__text, \ > - __left, \ > - __right), \ > - fmt, \ > - ##__VA_ARGS__); \ > + if (likely(__left op __right)) \ > + break; \ > + \ > + _KUNIT_FAILED(test, \ > + assert_type, \ > + assert_class, \ > + format_func, \ > + KUNIT_INIT_BINARY_ASSERT_STRUCT(&__text, \ > + __left, \ > + __right), \ > + fmt, \ > + ##__VA_ARGS__); \ > } while (0) > > #define KUNIT_BINARY_INT_ASSERTION(test, \ > @@ -639,16 +642,19 @@ do { \ > .right_text = #right, \ > }; \ > \ > - KUNIT_ASSERTION(test, \ > - assert_type, \ > - strcmp(__left, __right) op 0, \ > - kunit_binary_str_assert, \ > - kunit_binary_str_assert_format, \ > - KUNIT_INIT_BINARY_ASSERT_STRUCT(&__text, \ > - __left, \ > - __right), \ > - fmt, \ > - ##__VA_ARGS__); \ > + if (likely(strcmp(__left, __right) op 0)) \ > + break; \ > + \ > + \ > + _KUNIT_FAILED(test, \ > + assert_type, \ > + kunit_binary_str_assert, \ > + kunit_binary_str_assert_format, \ > + KUNIT_INIT_BINARY_ASSERT_STRUCT(&__text, \ > + __left, \ > + __right), \ > + fmt, \ > + ##__VA_ARGS__); \ > } while (0) > > #define KUNIT_PTR_NOT_ERR_OR_NULL_MSG_ASSERTION(test, \ > @@ -659,15 +665,16 @@ do { \ > do { \ > const typeof(ptr) __ptr = (ptr); \ > \ > - KUNIT_ASSERTION(test, \ > - assert_type, \ > - !IS_ERR_OR_NULL(__ptr), \ > - kunit_ptr_not_err_assert, \ > - kunit_ptr_not_err_assert_format, \ > - KUNIT_INIT_PTR_NOT_ERR_STRUCT(#ptr, \ > - __ptr), \ > - fmt, \ > - ##__VA_ARGS__); \ > + if (!IS_ERR_OR_NULL(__ptr)) \ > + break; \ > + \ > + _KUNIT_FAILED(test, \ > + assert_type, \ > + kunit_ptr_not_err_assert, \ > + kunit_ptr_not_err_assert_format, \ > + KUNIT_INIT_PTR_NOT_ERR_STRUCT(#ptr, __ptr), \ > + fmt, \ > + ##__VA_ARGS__); \ > } while (0) > > /** > -- > 2.38.0.rc1.362.ged0d419d3c-goog > > -- > You received this message because you are subscribed to the Google Groups "KUnit Development" group. > To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20221001002638.2881842-3-dlatypov%40google.com.