lib: Convert test_hexdump.c to KUnit
Subsystems (2)
PATCH v3 1/1 lib: Convert test_hexdump.c to KUnit
Convert test lib/test_hexdump.c to KUnit. More information about KUnit can be found at:
https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html.
KUnit provides a common framework for unit tests in the kernel. KUnit and kselftest are standardizing around KTAP, converting this test to KUnit makes this test output in KTAP which we are trying to make the standard test result format for the kernel.
I ran both the original and converted tests as is to produce the output for success of the test in the two cases. I also ran these tests with a small modification to show the difference in the output for failure of the test in both cases. The modification I made is:
static const char * const test_data_4_le[] __initconst = {
- "7bdb32be", "b293180a", "24c4ba70", "9b34837d",
+ "7bdb32be", "b293180a", "24c4ba70", "9b3483d",
The difference in outputs can be seen here:
https://gist.github.com/arpi-r/38f53a3c65692bf684a6bf3453884b6e
---
Changes v2->v3:
- Modify KUNIT_EXPECT macros used
- kunittest variable made static
- A more detailed commit message
Changes v1->v2:
- Replace KUNIT_EXPECT_EQ() with KUNIT_EXPECT_EQ_MSG()
- Replace KUNIT_EXPECT_NE() with KUNIT_EXPECT_NE_MSG()
- Prints expected and real buffer values in case of test failure
lib/Kconfig.debug | 7 ++-
lib/Makefile | 2 +-
lib/{test_hexdump.c => hexdump_kunit.c} | 73 +++++++++++--------------
3 files changed, 36 insertions(+), 46 deletions(-)
rename lib/{test_hexdump.c => hexdump_kunit.c} (78%)lib/Kconfig.debug +4 -3
@@ -2039,9 +2039,6 @@ config ASYNC_RAID6_TEST If unsure, say N. -config TEST_HEXDUMP - tristate "Test functions located in the hexdump module at runtime" - config TEST_STRING_HELPERS tristate "Test functions located in the string_helpers module at runtime"
@@ -2280,6 +2277,10 @@ config BITS_TEST If unsure, say N. +config HEXDUMP_KUNIT_TEST + tristate "KUnit test for functions located in the hexdump module at runtime" + depends on KUNIT + config TEST_UDELAY tristate "udelay test driver" help
lib/Makefile +1 -1
@@ -55,7 +55,6 @@ obj-$(CONFIG_STRING_SELFTEST) += test_string.o obj-y += string_helpers.o obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o obj-y += hexdump.o -obj-$(CONFIG_TEST_HEXDUMP) += test_hexdump.o obj-y += kstrtox.o obj-$(CONFIG_FIND_BIT_BENCHMARK) += find_bit_benchmark.o obj-$(CONFIG_TEST_BPF) += test_bpf.o
@@ -352,3 +351,4 @@ obj-$(CONFIG_BITFIELD_KUNIT) += bitfield_kunit.o obj-$(CONFIG_BITS_TEST) += bits_kunit.o obj-$(CONFIG_LINEAR_RANGES_TEST) += linear_ranges_kunit.o obj-$(CONFIG_LIST_KUNIT_TEST) += list_kunit.o +obj-$(CONFIG_HEXDUMP_KUNIT_TEST) += hexdump_kunit.o
lib/hexdump_kunit.c renamed from lib/test_hexdump.c +31 -42
142 lines not on this page · raw message
Signature · 6 lines
2.25.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Quoted 7 lines · Arpitha Raghunandan
Convert test
lib/test_hexdump.cto KUnit. More information about KUnit can be found at:
https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html.
KUnit provides a common framework for unit tests in the kernel. KUnit and kselftest are standardizing around KTAP, converting this test to KUnit makes this test output in KTAP which we are trying to make the standard test result format for the kernel.
Below doesn't suit commit message, perhaps adding it after '---' line would be good. In the commit message you can choose one failed case followed by all OK and show the difference.
Quoted 10 lines
I ran both the original and converted tests as is to produce the output for success of the test in the two cases. I also ran these tests with a small modification to show the difference in the output for failure of the test in both cases. The modification I made is:
static const char * const test_data_4_le[]__initconst= {(quoted diff) +1
-1- "7bdb32be", "b293180a", "24c4ba70", "9b34837d", + "7bdb32be", "b293180a", "24c4ba70", "9b3483d",The difference in outputs can be seen here:
https://gist.github.com/arpi-r/38f53a3c65692bf684a6bf3453884b6e
Looks pretty much good, what I'm sad to see is the absence of the test statistics. Any ideas if we can get it back?
Signature · 6 lines
With Best Regards, Andy Shevchenko _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Quoted 9 lines · Andy Shevchenko
Quoted 7 lines · Arpitha Raghunandan
Convert test
lib/test_hexdump.cto KUnit. More information about KUnit can be found at:
https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html.
KUnit provides a common framework for unit tests in the kernel. KUnit and kselftest are standardizing around KTAP, converting this test to KUnit makes this test output in KTAP which we are trying to make the standard test result format for the kernel.
Quoted 4 lines
Below doesn't suit commit message, perhaps adding it after '---' line would be good. In the commit message you can choose one failed case followed by all OK and show the difference.
Okay, I will make this change.
Quoted 8 lines
Quoted 8 lines
I ran both the original and converted tests as is to produce the output for success of the test in the two cases. I also ran these tests with a small modification to show the difference in the output for failure of the test in both cases. The modification I made is:
static const char * const test_data_4_le[]__initconst= {(quoted diff) +1
-1- "7bdb32be", "b293180a", "24c4ba70", "9b34837d", + "7bdb32be", "b293180a", "24c4ba70", "9b3483d",
Quoted 6 lines
Quoted 2 lines
The difference in outputs can be seen here:
https://gist.github.com/arpi-r/38f53a3c65692bf684a6bf3453884b6eLooks pretty much good, what I'm sad to see is the absence of the test statistics. Any ideas if we can get it back?
I could again include variable total_tests as was in the original test. Would that be fine?
Thanks!
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Quoted 2 lines · Arpitha Raghunandan
Quoted 1 line · Andy Shevchenko
On Tue, Dec 1, 2020 at 9:21 AM Arpitha Raghunandan <[email protected]> wrote:
...
Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
I ran both the original and converted tests as is to produce the output for success of the test in the two cases. I also ran these tests with a small modification to show the difference in the output for failure of the test in both cases. The modification I made is:
static const char * const test_data_4_le[]__initconst= {(quoted diff) +1
-1- "7bdb32be", "b293180a", "24c4ba70", "9b34837d", + "7bdb32be", "b293180a", "24c4ba70", "9b3483d",The difference in outputs can be seen here:
https://gist.github.com/arpi-r/38f53a3c65692bf684a6bf3453884b6e
Quoted 6 lines
Quoted 3 lines
Looks pretty much good, what I'm sad to see is the absence of the test statistics. Any ideas if we can get it back?
I could again include variable
total_testsas was in the original test. Would that be fine?
What I;m talking about is the output. How it will be implemented (using the same variable or differently) is up to you. So the point is I want to see the statistics of success/total at the end.
I think this should be done in KUNIT rather than in the individual test cases.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 22 lines · Andy Shevchenko
Quoted 2 lines · Arpitha Raghunandan
Quoted 1 line · Andy Shevchenko
On Tue, Dec 1, 2020 at 9:21 AM Arpitha Raghunandan <[email protected]> wrote:
...
Quoted 15 lines
Quoted 14 lines
Quoted 10 lines
I ran both the original and converted tests as is to produce the output for success of the test in the two cases. I also ran these tests with a small modification to show the difference in the output for failure of the test in both cases. The modification I made is:
static const char * const test_data_4_le[]__initconst= {(quoted diff) +1
-1- "7bdb32be", "b293180a", "24c4ba70", "9b34837d", + "7bdb32be", "b293180a", "24c4ba70", "9b3483d",The difference in outputs can be seen here:
https://gist.github.com/arpi-r/38f53a3c65692bf684a6bf3453884b6eLooks pretty much good, what I'm sad to see is the absence of the test statistics. Any ideas if we can get it back?
Quoted 8 lines
Quoted 2 lines
I could again include variable
total_testsas was in the original test. Would that be fine?What I;m talking about is the output. How it will be implemented (using the same variable or differently) is up to you. So the point is I want to see the statistics of success/total at the end.
I think this should be done in KUNIT rather than in the individual test cases.
I tend to agree here that this really is something for KUnit. At the moment, the tools/testing/kunit/kunit.py script will parse the kernel log and generate these sorts of statistics. I know that needing to run it through a script might seem like a step backwards, but there's no formal place for statistics in the KTAP specification[1] being worked on to standardise kselftest/kunit output formats. Note that there are other parsers for TAP-like formats which are being used with KUnit results, so systems like LAVA could also sum up these statistics. It's also possible, as Arpitha alluded to, to have the test dump them out as a comment.
This won't actually work for this test as-is, though, as the KUnit version is running as a single giant test case (so KUnit believes that 1/1 tests have passed, rather than having any more-detailed statistics). It looks like there are a few ways to split it up a bit which would make it neater (a test each for the for() loops in test_hexdump_init() seems sensible to me), but at the moment, there's not really a way of programmatically generating test cases which KUnit then counts
The "Parameterised Tests"[2] work Arpitha has been working on ought to go some way to helping here, though it won't solve this completely in this initial version. The problem there is that parameterised tests are not reported individually in a way the kunit.py parser can report cleanly, yet, so it'll still only be counted as one test until that's changed (though, at least, that shouldn't require any test-specific work).
My suggestion for the ultimate state of the test would be:
- Split up the test into separate KUnit tests for the different "categories" of tests: (e.g., test_hexdump_set, test_hexdump_overflow_set_ascii, etc)
- Replace the for loops in test_hexdump_init() with parameters, so that KUnit is aware of the original runs.
- Once KUnit and the tooling supports it, these will be reported as subtests. (In the meantime, the results will be listed individually, commented out)
Of course, it'll take a while before all of those KUnit pieces are in place. I personally think that a good compromise would be to just do the first of these for now, which would make kunit_tool give at least a 4/4 rather than 1/1 result. Then, once the parameterised testing work is merged (and perhaps the tooling fixes are finished), the tests could be updated to take advantage of that.
Cheres,
-- David
[1]: https://lore.kernel.org/linux-kselftest/CY4PR13MB1175B804E31E502221BC8163FD830@CY4PR13MB1175.namprd13.prod.outlook.com/T/
[2]: https://lore.kernel.org/linux-kselftest/[email protected]/
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Quoted 3 lines · David Gow
Quoted 1 line · Andy Shevchenko
On Wed, Dec 02, 2020 at 09:51:19AM +0530, Arpitha Raghunandan wrote:
...
Quoted 4 lines
Quoted 4 lines
What I;m talking about is the output. How it will be implemented (using the same variable or differently) is up to you. So the point is I want to see the statistics of success/total at the end.
Quoted 8 lines
Quoted 1 line
I think this should be done in KUNIT rather than in the individual test cases.
I tend to agree here that this really is something for KUnit. At the moment, the
tools/testing/kunit/kunit.pyscript will parse the kernel log and generate these sorts of statistics. I know that needing to run it through a script might seem like a step backwards, but there's no formal place for statistics in the KTAP specification[1] being worked on to standardise kselftest/kunit output formats.
Then it sucks. Fix specification (in a long term) and does it have a comment style of messages that we can have this statistics printed (but maybe not parsed)?
Quoted 5 lines
Note that there are
other parsers for TAP-like formats which are being used with KUnit results, so systems like LAVA could also sum up these statistics. It's also possible, as Arpitha alluded to, to have the test dump them out as a comment.
Fine to me.
Quoted 8 lines
This won't actually work for this test as-is, though, as the KUnit version is running as a single giant test case (so KUnit believes that 1/1 tests have passed, rather than having any more-detailed statistics). It looks like there are a few ways to split it up a bit which would make it neater (a test each for the
for()loops intest_hexdump_init()seems sensible to me), but at the moment, there's not really a way of programmatically generating test cases which KUnit then counts
Fix it, please. We rely on this statistics pretty much.
Quoted 8 lines
The "Parameterised Tests"[2] work Arpitha has been working on ought to go some way to helping here, though it won't solve this completely in this initial version. The problem there is that parameterised tests are not reported individually in a way the
kunit.pyparser can report cleanly, yet, so it'll still only be counted as one test until that's changed (though, at least, that shouldn't require any test-specific work).
Quoted 9 lines
My suggestion for the ultimate state of the test would be:
- Split up the test into separate KUnit tests for the different "categories" of tests: (e.g.,test_hexdump_set,test_hexdump_overflow_set_ascii, etc)
- Replace the for loops intest_hexdump_init()with parameters, so that KUnit is aware of the original runs.
- Once KUnit and the tooling supports it, these will be reported as subtests. (In the meantime, the results will be listed individually, commented out)
I'm fine as long as we have this information printed to the user.
Quoted 6 lines
Of course, it'll take a while before all of those KUnit pieces are in place. I personally think that a good compromise would be to just do the first of these for now, which would make
kunit_toolgive at least a 4/4 rather than 1/1 result. Then, once the parameterised testing work is merged (and perhaps the tooling fixes are finished), the tests could be updated to take advantage of that.
How can we guarantee it will be not forgotten?
Quoted 2 lines
[1]: https://lore.kernel.org/linux-kselftest/CY4PR13MB1175B804E31E502221BC8163FD830@CY4PR13MB1175.namprd13.prod.outlook.com/T/
[2]: https://lore.kernel.org/linux-kselftest/[email protected]/
Signature · 6 lines
With Best Regards, Andy Shevchenko _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Quoted 21 lines · Andy Shevchenko
Quoted 3 lines · David Gow
Quoted 1 line · Andy Shevchenko
On Wed, Dec 02, 2020 at 09:51:19AM +0530, Arpitha Raghunandan wrote:
...
Quoted 4 lines
Quoted 4 lines
What I;m talking about is the output. How it will be implemented (using the same variable or differently) is up to you. So the point is I want to see the statistics of success/total at the end.
Quoted 8 lines
Quoted 1 line
I think this should be done in KUNIT rather than in the individual test cases.
I tend to agree here that this really is something for KUnit. At the moment, the
tools/testing/kunit/kunit.pyscript will parse the kernel log and generate these sorts of statistics. I know that needing to run it through a script might seem like a step backwards, but there's no formal place for statistics in the KTAP specification[1] being worked on to standardise kselftest/kunit output formats.
Quoted 3 lines
Then it sucks. Fix specification (in a long term) and does it have a comment style of messages that we can have this statistics printed (but maybe not parsed)?
I should clarify: there's nothing in the spec which explicitly defines a place for such statistics (nor anything which requires them). There are "diagnostic" lines which are not parsed, and so it'd be possible to output statistics there. KUnit itself doesn't at present, but allows individual tests to log diagnostic lines which could be such statistics, particularly in cases like this where the full structure of the tests aren't quite exposed to the framework.
Quoted 17 lines
Quoted 5 lines
Note that there are
other parsers for TAP-like formats which are being used with KUnit results, so systems like LAVA could also sum up these statistics. It's also possible, as Arpitha alluded to, to have the test dump them out as a comment.Fine to me.
Quoted 8 lines
This won't actually work for this test as-is, though, as the KUnit version is running as a single giant test case (so KUnit believes that 1/1 tests have passed, rather than having any more-detailed statistics). It looks like there are a few ways to split it up a bit which would make it neater (a test each for the
for()loops intest_hexdump_init()seems sensible to me), but at the moment, there's not really a way of programmatically generating test cases which KUnit then counts
Quoted 1 line
Fix it, please. We rely on this statistics pretty much.
The hope is that the Parameterised Test feature will make this possible (though, as mentioned, there are a few other issues around then making those statistics available, but we should be able to work through those).
It may be a silly question, but what is it that makes these statistics useful in this test? Maybe I'm misunderstanding something, but I'd've thought that the important things were whether or not _all_ tests had passed, and -- if not --- _which_ ones had failed. Is the count of failing cases within a test like this really that useful for debugging, or is it more for comparing against different versions? Either way, we'll try to make sure they're available.
Quoted 18 lines
Quoted 8 lines
The "Parameterised Tests"[2] work Arpitha has been working on ought to go some way to helping here, though it won't solve this completely in this initial version. The problem there is that parameterised tests are not reported individually in a way the
kunit.pyparser can report cleanly, yet, so it'll still only be counted as one test until that's changed (though, at least, that shouldn't require any test-specific work).Quoted 9 lines
My suggestion for the ultimate state of the test would be:
- Split up the test into separate KUnit tests for the different "categories" of tests: (e.g.,test_hexdump_set,test_hexdump_overflow_set_ascii, etc)
- Replace the for loops intest_hexdump_init()with parameters, so that KUnit is aware of the original runs.
- Once KUnit and the tooling supports it, these will be reported as subtests. (In the meantime, the results will be listed individually, commented out)
Quoted 1 line
I'm fine as long as we have this information printed to the user.
Okay -- Arpitha: does this seem like a sensible approach to you?
If printing it to the kernel log is really essential, I'll have a look into how we can do that in KUnit.
Quoted 8 lines
Quoted 6 lines
Of course, it'll take a while before all of those KUnit pieces are in place. I personally think that a good compromise would be to just do the first of these for now, which would make
kunit_toolgive at least a 4/4 rather than 1/1 result. Then, once the parameterised testing work is merged (and perhaps the tooling fixes are finished), the tests could be updated to take advantage of that.How can we guarantee it will be not forgotten?
Thinking about it further, if we can get the parameterised testing stuff in 5.11 as planned, then any tooling/output fixes done later would automatically apply. Maybe rather than doing an intermediate version with just the first splitting up of tests, we try it with the current parameterised test patches, and we can possibly prototype some kernel-side statistics reporting which should work. To be honest, though, the subtest support on the kunit_tool side is likely to take quite a while, so it would be nice to have something (like statistics in the kernel log) which ameliorate the problem in the meantime.
I'll have a bit of a play around with the test output and parsing code this week and see if there's a simple change that can get us most of the way there. I think something should be possible if the test uses the Parameterised testing feature.
Cheers,
-- David
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees