* [PATCH 0/3] kunit: Improve the readability and functionality of macro.
@ 2024-07-10 17:04 Eric Chan
2024-07-10 17:06 ` [PATCH 1/3] kunit: Fix the comment of KUNIT_ASSERT_STRNEQ as assertion Eric Chan
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Eric Chan @ 2024-07-10 17:04 UTC (permalink / raw)
To: brendan.higgins, davidgow, rmoar
Cc: linux-kselftest, kunit-dev, linux-kernel, ericchancf
This series let kunit macro more neat and clear.
Fix comment and rename the macro.
Also introduce new type of assertion marco for functionality.
Eric Chan (3):
kunit: Fix the comment of KUNIT_ASSERT_STRNEQ as assertion
kunit: Rename KUNIT_ASSERT_FAILURE to KUNIT_ASSERT for readability
kunit: Introduce KUNIT_ASSERT_MEMEQ and KUNIT_ASSERT_MEMNEQ macros
drivers/input/tests/input_test.c | 2 +-
include/kunit/assert.h | 2 +-
include/kunit/test.h | 71 ++++++++++++++++++++++++++++++--
3 files changed, 70 insertions(+), 5 deletions(-)
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] kunit: Fix the comment of KUNIT_ASSERT_STRNEQ as assertion
2024-07-10 17:04 [PATCH 0/3] kunit: Improve the readability and functionality of macro Eric Chan
@ 2024-07-10 17:06 ` Eric Chan
2024-07-11 3:48 ` David Gow
2024-07-10 17:06 ` [PATCH 2/3] kunit: Rename KUNIT_ASSERT_FAILURE to KUNIT_ASSERT for readability Eric Chan
2024-07-10 17:06 ` [PATCH 3/3] kunit: Introduce KUNIT_ASSERT_MEMEQ and KUNIT_ASSERT_MEMNEQ macros Eric Chan
2 siblings, 1 reply; 8+ messages in thread
From: Eric Chan @ 2024-07-10 17:06 UTC (permalink / raw)
To: brendan.higgins, davidgow, rmoar
Cc: linux-kselftest, kunit-dev, linux-kernel, ericchancf
The current comment for KUNIT_ASSERT_STRNEQ incorrectly describes it as
an expectation. Since KUNIT_ASSERT_STRNEQ is an assertion, updates the
comment to correctly refer to it as such.
Signed-off-by: Eric Chan <ericchancf@google.com>
---
include/kunit/test.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 61637ef32302..87a232421089 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -1420,12 +1420,12 @@ do { \
##__VA_ARGS__)
/**
- * KUNIT_ASSERT_STRNEQ() - Expects that strings @left and @right are not equal.
+ * KUNIT_ASSERT_STRNEQ() - An assertion that strings @left and @right are not equal.
* @test: The test context object.
* @left: an arbitrary expression that evaluates to a null terminated string.
* @right: an arbitrary expression that evaluates to a null terminated string.
*
- * Sets an expectation that the values that @left and @right evaluate to are
+ * Sets an assertion that the values that @left and @right evaluate to are
* not equal. This is semantically equivalent to
* KUNIT_ASSERT_TRUE(@test, strcmp((@left), (@right))). See KUNIT_ASSERT_TRUE()
* for more information.
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] kunit: Rename KUNIT_ASSERT_FAILURE to KUNIT_ASSERT for readability
2024-07-10 17:04 [PATCH 0/3] kunit: Improve the readability and functionality of macro Eric Chan
2024-07-10 17:06 ` [PATCH 1/3] kunit: Fix the comment of KUNIT_ASSERT_STRNEQ as assertion Eric Chan
@ 2024-07-10 17:06 ` Eric Chan
2024-07-11 3:49 ` David Gow
2024-07-10 17:06 ` [PATCH 3/3] kunit: Introduce KUNIT_ASSERT_MEMEQ and KUNIT_ASSERT_MEMNEQ macros Eric Chan
2 siblings, 1 reply; 8+ messages in thread
From: Eric Chan @ 2024-07-10 17:06 UTC (permalink / raw)
To: brendan.higgins, davidgow, rmoar
Cc: linux-kselftest, kunit-dev, linux-kernel, ericchancf
Both KUNIT_FAIL and KUNIT_ASSERT_FAILURE defined to KUNIT_FAIL_ASSERTION
with different tpye of kunit_assert_type. The current naming of
KUNIT_ASSERT_FAILURE and KUNIT_FAIL_ASSERTION is confusing due to their
similarities. To improve readability and symmetry, renames
KUNIT_ASSERT_FAILURE to KUNIT_ASSERT. Makes the naming consistent,
with KUNIT_FAIL and KUNIT_ASSERT being symmetrical.
Additionally, an explanation for KUNIT_ASSERT has been added to clarify
its usage.
Signed-off-by: Eric Chan <ericchancf@google.com>
---
drivers/input/tests/input_test.c | 2 +-
include/kunit/assert.h | 2 +-
include/kunit/test.h | 13 ++++++++++++-
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/input/tests/input_test.c b/drivers/input/tests/input_test.c
index 2fa5b725ae0a..cbab24a265fa 100644
--- a/drivers/input/tests/input_test.c
+++ b/drivers/input/tests/input_test.c
@@ -31,7 +31,7 @@ static int input_test_init(struct kunit *test)
ret = input_register_device(input_dev);
if (ret) {
input_free_device(input_dev);
- KUNIT_ASSERT_FAILURE(test, "Register device failed: %d", ret);
+ KUNIT_ASSERT(test, "Register device failed: %d", ret);
}
test->priv = input_dev;
diff --git a/include/kunit/assert.h b/include/kunit/assert.h
index 24c2b9fa61e8..02c6f7bb1d26 100644
--- a/include/kunit/assert.h
+++ b/include/kunit/assert.h
@@ -60,7 +60,7 @@ void kunit_assert_prologue(const struct kunit_loc *loc,
* struct kunit_fail_assert - Represents a plain fail expectation/assertion.
* @assert: The parent of this type.
*
- * Represents a simple KUNIT_FAIL/KUNIT_ASSERT_FAILURE that always fails.
+ * Represents a simple KUNIT_FAIL/KUNIT_ASSERT that always fails.
*/
struct kunit_fail_assert {
struct kunit_assert assert;
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 87a232421089..d1b085fd5dc3 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -1193,7 +1193,18 @@ do { \
fmt, \
##__VA_ARGS__)
-#define KUNIT_ASSERT_FAILURE(test, fmt, ...) \
+/**
+ * KUNIT_ASSERT() - Always causes a test to assert when evaluated.
+ * @test: The test context object.
+ * @fmt: an informational message to be printed when the assertion is made.
+ * @...: string format arguments.
+ *
+ * The opposite of KUNIT_SUCCEED(), it is an assertion that always fails. In
+ * other words, it always results in a failed assertion, and consequently
+ * always causes the test case to assert when evaluated. See KUNIT_ASSERT_TRUE()
+ * for more information.
+ */
+#define KUNIT_ASSERT(test, fmt, ...) \
KUNIT_FAIL_ASSERTION(test, KUNIT_ASSERTION, fmt, ##__VA_ARGS__)
/**
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] kunit: Introduce KUNIT_ASSERT_MEMEQ and KUNIT_ASSERT_MEMNEQ macros
2024-07-10 17:04 [PATCH 0/3] kunit: Improve the readability and functionality of macro Eric Chan
2024-07-10 17:06 ` [PATCH 1/3] kunit: Fix the comment of KUNIT_ASSERT_STRNEQ as assertion Eric Chan
2024-07-10 17:06 ` [PATCH 2/3] kunit: Rename KUNIT_ASSERT_FAILURE to KUNIT_ASSERT for readability Eric Chan
@ 2024-07-10 17:06 ` Eric Chan
2024-07-11 3:49 ` David Gow
2 siblings, 1 reply; 8+ messages in thread
From: Eric Chan @ 2024-07-10 17:06 UTC (permalink / raw)
To: brendan.higgins, davidgow, rmoar
Cc: linux-kselftest, kunit-dev, linux-kernel, ericchancf
Introduces KUNIT_ASSERT_MEMEQ and KUNIT_ASSERT_MEMNEQ macros
to provide assert-type equivalents for memory comparison.
While KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ are available for
expectations, the addition of these new macros ensures that assertions
can also be used for memory comparisons, enhancing the consistency and
completeness of the kunit framework.
Signed-off-by: Eric Chan <ericchancf@google.com>
---
include/kunit/test.h | 54 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index d1b085fd5dc3..52bd50d2b150 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -1451,6 +1451,60 @@ do { \
fmt, \
##__VA_ARGS__)
+/**
+ * KUNIT_ASSERT_MEMEQ() - Asserts that the first @size bytes of @left and @right are equal.
+ * @test: The test context object.
+ * @left: An arbitrary expression that evaluates to the specified size.
+ * @right: An arbitrary expression that evaluates to the specified size.
+ * @size: Number of bytes compared.
+ *
+ * Sets an assertion that the values that @left and @right evaluate to are
+ * equal. This is semantically equivalent to
+ * KUNIT_ASSERT_TRUE(@test, !memcmp((@left), (@right), (@size))). See
+ * KUNIT_ASSERT_TRUE() for more information.
+ *
+ * Although this assertion works for any memory block, it is not recommended
+ * for comparing more structured data, such as structs. This assertion is
+ * recommended for comparing, for example, data arrays.
+ */
+#define KUNIT_ASSERT_MEMEQ(test, left, right, size) \
+ KUNIT_ASSERT_MEMEQ_MSG(test, left, right, size, NULL)
+
+#define KUNIT_ASSERT_MEMEQ_MSG(test, left, right, size, fmt, ...) \
+ KUNIT_MEM_ASSERTION(test, \
+ KUNIT_ASSERTION, \
+ left, ==, right, \
+ size, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_ASSERT_MEMNEQ() - Asserts that the first @size bytes of @left and @right are not equal.
+ * @test: The test context object.
+ * @left: An arbitrary expression that evaluates to the specified size.
+ * @right: An arbitrary expression that evaluates to the specified size.
+ * @size: Number of bytes compared.
+ *
+ * Sets an assertion that the values that @left and @right evaluate to are
+ * not equal. This is semantically equivalent to
+ * KUNIT_ASSERT_TRUE(@test, memcmp((@left), (@right), (@size))). See
+ * KUNIT_ASSERT_TRUE() for more information.
+ *
+ * Although this assertion works for any memory block, it is not recommended
+ * for comparing more structured data, such as structs. This assertion is
+ * recommended for comparing, for example, data arrays.
+ */
+#define KUNIT_ASSERT_MEMNEQ(test, left, right, size) \
+ KUNIT_ASSERT_MEMNEQ_MSG(test, left, right, size, NULL)
+
+#define KUNIT_ASSERT_MEMNEQ_MSG(test, left, right, size, fmt, ...) \
+ KUNIT_MEM_ASSERTION(test, \
+ KUNIT_ASSERTION, \
+ left, !=, right, \
+ size, \
+ fmt, \
+ ##__VA_ARGS__)
+
/**
* KUNIT_ASSERT_NULL() - Asserts that pointers @ptr is null.
* @test: The test context object.
--
2.45.2.803.g4e1b14247a-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] kunit: Fix the comment of KUNIT_ASSERT_STRNEQ as assertion
2024-07-10 17:06 ` [PATCH 1/3] kunit: Fix the comment of KUNIT_ASSERT_STRNEQ as assertion Eric Chan
@ 2024-07-11 3:48 ` David Gow
0 siblings, 0 replies; 8+ messages in thread
From: David Gow @ 2024-07-11 3:48 UTC (permalink / raw)
To: Eric Chan
Cc: brendan.higgins, rmoar, linux-kselftest, kunit-dev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]
On Thu, 11 Jul 2024 at 01:06, Eric Chan <ericchancf@google.com> wrote:
>
> The current comment for KUNIT_ASSERT_STRNEQ incorrectly describes it as
> an expectation. Since KUNIT_ASSERT_STRNEQ is an assertion, updates the
> comment to correctly refer to it as such.
>
> Signed-off-by: Eric Chan <ericchancf@google.com>
> ---
Nice catch -- copy-and-paste strikes again!
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
> include/kunit/test.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 61637ef32302..87a232421089 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -1420,12 +1420,12 @@ do { \
> ##__VA_ARGS__)
>
> /**
> - * KUNIT_ASSERT_STRNEQ() - Expects that strings @left and @right are not equal.
> + * KUNIT_ASSERT_STRNEQ() - An assertion that strings @left and @right are not equal.
> * @test: The test context object.
> * @left: an arbitrary expression that evaluates to a null terminated string.
> * @right: an arbitrary expression that evaluates to a null terminated string.
> *
> - * Sets an expectation that the values that @left and @right evaluate to are
> + * Sets an assertion that the values that @left and @right evaluate to are
> * not equal. This is semantically equivalent to
> * KUNIT_ASSERT_TRUE(@test, strcmp((@left), (@right))). See KUNIT_ASSERT_TRUE()
> * for more information.
> --
> 2.45.2.803.g4e1b14247a-goog
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4014 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] kunit: Rename KUNIT_ASSERT_FAILURE to KUNIT_ASSERT for readability
2024-07-10 17:06 ` [PATCH 2/3] kunit: Rename KUNIT_ASSERT_FAILURE to KUNIT_ASSERT for readability Eric Chan
@ 2024-07-11 3:49 ` David Gow
2024-07-11 19:48 ` Eric Chan
0 siblings, 1 reply; 8+ messages in thread
From: David Gow @ 2024-07-11 3:49 UTC (permalink / raw)
To: Eric Chan
Cc: brendan.higgins, rmoar, linux-kselftest, kunit-dev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3556 bytes --]
On Thu, 11 Jul 2024 at 01:06, Eric Chan <ericchancf@google.com> wrote:
>
> Both KUNIT_FAIL and KUNIT_ASSERT_FAILURE defined to KUNIT_FAIL_ASSERTION
> with different tpye of kunit_assert_type. The current naming of
> KUNIT_ASSERT_FAILURE and KUNIT_FAIL_ASSERTION is confusing due to their
> similarities. To improve readability and symmetry, renames
> KUNIT_ASSERT_FAILURE to KUNIT_ASSERT. Makes the naming consistent,
> with KUNIT_FAIL and KUNIT_ASSERT being symmetrical.
> Additionally, an explanation for KUNIT_ASSERT has been added to clarify
> its usage.
>
> Signed-off-by: Eric Chan <ericchancf@google.com>
> ---
I personally am not a fan of KUNIT_ASSERT() as a name here: to me it
implies that we're checking a boolean (like KUNIT_ASSERT_TRUE()).
Does making this 'KUNIT_FAIL_AND_EXIT()' / 'KUNIT_FAIL_AND_ABORT()' or
similar seem clearer to you?
(Or possibly we could make this KUNIT_FAIL(), and make the existing
KUNIT_FAIL() become KUNIT_MARK_FAILED(), though I think it's not worth
the churn personally.)
-- David
> drivers/input/tests/input_test.c | 2 +-
> include/kunit/assert.h | 2 +-
> include/kunit/test.h | 13 ++++++++++++-
> 3 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/tests/input_test.c b/drivers/input/tests/input_test.c
> index 2fa5b725ae0a..cbab24a265fa 100644
> --- a/drivers/input/tests/input_test.c
> +++ b/drivers/input/tests/input_test.c
> @@ -31,7 +31,7 @@ static int input_test_init(struct kunit *test)
> ret = input_register_device(input_dev);
> if (ret) {
> input_free_device(input_dev);
> - KUNIT_ASSERT_FAILURE(test, "Register device failed: %d", ret);
> + KUNIT_ASSERT(test, "Register device failed: %d", ret);
> }
>
> test->priv = input_dev;
> diff --git a/include/kunit/assert.h b/include/kunit/assert.h
> index 24c2b9fa61e8..02c6f7bb1d26 100644
> --- a/include/kunit/assert.h
> +++ b/include/kunit/assert.h
> @@ -60,7 +60,7 @@ void kunit_assert_prologue(const struct kunit_loc *loc,
> * struct kunit_fail_assert - Represents a plain fail expectation/assertion.
> * @assert: The parent of this type.
> *
> - * Represents a simple KUNIT_FAIL/KUNIT_ASSERT_FAILURE that always fails.
> + * Represents a simple KUNIT_FAIL/KUNIT_ASSERT that always fails.
> */
> struct kunit_fail_assert {
> struct kunit_assert assert;
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 87a232421089..d1b085fd5dc3 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -1193,7 +1193,18 @@ do { \
> fmt, \
> ##__VA_ARGS__)
>
> -#define KUNIT_ASSERT_FAILURE(test, fmt, ...) \
> +/**
> + * KUNIT_ASSERT() - Always causes a test to assert when evaluated.
> + * @test: The test context object.
> + * @fmt: an informational message to be printed when the assertion is made.
> + * @...: string format arguments.
> + *
> + * The opposite of KUNIT_SUCCEED(), it is an assertion that always fails. In
> + * other words, it always results in a failed assertion, and consequently
> + * always causes the test case to assert when evaluated. See KUNIT_ASSERT_TRUE()
> + * for more information.
> + */
> +#define KUNIT_ASSERT(test, fmt, ...) \
> KUNIT_FAIL_ASSERTION(test, KUNIT_ASSERTION, fmt, ##__VA_ARGS__)
>
> /**
> --
> 2.45.2.803.g4e1b14247a-goog
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4014 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] kunit: Introduce KUNIT_ASSERT_MEMEQ and KUNIT_ASSERT_MEMNEQ macros
2024-07-10 17:06 ` [PATCH 3/3] kunit: Introduce KUNIT_ASSERT_MEMEQ and KUNIT_ASSERT_MEMNEQ macros Eric Chan
@ 2024-07-11 3:49 ` David Gow
0 siblings, 0 replies; 8+ messages in thread
From: David Gow @ 2024-07-11 3:49 UTC (permalink / raw)
To: Eric Chan
Cc: brendan.higgins, rmoar, linux-kselftest, kunit-dev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 4233 bytes --]
On Thu, 11 Jul 2024 at 01:07, Eric Chan <ericchancf@google.com> wrote:
>
> Introduces KUNIT_ASSERT_MEMEQ and KUNIT_ASSERT_MEMNEQ macros
> to provide assert-type equivalents for memory comparison.
> While KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ are available for
> expectations, the addition of these new macros ensures that assertions
> can also be used for memory comparisons, enhancing the consistency and
> completeness of the kunit framework.
>
> Signed-off-by: Eric Chan <ericchancf@google.com>
> ---
Nice catch, thanks!
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
> include/kunit/test.h | 54 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index d1b085fd5dc3..52bd50d2b150 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -1451,6 +1451,60 @@ do { \
> fmt, \
> ##__VA_ARGS__)
>
> +/**
> + * KUNIT_ASSERT_MEMEQ() - Asserts that the first @size bytes of @left and @right are equal.
> + * @test: The test context object.
> + * @left: An arbitrary expression that evaluates to the specified size.
> + * @right: An arbitrary expression that evaluates to the specified size.
> + * @size: Number of bytes compared.
> + *
> + * Sets an assertion that the values that @left and @right evaluate to are
> + * equal. This is semantically equivalent to
> + * KUNIT_ASSERT_TRUE(@test, !memcmp((@left), (@right), (@size))). See
> + * KUNIT_ASSERT_TRUE() for more information.
> + *
> + * Although this assertion works for any memory block, it is not recommended
> + * for comparing more structured data, such as structs. This assertion is
> + * recommended for comparing, for example, data arrays.
> + */
> +#define KUNIT_ASSERT_MEMEQ(test, left, right, size) \
> + KUNIT_ASSERT_MEMEQ_MSG(test, left, right, size, NULL)
> +
> +#define KUNIT_ASSERT_MEMEQ_MSG(test, left, right, size, fmt, ...) \
> + KUNIT_MEM_ASSERTION(test, \
> + KUNIT_ASSERTION, \
> + left, ==, right, \
> + size, \
> + fmt, \
> + ##__VA_ARGS__)
> +
> +/**
> + * KUNIT_ASSERT_MEMNEQ() - Asserts that the first @size bytes of @left and @right are not equal.
> + * @test: The test context object.
> + * @left: An arbitrary expression that evaluates to the specified size.
> + * @right: An arbitrary expression that evaluates to the specified size.
> + * @size: Number of bytes compared.
> + *
> + * Sets an assertion that the values that @left and @right evaluate to are
> + * not equal. This is semantically equivalent to
> + * KUNIT_ASSERT_TRUE(@test, memcmp((@left), (@right), (@size))). See
> + * KUNIT_ASSERT_TRUE() for more information.
> + *
> + * Although this assertion works for any memory block, it is not recommended
> + * for comparing more structured data, such as structs. This assertion is
> + * recommended for comparing, for example, data arrays.
> + */
> +#define KUNIT_ASSERT_MEMNEQ(test, left, right, size) \
> + KUNIT_ASSERT_MEMNEQ_MSG(test, left, right, size, NULL)
> +
> +#define KUNIT_ASSERT_MEMNEQ_MSG(test, left, right, size, fmt, ...) \
> + KUNIT_MEM_ASSERTION(test, \
> + KUNIT_ASSERTION, \
> + left, !=, right, \
> + size, \
> + fmt, \
> + ##__VA_ARGS__)
> +
> /**
> * KUNIT_ASSERT_NULL() - Asserts that pointers @ptr is null.
> * @test: The test context object.
> --
> 2.45.2.803.g4e1b14247a-goog
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4014 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] kunit: Rename KUNIT_ASSERT_FAILURE to KUNIT_ASSERT for readability
2024-07-11 3:49 ` David Gow
@ 2024-07-11 19:48 ` Eric Chan
0 siblings, 0 replies; 8+ messages in thread
From: Eric Chan @ 2024-07-11 19:48 UTC (permalink / raw)
To: davidgow
Cc: brendan.higgins, ericchancf, kunit-dev, linux-kernel,
linux-kselftest, rmoar
On Thu, Jul 11, 2024 at 11:49 AM David Gow <davidgow@google.com> wrote:
>
> On Thu, 11 Jul 2024 at 01:06, Eric Chan <ericchancf@google.com> wrote:
> >
> > Both KUNIT_FAIL and KUNIT_ASSERT_FAILURE defined to KUNIT_FAIL_ASSERTION
> > with different tpye of kunit_assert_type. The current naming of
> > KUNIT_ASSERT_FAILURE and KUNIT_FAIL_ASSERTION is confusing due to their
> > similarities. To improve readability and symmetry, renames
> > KUNIT_ASSERT_FAILURE to KUNIT_ASSERT. Makes the naming consistent,
> > with KUNIT_FAIL and KUNIT_ASSERT being symmetrical.
> > Additionally, an explanation for KUNIT_ASSERT has been added to clarify
> > its usage.
> >
> > Signed-off-by: Eric Chan <ericchancf@google.com>
> > ---
>
> I personally am not a fan of KUNIT_ASSERT() as a name here: to me it
> implies that we're checking a boolean (like KUNIT_ASSERT_TRUE()).
>
> Does making this 'KUNIT_FAIL_AND_EXIT()' / 'KUNIT_FAIL_AND_ABORT()' or
> similar seem clearer to you?
>
> (Or possibly we could make this KUNIT_FAIL(), and make the existing
> KUNIT_FAIL() become KUNIT_MARK_FAILED(), though I think it's not worth
> the churn personally.)
>
> -- David
Hi David,
Thank you very much for patiently reviewing.
I understand your suggestion, indeed KUNIT_ASSERT will be misunderstood as still an assert behavior.
But in fact, this macro has one extra abort behavior than KUNIT_FAIL.
I think KUNIT_FAIL_AND_ABORT is a pretty good name to understand itself.
I've updated patch v2 at [0].
Thanks for the reviewing and suggestions.
[0] https://lore.kernel.org/lkml/20240711193729.108720-1-ericchancf@google.com/
Sincerely,
Eric Chan
>
>
>
>
> > drivers/input/tests/input_test.c | 2 +-
> > include/kunit/assert.h | 2 +-
> > include/kunit/test.h | 13 ++++++++++++-
> > 3 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/input/tests/input_test.c b/drivers/input/tests/input_test.c
> > index 2fa5b725ae0a..cbab24a265fa 100644
> > --- a/drivers/input/tests/input_test.c
> > +++ b/drivers/input/tests/input_test.c
> > @@ -31,7 +31,7 @@ static int input_test_init(struct kunit *test)
> > ret = input_register_device(input_dev);
> > if (ret) {
> > input_free_device(input_dev);
> > - KUNIT_ASSERT_FAILURE(test, "Register device failed: %d", ret);
> > + KUNIT_ASSERT(test, "Register device failed: %d", ret);
> > }
> >
> > test->priv = input_dev;
> > diff --git a/include/kunit/assert.h b/include/kunit/assert.h
> > index 24c2b9fa61e8..02c6f7bb1d26 100644
> > --- a/include/kunit/assert.h
> > +++ b/include/kunit/assert.h
> > @@ -60,7 +60,7 @@ void kunit_assert_prologue(const struct kunit_loc *loc,
> > * struct kunit_fail_assert - Represents a plain fail expectation/assertion.
> > * @assert: The parent of this type.
> > *
> > - * Represents a simple KUNIT_FAIL/KUNIT_ASSERT_FAILURE that always fails.
> > + * Represents a simple KUNIT_FAIL/KUNIT_ASSERT that always fails.
> > */
> > struct kunit_fail_assert {
> > struct kunit_assert assert;
> > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > index 87a232421089..d1b085fd5dc3 100644
> > --- a/include/kunit/test.h
> > +++ b/include/kunit/test.h
> > @@ -1193,7 +1193,18 @@ do { \
> > fmt, \
> > ##__VA_ARGS__)
> >
> > -#define KUNIT_ASSERT_FAILURE(test, fmt, ...) \
> > +/**
> > + * KUNIT_ASSERT() - Always causes a test to assert when evaluated.
> > + * @test: The test context object.
> > + * @fmt: an informational message to be printed when the assertion is made.
> > + * @...: string format arguments.
> > + *
> > + * The opposite of KUNIT_SUCCEED(), it is an assertion that always fails. In
> > + * other words, it always results in a failed assertion, and consequently
> > + * always causes the test case to assert when evaluated. See KUNIT_ASSERT_TRUE()
> > + * for more information.
> > + */
> > +#define KUNIT_ASSERT(test, fmt, ...) \
> > KUNIT_FAIL_ASSERTION(test, KUNIT_ASSERTION, fmt, ##__VA_ARGS__)
> >
> > /**
> > --
> > 2.45.2.803.g4e1b14247a-goog
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-07-11 19:48 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-10 17:04 [PATCH 0/3] kunit: Improve the readability and functionality of macro Eric Chan
2024-07-10 17:06 ` [PATCH 1/3] kunit: Fix the comment of KUNIT_ASSERT_STRNEQ as assertion Eric Chan
2024-07-11 3:48 ` David Gow
2024-07-10 17:06 ` [PATCH 2/3] kunit: Rename KUNIT_ASSERT_FAILURE to KUNIT_ASSERT for readability Eric Chan
2024-07-11 3:49 ` David Gow
2024-07-11 19:48 ` Eric Chan
2024-07-10 17:06 ` [PATCH 3/3] kunit: Introduce KUNIT_ASSERT_MEMEQ and KUNIT_ASSERT_MEMNEQ macros Eric Chan
2024-07-11 3:49 ` David Gow
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox