diff mbox series

[v2,01/10] kunit: Do not typecheck binary assertions

Message ID 20210513193204.816681-1-davidgow@google.com
State Accepted
Commit 6e62dfa6d14f8fd2b07ad30b8a1c597d40d36ac1
Headers show
Series [v2,01/10] kunit: Do not typecheck binary assertions | expand

Commit Message

David Gow May 13, 2021, 7:31 p.m. UTC 
The use of typecheck() in KUNIT_EXPECT_EQ() and friends is causing more
problems than I think it's worth. Things like enums need to have their
values explicitly cast, and literals all need to be very precisely
typed, else a large warning will be printed.

While typechecking does have its uses, the additional overhead of having
lots of needless casts -- combined with the awkward error messages which
don't mention which types are involved -- makes tests less readable and
more difficult to write.

By removing the typecheck() call, the two arguments still need to be of
compatible types, but don't need to be of exactly the same time, which
seems a less confusing and more useful compromise.

Signed-off-by: David Gow <davidgow@google.com>
Reviewed-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
---

Changes since v1:
https://lore.kernel.org/linux-kselftest/20210507050908.1008686-1-davidgow@google.com/
- Tidy up the patch description to note that a warning was being
  produced, not an error.
- Add additional patches to remove many of the now unnecessary casts.

 include/kunit/test.h | 1 -
 1 file changed, 1 deletion(-)

Comments

Andrew Jeffery May 14, 2021, 1:55 a.m. UTC | #1 
On Fri, 14 May 2021, at 05:02, David Gow wrote:
> With KUnit's EXPECT macros no longer typechecking arguments as strictly,

> get rid of a number of now unnecessary casts.

> 

> Signed-off-by: David Gow <davidgow@google.com>

> ---

> This should be a no-op functionality wise, and while it depends on the

> first couple of patches in this series, it's otherwise independent from

> the others. I think this makes the test more readable, but if you

> particularly dislike it, I'm happy to drop it.


No, happy to have that cleaned up.

Thanks David.

Acked-by: Andrew Jeffery <andrew@aj.id.au>
Mika Westerberg May 14, 2021, 6:06 a.m. UTC | #2 
Hi,

On Thu, May 13, 2021 at 12:32:01PM -0700, David Gow wrote:
> With some of the stricter type checking in KUnit's EXPECT macros

> removed, several casts in the thunderbolt KUnit tests are no longer

> required.

> 

> Remove the unnecessary casts, making the conditions clearer.

> 

> Signed-off-by: David Gow <davidgow@google.com>


Looks good.

Does this go through KUnit tree or you want me to take it? In case of
the former feel free to add:

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Shuah Khan May 14, 2021, 7:57 p.m. UTC | #3 
On 5/14/21 1:27 AM, David Gow wrote:
> On Fri, May 14, 2021 at 2:08 PM Mika Westerberg

> <mika.westerberg@linux.intel.com> wrote:

>>

>> Hi,

>>

>> On Thu, May 13, 2021 at 12:32:01PM -0700, David Gow wrote:

>>> With some of the stricter type checking in KUnit's EXPECT macros

>>> removed, several casts in the thunderbolt KUnit tests are no longer

>>> required.

>>>

>>> Remove the unnecessary casts, making the conditions clearer.

>>>

>>> Signed-off-by: David Gow <davidgow@google.com>

>>

>> Looks good.

>>

>> Does this go through KUnit tree or you want me to take it? In case of

>> the former feel free to add:

>>

>> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

>>

> 

> Thanks. I think it's probably easier for this to go in via the KUnit

> tree, unless Brendan or Shuah have any objections.

> 


It is fine either way unless there are dependencies on the KUnit
tree. I can take this in through KUnit once Brendan looks at it.

thanks,
-- Shuah
diff mbox series

Patch

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 49601c4b98b8..4c56ffcb7403 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -775,7 +775,6 @@  void kunit_do_assertion(struct kunit *test,
 do {									       \
 	typeof(left) __left = (left);					       \
 	typeof(right) __right = (right);				       \
-	((void)__typecheck(__left, __right));				       \
 									       \
 	KUNIT_ASSERTION(test,						       \
 			__left op __right,				       \