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

[Mika Westerberg <mika.westerberg@linux.intel.com>]

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

Nah, either build it into the kernel or load the driver manually:

  # modprobe thunderbolt