[PATCH v3 0/2] scanf: convert self-test to KUnit

Tamir Duberstein posted 2 patches 10 months, 1 week ago
There is a newer version of this series
MAINTAINERS                          |   2 +-
arch/m68k/configs/amiga_defconfig    |   1 -
arch/m68k/configs/apollo_defconfig   |   1 -
arch/m68k/configs/atari_defconfig    |   1 -
arch/m68k/configs/bvme6000_defconfig |   1 -
arch/m68k/configs/hp300_defconfig    |   1 -
arch/m68k/configs/mac_defconfig      |   1 -
arch/m68k/configs/multi_defconfig    |   1 -
arch/m68k/configs/mvme147_defconfig  |   1 -
arch/m68k/configs/mvme16x_defconfig  |   1 -
arch/m68k/configs/q40_defconfig      |   1 -
arch/m68k/configs/sun3_defconfig     |   1 -
arch/m68k/configs/sun3x_defconfig    |   1 -
arch/powerpc/configs/ppc64_defconfig |   1 -
lib/Kconfig.debug                    |  20 +-
lib/Makefile                         |   2 +-
lib/scanf_kunit.c                    | 800 ++++++++++++++++++++++++++++++++++
lib/test_scanf.c                     | 814 -----------------------------------
tools/testing/selftests/lib/Makefile |   2 +-
tools/testing/selftests/lib/config   |   1 -
tools/testing/selftests/lib/scanf.sh |   4 -
21 files changed, 820 insertions(+), 838 deletions(-)
[PATCH v3 0/2] scanf: convert self-test to KUnit
Posted by Tamir Duberstein 10 months, 1 week ago
This is one of just 3 remaining "Test Module" kselftests (the others
being bitmap and printf), the rest having been converted to KUnit. In
addition to the enclosed patch, please consider this an RFC on the
removal of the "Test Module" kselftest machinery.

I tested this using:

$ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 scanf

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Changes in v3:
- Reduce diff noise in lib/Makefile. (Petr Mladek)
- Split `scanf_test` into a few test cases. New output:
  : =================== scanf (10 subtests) ====================
  : [PASSED] numbers_simple
  : ====================== numbers_list  =======================
  : [PASSED] delim=" "
  : [PASSED] delim=":"
  : [PASSED] delim=","
  : [PASSED] delim="-"
  : [PASSED] delim="/"
  : ================== [PASSED] numbers_list ===================
  : ============ numbers_list_field_width_typemax  =============
  : [PASSED] delim=" "
  : [PASSED] delim=":"
  : [PASSED] delim=","
  : [PASSED] delim="-"
  : [PASSED] delim="/"
  : ======== [PASSED] numbers_list_field_width_typemax =========
  : =========== numbers_list_field_width_val_width  ============
  : [PASSED] delim=" "
  : [PASSED] delim=":"
  : [PASSED] delim=","
  : [PASSED] delim="-"
  : [PASSED] delim="/"
  : ======= [PASSED] numbers_list_field_width_val_width ========
  : [PASSED] numbers_slice
  : [PASSED] numbers_prefix_overflow
  : [PASSED] test_simple_strtoull
  : [PASSED] test_simple_strtoll
  : [PASSED] test_simple_strtoul
  : [PASSED] test_simple_strtol
  : ====================== [PASSED] scanf ======================
  : ============================================================
  : Testing complete. Ran 22 tests: passed: 22
  : Elapsed time: 5.517s total, 0.001s configuring, 5.440s building, 0.067s running
- Link to v2: https://lore.kernel.org/r/20250203-scanf-kunit-convert-v2-1-277a618d804e@gmail.com

Changes in v2:
- Rename lib/{test_scanf.c => scanf_kunit.c}. (Andy Shevchenko)
- Link to v1: https://lore.kernel.org/r/20250131-scanf-kunit-convert-v1-1-0976524f0eba@gmail.com

