Re: [PATCH v2 2/6] unit-tests: add for_test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 23/07/2024 14:24, Phillip Wood wrote:
On 22/07/2024 20:13, Kyle Lippincott wrote:
On Sat, Jul 20, 2024 at 11:22 PM René Scharfe <l.s.r@xxxxxx> wrote:

I think
having multiple test functions is generally easier to understand, and
the overhead isn't really relevant. It's not _as_ compact in the
source file, and requires that we have both the TEST statement and the
function (and forgetting the TEST statement means that we won't invoke
the function). If that is the main issue we're facing here, I wonder
if there's some other way of resolving that (such as unused function
detection via some compiler flags; even if it's not detected on all
platforms, getting at least one of the CI platforms should be
sufficient to prevent the issue [but we should target as many as
possible, so it's caught earlier than "I tried to send it to the
list"])

I also have a preference for function based tests but I do think something like for_test() can be useful in certain situations.

I'm no-longer convinced that this is really the case

For example in test-ctype.c where testing the ctype macros leads to a lot of repetition or a giant macro with a function based approach.

Having re-read the history of t/unit-tests/t-ctype.c I don't think the repetition in the original version was really that bad. The objection to it seems to have been that one had to write the class name (for example isalpha()) twice - once when defining the test function and again when calling it. I'm not sure that is really worth worrying about.

Another example is where we are using a helper function with several inputs and we would prefer to write

for_test("test 1") {
     int input[] = ...
     int expect[] = ...

     test_helper(input, expect);

...

for_test("test 10") {
     int input[] = ...
     int expect[] = ...

     test_helper(input, expect);
}

One can use blocks to achieve the same outcome with the TEST() macro

{
      int input[] = ...
      int expect[] = ...

      TEST(test_helper(input, expect), "test 1");
}

So overall I'm less convinced that adding something like for_test() is necessary and I'm very convinced that calling it for_test() and using "continue" to exit a test early is going to confuse contributors.

Best Wishes

Phillip

rather then declaring all the input and expect variables up front with

     int input_1 = ...
     int input_2 = ...
     ...
     int expect_1 = ...
     int expect_2 = ...

     TEST(test_helper(input_1, expect_1), "test 1");
     ...
     TEST(test_helper(input_10, expect_10), "test 10");

If others agree that this is a good simplification for the people
reading the test code (and hopefully for the test author), I'm fine
with this going in (with a different name). I'm not trying to veto the
concept.

That's my feeling too.

Best Wishes

Phillip


+
  /*
   * Print a test plan, should be called before any tests. If the number
   * of tests is not known in advance test_done() will automatically
--
2.45.2







[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux