On Fri, Feb 11, 2022 at 04:49:21PM +0100, Ricardo Ribalda wrote:
> Hi Mika
>
> On Fri, 11 Feb 2022 at 11:08, Mika Westerberg
> <mika.westerberg@xxxxxxxxxxxxxxx> 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@xxxxxxxxxx>
> > > Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> > > ---
> >
> > ...
> >
> > > @@ -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?
Nah, either build it into the kernel or load the driver manually:
# modprobe thunderbolt