---
Tamir Duberstein (2):
      scanf: convert self-test to KUnit
      scanf: break kunit into test cases

 MAINTAINERS                          |   2 +-
 arch/m68k/configs/amiga_defconfig    |   1 -
 arch/m68k/configs/apollo_defconfig   |   1 -
 arch/m68k/configs/atari_defconfig    |   1 -
 arch/m68k/configs/bvme6000_defconfig |   1 -
 arch/m68k/configs/hp300_defconfig    |   1 -
 arch/m68k/configs/mac_defconfig      |   1 -
 arch/m68k/configs/multi_defconfig    |   1 -
 arch/m68k/configs/mvme147_defconfig  |   1 -
 arch/m68k/configs/mvme16x_defconfig  |   1 -
 arch/m68k/configs/q40_defconfig      |   1 -
 arch/m68k/configs/sun3_defconfig     |   1 -
 arch/m68k/configs/sun3x_defconfig    |   1 -
 arch/powerpc/configs/ppc64_defconfig |   1 -
 lib/Kconfig.debug                    |  20 +-
 lib/Makefile                         |   2 +-
 lib/scanf_kunit.c                    | 800 ++++++++++++++++++++++++++++++++++
 lib/test_scanf.c                     | 814 -----------------------------------
 tools/testing/selftests/lib/Makefile |   2 +-
 tools/testing/selftests/lib/config   |   1 -
 tools/testing/selftests/lib/scanf.sh |   4 -
 21 files changed, 820 insertions(+), 838 deletions(-)
---
base-commit: a86bf2283d2c9769205407e2b54777c03d012939
change-id: 20250131-scanf-kunit-convert-f70dc33bb34c

Best regards,
-- 
Tamir Duberstein <tamird@gmail.com>
Re: [PATCH v3 0/2] scanf: convert self-test to KUnit
Posted by Kees Cook 9 months, 1 week ago
On Tue, Feb 04, 2025 at 02:25:54PM -0500, Tamir Duberstein wrote:
>  lib/scanf_kunit.c                    | 800 ++++++++++++++++++++++++++++++++++
>  lib/test_scanf.c                     | 814 -----------------------------------

If you can rebase this on -next and folks Ack it, I can carry this in
the "lib/ kunit tests move to lib/tests/" tree.

-Kees

-- 
Kees Cook
Re: [PATCH v3 0/2] scanf: convert self-test to KUnit
Posted by Tamir Duberstein 9 months, 1 week ago
On Fri, Mar 7, 2025 at 11:51 PM Kees Cook <kees@kernel.org> wrote:
>
> On Tue, Feb 04, 2025 at 02:25:54PM -0500, Tamir Duberstein wrote:
> >  lib/scanf_kunit.c                    | 800 ++++++++++++++++++++++++++++++++++
> >  lib/test_scanf.c                     | 814 -----------------------------------
>
> If you can rebase this on -next and folks Ack it, I can carry this in
> the "lib/ kunit tests move to lib/tests/" tree.

I think the plan is to take it through the printk tree. We're still
working on it, here's v9:
https://lore.kernel.org/all/20250307-scanf-kunit-convert-v9-0-b98820fa39ff@gmail.com/
where I've rebased and put the test in lib/tests.

Tamir
Re: [PATCH v3 0/2] scanf: convert self-test to KUnit
Posted by David Gow 10 months, 1 week ago
On Wed, 5 Feb 2025 at 03:26, Tamir Duberstein <tamird@gmail.com> wrote:
>
> This is one of just 3 remaining "Test Module" kselftests (the others
> being bitmap and printf), the rest having been converted to KUnit. In
> addition to the enclosed patch, please consider this an RFC on the
> removal of the "Test Module" kselftest machinery.
>
> I tested this using:
>
> $ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 scanf
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---

Thanks very much for doing this. I'm happy with these changes from a KUnit POV.

