On Tue, Oct 20, 2020 at 1:08 AM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Mon, Oct 19, 2020 at 03:45:56PM -0700, Daniel Latypov wrote:
Add basic test coverage for files that don't require any config options:
- gcd.c
- lcm.c
- int_sqrt.c
- reciprocal_div.c
(Ignored int_pow.c since it's a simple textbook algorithm.)
These tests aren't particularly interesting, but
- they're chosen as easy to understand examples of how to write tests
- provides a place to add tests for any new files in this dir
- written so adding new test cases to cover edge cases should be easy
Is documentation not enough?
I have recently wrote my first KUnit test and I found documentation pretty good for the start. I think we actually need more complex examples in the code (and useful).
I should have been more clear. The documentation clearly covers the mechanics of how to set up a test suite with some test cases and KUNIT_EXPECT_* calls.
But it doesn't provide much guidance in the way of _how_ to structure tests. Or how to use more advanced features, e.g. there are only two files tree-wide using the _MSG variants of macros: $ ag KUNIT_.*MSG -l --ignore include/kunit/test.h fs/ext4/inode-test.c lib/bitfield_kunit.c (And both happen to be written by people working on/with KUnit).
While the _MSG macros are not perfect, they add some context when calling KUNIT_* in a loop. I already see some tests merged that probably would benefit from using it. Considering the perspective of an outsider whose change broke one of those tests, they'd need to first edit the test to add more debug info to even understand what failed.
But I can see a case for mentioning the _MSG variants in the example test or Documentation/kunit instead of this patch. There doesn't seem to be any mention of them at present in the docs.
To put it in kunit_example_test.c (and have the value be clear), we'd need something like @@ -27,6 +28,9 @@ static void example_simple_test(struct kunit *test) * behavior matched what was expected. */ KUNIT_EXPECT_EQ(test, 1 + 1, 2); + + for (x = 0; x < 2; ++x) + KUNIT_EXPECT_EQ_MSG(test, 42, myfunc(x), "myfunc(%d)", x); }
Using and testing an actual function like gcd() et al. felt a bit better than adding a trivial function there.
So, I'm in doubt these test are a good point to start with.
If the above still seems too contrived, I can take a look at where to update kunit/usage.rst instead.
-- With Best Regards, Andy Shevchenko