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

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!