Two things I think we need to be careful about:
- This and the printf test are both changing the m68k configs. This is
fine, but could lead to a (harmless) merge conflict, so we should make
that clear and try to avoid having them go up in separate trees. (And,
if one gets merged first, rebase the other.)
- There has been some pushback on some kselftest->kunit conversions in
the past, especially if the test is being used to debug live systems
(which typically don't have CONFIG_KUNIT enabled). I can't personally
imagine that as an issue with scanf (though my imagination isn't
perfect), so I'd doubt it's a problem.

I'm assuming that, as mentioned in v2, these will go in via printk,
not ksefltest/kunit. Either would work for me (but, as mentioned
above, I think this and the printf tests should go in via the same
tree).

This series is:
Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David


> Changes in v3:
> - Reduce diff noise in lib/Makefile. (Petr Mladek)
> - Split `scanf_test` into a few test cases. New output:
>   : =================== scanf (10 subtests) ====================
>   : [PASSED] numbers_simple
>   : ====================== numbers_list  =======================
>   : [PASSED] delim=" "
>   : [PASSED] delim=":"
>   : [PASSED] delim=","
>   : [PASSED] delim="-"
>   : [PASSED] delim="/"
>   : ================== [PASSED] numbers_list ===================
>   : ============ numbers_list_field_width_typemax  =============
>   : [PASSED] delim=" "
>   : [PASSED] delim=":"
>   : [PASSED] delim=","
>   : [PASSED] delim="-"
>   : [PASSED] delim="/"
>   : ======== [PASSED] numbers_list_field_width_typemax =========
>   : =========== numbers_list_field_width_val_width  ============
>   : [PASSED] delim=" "
>   : [PASSED] delim=":"
>   : [PASSED] delim=","
>   : [PASSED] delim="-"
>   : [PASSED] delim="/"
>   : ======= [PASSED] numbers_list_field_width_val_width ========
>   : [PASSED] numbers_slice
>   : [PASSED] numbers_prefix_overflow
>   : [PASSED] test_simple_strtoull
>   : [PASSED] test_simple_strtoll
>   : [PASSED] test_simple_strtoul
>   : [PASSED] test_simple_strtol
>   : ====================== [PASSED] scanf ======================
>   : ============================================================
>   : Testing complete. Ran 22 tests: passed: 22
>   : Elapsed time: 5.517s total, 0.001s configuring, 5.440s building, 0.067s running
> - Link to v2: https://lore.kernel.org/r/20250203-scanf-kunit-convert-v2-1-277a618d804e@gmail.com
>
> Changes in v2:
> - Rename lib/{test_scanf.c => scanf_kunit.c}. (Andy Shevchenko)
> - Link to v1: https://lore.kernel.org/r/20250131-scanf-kunit-convert-v1-1-0976524f0eba@gmail.com
>
> ---
> Tamir Duberstein (2):
>       scanf: convert self-test to KUnit
>       scanf: break kunit into test cases
>
>  MAINTAINERS                          |   2 +-
>  arch/m68k/configs/amiga_defconfig    |   1 -
>  arch/m68k/configs/apollo_defconfig   |   1 -
>  arch/m68k/configs/atari_defconfig    |   1 -
>  arch/m68k/configs/bvme6000_defconfig |   1 -
>  arch/m68k/configs/hp300_defconfig    |   1 -
>  arch/m68k/configs/mac_defconfig      |   1 -
>  arch/m68k/configs/multi_defconfig    |   1 -
>  arch/m68k/configs/mvme147_defconfig  |   1 -
>  arch/m68k/configs/mvme16x_defconfig  |   1 -
>  arch/m68k/configs/q40_defconfig      |   1 -
>  arch/m68k/configs/sun3_defconfig     |   1 -
>  arch/m68k/configs/sun3x_defconfig    |   1 -
>  arch/powerpc/configs/ppc64_defconfig |   1 -
>  lib/Kconfig.debug                    |  20 +-
>  lib/Makefile                         |   2 +-
>  lib/scanf_kunit.c                    | 800 ++++++++++++++++++++++++++++++++++
>  lib/test_scanf.c                     | 814 -----------------------------------
>  tools/testing/selftests/lib/Makefile |   2 +-
>  tools/testing/selftests/lib/config   |   1 -
>  tools/testing/selftests/lib/scanf.sh |   4 -
>  21 files changed, 820 insertions(+), 838 deletions(-)
> ---
> base-commit: a86bf2283d2c9769205407e2b54777c03d012939
> change-id: 20250131-scanf-kunit-convert-f70dc33bb34c
>
> Best regards,
> --
> Tamir Duberstein <tamird@gmail.com>
>
Re: [PATCH v3 0/2] scanf: convert self-test to KUnit
Posted by Tamir Duberstein 10 months, 1 week ago
On Fri, Feb 7, 2025 at 2:33 AM David Gow <davidgow@google.com> wrote:
>
> Thanks very much for doing this. I'm happy with these changes from a KUnit POV.
>
> Two things I think we need to be careful about:
> - This and the printf test are both changing the m68k configs. This is
> fine, but could lead to a (harmless) merge conflict, so we should make
> that clear and try to avoid having them go up in separate trees. (And,
> if one gets merged first, rebase the other.)
> - There has been some pushback on some kselftest->kunit conversions in
> the past, especially if the test is being used to debug live systems
> (which typically don't have CONFIG_KUNIT enabled). I can't personally
> imagine that as an issue with scanf (though my imagination isn't
> perfect), so I'd doubt it's a problem.
>
> I'm assuming that, as mentioned in v2, these will go in via printk,
> not ksefltest/kunit. Either would work for me (but, as mentioned
> above, I think this and the printf tests should go in via the same
> tree).
>
> This series is:
> Reviewed-by: David Gow <davidgow@google.com>
>
> Cheers,
> -- David

Thanks for the review David. Given the discussion on the printf series
I applied the same scrutiny to this series; I reduced the churn, and
kept the original control flow and failure messages.

I'll pick up your Reviewed-by and send v4 shortly.

Cheers.
Tamir