linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Gow <davidgow@google.com>
To: Brendan Higgins <brendanhiggins@google.com>,
	Daniel Latypov <dlatypov@google.com>,
	Shuah Khan <skhan@linuxfoundation.org>
Cc: David Gow <davidgow@google.com>,
	kunit-dev@googlegroups.com, linux-kselftest@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH 1/2] kunit: Do not typecheck binary assertions
Date: Thu,  6 May 2021 22:09:07 -0700	[thread overview]
Message-ID: <20210507050908.1008686-1-davidgow@google.com> (raw)

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
for the code to compile.

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>
---

I appreciate that this is probably a bit controversial (and, indeed, I
was a bit hesitant about sending it out myself), but after sitting on it
for a few days, I still think this is probably an improvement overall.

The second patch does fix what I think is an actual bug, though, so even
if this isn't determined to be a good idea, it (or some equivalent)
should probably go through.

Cheers,
-- David

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

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,				       \
-- 
2.31.1.607.g51e8a6a459-goog


             reply	other threads:[~2021-05-07  5:09 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-07  5:09 David Gow [this message]
2021-05-07  5:09 ` [PATCH 2/2] kunit: Assign strings to 'const char*' in STREQ assertions David Gow
2021-05-07  7:28   ` Daniel Latypov
2021-05-07 20:07   ` Brendan Higgins
2021-05-07  7:25 ` [PATCH 1/2] kunit: Do not typecheck binary assertions Daniel Latypov
2021-05-07 20:05 ` Brendan Higgins
2021-05-08  5:56   ` David Gow

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=20210507050908.1008686-1-davidgow@google.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=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).