On Tue, Feb 11 2025, David Gow davidgow@google.com wrote:
On Mon, 10 Feb 2025 at 19:57, Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Fri, Feb 07 2025, Tamir Duberstein tamird@gmail.com wrote:
On Fri, Feb 7, 2025 at 5:01 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Thu, Feb 06 2025, Tamir Duberstein tamird@gmail.com wrote:
I'll have to see the actual code, of course. In general, I find reading code using those KUNIT macros quite hard, because I'm not familiar with those macros and when I try to look up what they do they turn out to be defined in terms of other KUNIT macros 10 levels deep.
But that still leaves a few points. First, I really like that "388 test cases passed" tally or some other free-form summary (so that I can see that I properly hooked up, compiled, and ran a new testcase inside test_number(), so any kind of aggregation on those top-level test_* is too coarse).
This one I'm not sure how to address. What you're calling test cases here would typically be referred to as assertions, and I'm not aware of a way to report a count of assertions.
I'm not sure that's accurate.
The thing is, each of the current test() instances results in four different tests being done, which is roughly why we end up at the 4*97 == 388, but each of those tests has several assertions being done - depending on which variant of the test we're doing (i.e. the buffer length used or if we're passing it through kasprintf), we may do only some of those assertions, and we do an early return in case one of those assertions fail (because it wouldn't be safe to do the following assertions, and the test as such has failed already). So there are far more assertions than those 388.
OTOH, that the number reported is 388 is more a consequence of the implementation than anything explicitly designed. I can certainly live with 388 being replaced by 97, i.e. that each current test() invocation would count as one KUNIT case, as that would still allow me to detect a PEBKAC when I've added a new test() instance and failed to actually run that.
It'd be possible to split things up further into tests, at the cost of it being a more extensive refactoring, if having the more granular count tracked by KUnit were desired.
I think the problem is that kunit is simply not a good framework to do these kinds of tests in, and certainly it's very hard to retrofit kunit after the fact.
It'd also be possible to make
these more explicitly data driven via a parameterised test (so each input/output pair is listed in an array, and automatically gets converted to a KUnit subtest).
So that "array of input/output" very much doesn't work for these specific tests: We really want the format string/varargs to be checked by the compiler, and besides, there's no way to store the necessary varargs and generate a call from those in an array. Moreover, we verify a lot more than just that the correct string is produced; it's also a matter of the right return value regardless of the passed buffer size, etc.
That's also why is nigh impossible to simply change __test() into (another) macro that expands to something that defines an individual struct kunit_case, because the framework is really built around the notion that each case can be represented by a void function call and the name of the test is the stringification of the function name.
So I don't mind the conversion to kunit if that really helps other people, as long as the basic functionality is still present and doesn't impede future extensions - and certainly I don't want to end up in a situation where somebody adds a new %p extension but cannot really add a test for it because kunit makes that hard.
But I hope you all agree that it doesn't make much _sense_ to consider test_number() and test_string() and so on individual "test cases"; the atomic units of test being done in the printf suite is each invocation of the __test() function, with one specific format string/varargs combination.
Rasmus