From: David Gow <davidgow@google.com>
To: Daniel Latypov <dlatypov@google.com>
Cc: Brendan Higgins <brendanhiggins@google.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
KUnit Development <kunit-dev@googlegroups.com>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@vger.kernel.org>,
Shuah Khan <skhan@linuxfoundation.org>,
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Subject: Re: [PATCH 2/4] kunit: rename base KUNIT_ASSERTION macro to _KUNIT_FAILED
Date: Sat, 1 Oct 2022 11:26:40 +0800 [thread overview]
Message-ID: <CABVgOSn3SupF_z84FghxX-yK-CLx_RQMkUxF_hGUw6a3w7h-7Q@mail.gmail.com> (raw)
In-Reply-To: <20221001002638.2881842-3-dlatypov@google.com>
[-- Attachment #1: Type: text/plain, Size: 14019 bytes --]
On Sat, Oct 1, 2022 at 8:26 AM 'Daniel Latypov' via KUnit Development
<kunit-dev@googlegroups.com> 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 <dlatypov@google.com>
> ---
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 <davidgow@google.com>
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.
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]
next prev parent reply other threads:[~2022-10-01 3:34 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-01 0:26 [PATCH 0/4] kunit: more assertion reworking Daniel Latypov
2022-10-01 0:26 ` [PATCH 1/4] kunit: remove format func from struct kunit_assert, get it to 0 bytes Daniel Latypov
2022-10-01 3:26 ` David Gow
2022-10-01 0:26 ` [PATCH 2/4] kunit: rename base KUNIT_ASSERTION macro to _KUNIT_FAILED Daniel Latypov
2022-10-01 3:26 ` David Gow [this message]
2022-10-01 3:50 ` Daniel Latypov
2022-10-01 4:13 ` David Gow
2022-10-01 0:26 ` [PATCH 3/4] kunit: eliminate KUNIT_INIT_*_ASSERT_STRUCT macros Daniel Latypov
2022-10-01 3:26 ` David Gow
2022-10-01 10:12 ` Miguel Ojeda
2022-10-01 17:48 ` Daniel Latypov
2022-10-01 0:26 ` [PATCH 4/4] kunit: declare kunit_assert structs as const Daniel Latypov
2022-10-01 3:26 ` David Gow
2022-10-01 10:06 ` Miguel Ojeda
2022-10-01 10:15 ` [PATCH 0/4] kunit: more assertion reworking Miguel Ojeda
2022-10-01 18:00 ` Daniel Latypov
2022-10-18 23:20 ` Miguel Ojeda
2022-10-18 23:26 ` Daniel Latypov
2022-10-18 23:39 ` Miguel Ojeda
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABVgOSn3SupF_z84FghxX-yK-CLx_RQMkUxF_hGUw6a3w7h-7Q@mail.gmail.com \
--to=davidgow@google.com \
--cc=brendanhiggins@google.com \
--cc=dlatypov@google.com \
--cc=kunit-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=miguel.ojeda.sandonis@gmail.com \
--cc=skhan@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).