Message-ID: <aUPbaCU-w_SI5ezq@li-dc0c254c-257c-11b2-a85c-98b6c1322444.ibm.com>
Date: Thu, 18 Dec 2025 16:16:00 +0530
From: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
To: David Gow <davidgow@...gle.com>
Cc: linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com,
        Shuah Khan <skhan@...uxfoundation.org>,
        Brendan Higgins <brendan.higgins@...ux.dev>,
        Rae Moar <raemoar63@...il.com>, linux-kernel@...r.kernel.org
Subject: Re: Issue in parsing of tests that use KUNIT_CASE_PARAM

On Thu, Dec 18, 2025 at 06:15:31PM +0800, David Gow wrote:
> On Thu, 18 Dec 2025 at 17:47, Ojaswin Mujoo <ojaswin@...ux.ibm.com> wrote:
> >
> > On Thu, Dec 18, 2025 at 04:58:33PM +0800, David Gow wrote:
> > > On Thu, 18 Dec 2025 at 15:20, Ojaswin Mujoo <ojaswin@...ux.ibm.com> wrote:
> > > >
> > > > On Wed, Dec 17, 2025 at 08:47:57PM +0530, Ojaswin Mujoo wrote:
> > > > > Hello,
> > > > >
> > > > > While writing some Kunit tests for ext4 filesystem, I'm encountering an
> > > > > issue in the way we display the diagnostic logs upon failures, when
> > > > > using KUNIT_CASE_PARAM() to write the tests.
> > > > >
> > > > > This can be observed by patching fs/ext4/mballoc-test.c to fail
> > > > > and print one of the params:
> > > > >
> > > > > --- a/fs/ext4/mballoc-test.c
> > > > > +++ b/fs/ext4/mballoc-test.c
> > > > > @@ -350,6 +350,8 @@ static int mbt_kunit_init(struct kunit *test)
> > > > >         struct super_block *sb;
> > > > >         int ret;
> > > > >
> > > > > +       KUNIT_FAIL(test, "Failed: blocksize_bits=%d", layout->blocksize_bits);
> > > > > +
> > > > >         sb = mbt_ext4_alloc_super_block();
> > > > >         if (sb == NULL)
> > > > >                 return -ENOMEM;
> > > > >
> > > > > With the above change, we can observe the following output (snipped):
> > > > >
> > > > > [18:50:25] ============== ext4_mballoc_test (7 subtests) ==============
> > > > > [18:50:25] ================= test_new_blocks_simple  ==================
> > > > > [18:50:25] [FAILED] block_bits=10 cluster_bits=3 blocks_per_group=8192 group_count=4 desc_size=64
> > > > > [18:50:25]     # test_new_blocks_simple: EXPECTATION FAILED at fs/ext4/mballoc-test.c:364
> > > > > [18:50:25] Failed: blocksize_bits=12
> > > > > [18:50:25] [FAILED] block_bits=12 cluster_bits=3 blocks_per_group=8192 group_count=4 desc_size=64
> > > > > [18:50:25]     # test_new_blocks_simple: EXPECTATION FAILED at fs/ext4/mballoc-test.c:364
> > > > > [18:50:25] Failed: blocksize_bits=16
> > > > > [18:50:25] [FAILED] block_bits=16 cluster_bits=3 blocks_per_group=8192 group_count=4 desc_size=64
> > > > > [18:50:25]     # test_new_blocks_simple: EXPECTATION FAILED at fs/ext4/mballoc-test.c:364
> > > > > [18:50:25] Failed: blocksize_bits=10
> > > > > [18:50:25]     # test_new_blocks_simple: pass:0 fail:3 skip:0 total:3
> > > > > [18:50:25] ============= [FAILED] test_new_blocks_simple ==============
> > > > > <snip>
> > > > >
> > > > > Note that the diagnostic logs don't show up correctly. Ideally they
> > > > > should be before test result but here the first [FAILED] test has no
> > > > > logs printed above whereas the last "Failed: blocksize_bits=10" print
> > > > > comes after the last subtest, when it actually corresponds to the first
> > > > > subtest.
> > > > >
> > > > > The KTAP file itself seems to have diagnostic logs in the right place:
> > > > >
> > > > > KTAP version 1
> > > > > 1..2
> > > > >     KTAP version 1
> > > > >     # Subtest: ext4_mballoc_test
> > > > >     # module: ext4
> > > > >     1..7
> > > > >         KTAP version 1
> > > > >         # Subtest: test_new_blocks_simple
> > > >
> > > > So looking into this a bit more and comparing the parameterized output
> > > > with non parameterized output, I'm seeing that the difference is that
> > > > output via KUNIT_CASE_PARAM is not printing the test plan line right
> > > > here. This plan sort of serves as divider between the parent and the 3
> > > > children's logs and without it our parsing logic gets confused. When I
> > > > manually added a "1..3" test plan I could see the parsing work correctly
> > > > without any changes to kunit_parser.py.
> > > >
> > >
> > > Thanks for looking into this!
> > >
> > > There's been a bit of back-and-forth on how to include the test plan
> > > line for the parameterised tests: it's not always possible to know how
> > > many times a test will run in advance if the gen_params function is
> > > particularly complicated.
> > >
> > > We did have a workaround where array parameters would record the array
> > > size, but there were a couple of tests which were wrapping the
> > > gen_params function to skip / add entries which weren't in the array.
> > >
> > > One "fix" would be to use KUNIT_CASE_PARAM_WITH_INIT() and have an
> > > init function which calls kunit_register_params_array(), and then use
> > > kunit_array_gen_params() as the generator function: this has an escape
> > > hatch which will print the test plan.
> > >
> > > Otherwise, as a hack, you could effectively revert
> > > https://lore.kernel.org/linux-kselftest/20250821135447.1618942-2-davidgow@google.com/
> > > — which would fix the issue (but break some other tests).
> > >
> > > Going through and fixing this properly has been on my to-do list; with
> > > some combination of fixing tests which modify the gen_params function
> > > and improving the parsing to better handle cases without the test
> > > plan.
> > >
> > > Cheers,
> > > -- David
> >
> > Hi David,
> >
> > Thanks for the workaround, KUNIT_CASE_PARAM_WITH_INIT() did the trick!
> >
> > So I'm just wondering if it makes sense to still have a placeholder test
> > plan line in cases we can't determine the number of tests. I think something
> > like 1..X should be enough to not throw off the parsing. (Although I
> > think this might not be exactly compliant to KTAP).
> >
> Hmm… that could be a good idea as something to add to KTAPv2.
> 
> One other option might be to use the proposed KTAP metadata's
> :ktap_test: line as a way of delimiting tests in the parser:
> https://lwn.net/ml/all/20251107052926.3403265-4-rmoar@google.com/

Ohh, nice that can also be a good idea.

> 
> In the meantime, I'm going to look into if we can update all of the
> tests using KUNIT_ARRAY_PARAM() with modified gen_params, so we can
> get the correct test plan in most cases.

Sure, thanks for looking into this issue and providing a quick
workaround!

Regards,
ojaswin

> 
> Cheers,
> -- David

Powered by blists - more mailing lists