On 3/16/23 17:59, Rae Moar wrote: > Change the KTAP v2 spec to allow variable prefixes to KTAP lines, > instead of fixed indentation of two spaces. However, the prefix must be > constant on the same level of testing (besides unknown lines). > > This was proposed by Tim Bird in 2021 and then supported by Frank Rowand > in 2022 (see link below). > > Link: https://lore.kernel.org/all/bc6e9ed7-d98b-c4da-2a59-ee0915c18f10@xxxxxxxxx/ Another link to the same thread, but expanded to show all replies in one page is: https://lore.kernel.org/all/bc6e9ed7-d98b-c4da-2a59-ee0915c18f10@xxxxxxxxx/T/#u Near the top of that thread I proposed alternative 1 (essentially what Tim originally suggested, and what Rae proposes here) and alternative 2 (with slight variant 2b). The overall preference seemed to be alternative 1, but if we wanted to provide a method to provide test or system metadata then alternative 2 might provide both a test prefix and metadata. Alternate 1 provides the vast majority of what I need the prefix for, but I think there has been a recent comment that it would be useful to be able to report system metadata (sorry, I haven't found a reference for that yet). In my case, it would be informative to use metadata to report which config options that impact the DT unittests are enabled. > > As cited in the original proposal, it is useful in some Fuego tests to > include an identifier in the prefix. This is an example: > > KTAP version 1 > 1..2 > [batch_id 4] KTAP version 1 > [batch_id 4] 1..2 > [batch_id 4] ok 1 cyclictest with 1000 cycles > [batch_id 4] # problem setting CLOCK_REALTIME > [batch_id 4] not ok 2 cyclictest with CLOCK_REALTIME > not ok 1 check realtime > [batch_id 4] KTAP version 1 > [batch_id 4] 1..1 > [batch_id 4] ok 1 IOZone read/write 4k blocks > ok 2 check I/O performance > > Here is a link to a version of the KUnit parser that is able to parse > variable length prefixes for KTAP version 2. Note that the prefix must > be constant at the same level of testing. > > Link: https://kunit-review.googlesource.com/c/linux/+/5710 > > Signed-off-by: Rae Moar <rmoar@xxxxxxxxxx> > --- > > This idea has already been proposed but I wanted to potentially > restart the discussion by demonstrating this change can by > implemented in the KUnit parser. Let me know what you think. > > Note: this patch is based on Frank's ktap_spec_version_2 branch. > > Documentation/dev-tools/ktap.rst | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/Documentation/dev-tools/ktap.rst b/Documentation/dev-tools/ktap.rst > index ff77f4aaa6ef..ac61fdd97096 100644 > --- a/Documentation/dev-tools/ktap.rst > +++ b/Documentation/dev-tools/ktap.rst Some additional lines of the Spec to be updated (from my alternate 1 email, I haven't checked the current Spec to see if these are the exact changes needed, but at least capture the intent: The "Version lines" format is changed from: KTAP version 1 to: [<prefix string>] KTAP version 1 The "Plan lines" format is changed from: "1..N" to: [<prefix string>] "1..N" The "Test case result lines" format is changed from: <result> <number> [<description>][ # [<directive>] [<diagnostic data>]] to: [<prefix string>] <result> <number> [<description>][ # [<directive>] [<diagnostic data>]] <prefix content is a constant string> I wrote (with a bit of imprecision): Indentation for "Nested tests" follows <prefix string>. The indentation does NOT precede <prefix string>. which was meant to imply that the two space indentation would follow the <prefix string>. The patch I am replying to instead replaces the two space indentation entirely with the <prefix string>. I think this patches' version of indentation is superior to what I suggested. > @@ -192,9 +192,11 @@ starting with another KTAP version line and test plan, and end with the overall > result. If one of the subtests fail, for example, the parent test should also > fail. > > -Additionally, all lines in a subtest should be indented. One level of > -indentation is two spaces: " ". The indentation should begin at the version > -line and should end before the parent test's result line. > +Additionally, all lines in a subtest should be indented. The standard for one > +level of indentation is two spaces: " ". However, any prefix for indentation > +is allowed as long as the prefix is consistent throughout that level of > +testing. The indentation should begin at the version line and should end > +before the parent test's result line. > > "Unknown lines" are not considered to be lines in a subtest and thus are > allowed to be either indented or not indented. I was a little more verbose about "Unknown lines": "Unknown lines" may optionally be prefixed with the <prefix string>, but are not required to be prefixed with the <prefix string>. It is allowed for some "Unknown lines" to not be prefixed with the <prefix string>, even if one or more other "Unknown lines" are prefixed with the <prefix string>. I think combining the intent ("not considered to be lines in a subtest") with the extra verbosity would be useful. > @@ -229,6 +231,19 @@ An example format with multiple levels of nested testing: > not ok 1 example_test_1 > ok 2 example_test_2 > > +An example of a test with two nested subtests using prefixes: > + > +:: > + > + KTAP version 2 > + 1..1 > + [prefix_1] KTAP version 2 > + [prefix_1] 1..2 > + [prefix_1] ok 1 test_1 > + [prefix_1] ok 2 test_2 > + # example passed > + ok 1 example > + The "[" and "]" are meant to indicate an optional field, so the example would be: + KTAP version 2 + 1..1 + prefix_1 KTAP version 2 + prefix_1 1..2 + prefix_1 ok 1 test_1 + prefix_1 ok 2 test_2 + # example passed + ok 1 example + Of course, "[" and "]" are valid characters within the prefix string, so that an example of "[prefix_1]" could be mentioned as a valid example. I would suggest some additional more complex examples: + prefix_0 KTAP version 2 + prefix_0 1..1 + prefix_0 prefix_1 KTAP version 2 + prefix_0 prefix_1 1..2 + prefix_0 prefix_1 ok 1 test_1 + prefix_0 prefix_1 ok 2 test_2 + # example passed + prefix_0 ok 1 example + + KTAP version 2 + 1..2 + prefix_1 KTAP version 2 + prefix_1 1..2 + prefix_1 ok 1 test_a_1 + prefix_1 ok 2 test_a_2 + # example passed + ok 1 example + prefix_2 KTAP version 2 + prefix_2 1..2 + prefix_2 ok 1 test_b_1 + prefix_2 ok 2 test_b_2 + # example passed + ok 2 example + + KTAP version 2 + 1..3 + prefix_1 KTAP version 2 + prefix_1 1..2 + prefix_1 ok 1 test_a_1 + prefix_1 ok 2 test_a_2 + # example passed + ok 1 example + KTAP version 2 + 1..2 + ok 1 test_b_1 + ok 2 test_b_2 + # example passed + ok 2 example + prefix_2 KTAP version 2 + prefix_2 1..2 + prefix_2 ok 1 test_c_1 + prefix_2 ok 2 test_c_2 + # example passed + ok 3 example + > > Major differences between TAP and KTAP > -------------------------------------- > > base-commit: 906f02e42adfbd5ae70d328ee71656ecb602aaf5