On Tue, Feb 11, 2025 at 3:40 AM Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx> wrote: > > On Tue, Feb 11 2025, David Gow <davidgow@xxxxxxxxxx> wrote: > > > On Mon, 10 Feb 2025 at 19:57, Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx> wrote: > >> > >> On Fri, Feb 07 2025, Tamir Duberstein <tamird@xxxxxxxxx> wrote: > >> > >> > On Fri, Feb 7, 2025 at 5:01 AM Rasmus Villemoes > >> > <linux@xxxxxxxxxxxxxxxxxx> wrote: > >> >> > >> >> On Thu, Feb 06 2025, Tamir Duberstein <tamird@xxxxxxxxx> 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 Rasmus, does v3 meet your needs? https://lore.kernel.org/all/20250210-printf-kunit-convert-v3-1-ee6ac5500f5e@xxxxxxxxx/ Weirdly the cover letter seems to be missing on lore, should I resend?
