On Thu, Dec 18, 2025 at 06:15:31PM +0800, David Gow wrote: > On Thu, 18 Dec 2025 at 17:47, Ojaswin Mujoo <[email protected]> wrote: > > > > On Thu, Dec 18, 2025 at 04:58:33PM +0800, David Gow wrote: > > > On Thu, 18 Dec 2025 at 15:20, Ojaswin Mujoo <[email protected]> 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/[email protected]/ > > > — 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/[email protected]/
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
