Re: [PATCH v5 3/6] thunderbolt: test: use NULL macros

[Ricardo Ribalda <ribalda@chromium.org>]

Hi Mika

On Fri, 11 Feb 2022 at 11:08, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> Hi,
>
> On Fri, Feb 11, 2022 at 10:41:30AM +0100, Ricardo Ribalda wrote:
> > Replace the NULL checks with the more specific and idiomatic NULL macros.
> >
> > Acked-by: Daniel Latypov <dlatypov@google.com>
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
>
> ...
>
> > @@ -2496,50 +2496,50 @@ static void tb_test_property_parse(struct kunit *test)
> >       struct tb_property *p;
> >
> >       dir = tb_property_parse_dir(root_directory, ARRAY_SIZE(root_directory));
> > -     KUNIT_ASSERT_TRUE(test, dir != NULL);
> > +     KUNIT_ASSERT_NOT_NULL(test, dir);
> >
> >       p = tb_property_find(dir, "foo", TB_PROPERTY_TYPE_TEXT);
> > -     KUNIT_ASSERT_TRUE(test, !p);
> > +     KUNIT_ASSERT_NOT_NULL(test, p);
>
> This should be KUNIT_ASSERT_NULL(test, p) as we specifically want to
> check that the property does not exist (!p is same as p == NULL).
>
> >       p = tb_property_find(dir, "vendorid", TB_PROPERTY_TYPE_TEXT);
> > -     KUNIT_ASSERT_TRUE(test, p != NULL);
> > +     KUNIT_ASSERT_NOT_NULL(test, p);
> >       KUNIT_EXPECT_STREQ(test, p->value.text, "Apple Inc.");
> >
> >       p = tb_property_find(dir, "vendorid", TB_PROPERTY_TYPE_VALUE);
> > -     KUNIT_ASSERT_TRUE(test, p != NULL);
> > +     KUNIT_ASSERT_NOT_NULL(test, p);
> >       KUNIT_EXPECT_EQ(test, p->value.immediate, 0xa27);
> >
> >       p = tb_property_find(dir, "deviceid", TB_PROPERTY_TYPE_TEXT);
> > -     KUNIT_ASSERT_TRUE(test, p != NULL);
> > +     KUNIT_ASSERT_NOT_NULL(test, p);
> >       KUNIT_EXPECT_STREQ(test, p->value.text, "Macintosh");
> >
> >       p = tb_property_find(dir, "deviceid", TB_PROPERTY_TYPE_VALUE);
> > -     KUNIT_ASSERT_TRUE(test, p != NULL);
> > +     KUNIT_ASSERT_NOT_NULL(test, p);
> >       KUNIT_EXPECT_EQ(test, p->value.immediate, 0xa);
> >
> >       p = tb_property_find(dir, "missing", TB_PROPERTY_TYPE_DIRECTORY);
> > -     KUNIT_ASSERT_TRUE(test, !p);
> > +     KUNIT_ASSERT_NOT_NULL(test, p);
>
> Ditto here.
>
> With those fixed (please also run the tests if possible to see that they
> still pass) you can add,
>

Thanks!

To test it I had enabled:
PCI, USB4 and USB4_KUNIT_TEST

and then run it with

./tools/testing/kunit/kunit.py run --jobs=$(nproc) --arch=x86_64

Unfortunately, kunit was not able to run the tests

This hack did the trick:


 int tb_test_init(void)
 {
-       return __kunit_test_suites_init(tb_test_suites);
+       //return __kunit_test_suites_init(tb_test_suites);
+       return 0;
 }

 void tb_test_exit(void)
 {
-       return __kunit_test_suites_exit(tb_test_suites);
+       //return __kunit_test_suites_exit(tb_test_suites);
 }
+
+kunit_test_suites(&tb_test_suite);

I looked into why we do this and I found:

thunderbolt: Allow KUnit tests to be built also when CONFIG_USB4=m


I am a bit confused. The patch talks about build coverage, but even
with that patch reverted if
USB4_KUNIT_TEST=m
then test.c is built.

Shouldn't we simply revert that patch?

Thanks!

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



-- 
Ricardo Ribalda