On 06/15/2017 10:48 AM, Bird, Timothy wrote: > > >> -----Original Message----- >> From: Shuah Khan on Thursday, June 15, 2017 9:49 AM >> >> On 06/14/2017 06:40 PM, Bird, Timothy wrote: >>>> -----Original Message----- >>>> From: Alice Ferrazzi on Thursday, June 15, 2017 8:38 AM >>>> >>>> On Thu, Jun 15, 2017 at 8:30 AM, Bird, Timothy <Tim.Bird@xxxxxxxx >>>> <mailto:Tim.Bird@xxxxxxxx> > wrote: >>>> >>>> >>>> > -----Original Message----- >>>> > From: Alice Ferrazz on Thursday, June 15, 2017 7:33 AM >>>> > >>>> > Make the kcmp test output in the TAP13 format by using the >>>> > TAP13 output functions defined in kselftest.h >>>> > >>>> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx >>>> <mailto:gregkh@xxxxxxxxxxxxxxxxxxx> > >>>> > Signed-off-by: Paul Elder <paul.elder@xxxxxxxx >>>> <mailto:paul.elder@xxxxxxxx> > >>>> > Signed-off-by: Alice Ferrazzi <alice.ferrazzi@xxxxxxxxx >>>> <mailto:alice.ferrazzi@xxxxxxxxx> > >>>> > --- >>>> > v3: Add kcmp test port to TAP13 - Alice >>>> > >>>> > tools/testing/selftests/kcmp/kcmp_test.c | 33 ++++++++++++++-- >>>> ----------- >>>> > ----- >>>> > 1 file changed, 14 insertions(+), 19 deletions(-) >>>> > >>>> > diff --git a/tools/testing/selftests/kcmp/kcmp_test.c >>>> > b/tools/testing/selftests/kcmp/kcmp_test.c >>>> > index a5a4da856d..b007e62e03 100644 >>>> > --- a/tools/testing/selftests/kcmp/kcmp_test.c >>>> > +++ b/tools/testing/selftests/kcmp/kcmp_test.c >>>> > @@ -35,30 +35,28 @@ int main(int argc, char **argv) >>>> > pid1 = getpid(); >>>> > >>>> > if (fd1 < 0) { >>>> > - perror("Can't create file"); >>>> > - ksft_exit_fail(); >>>> > + ksft_exit_fail_msg("Can't create file"); >>>> > } >>>> > >>>> > pid2 = fork(); >>>> > if (pid2 < 0) { >>>> > - perror("fork failed"); >>>> > - ksft_exit_fail(); >>>> > + ksft_exit_fail_msg("fork failed"); >>>> > } >>>> > >>>> > if (!pid2) { >>>> > int pid2 = getpid(); >>>> > int ret; >>>> > + char buf[512]; >>>> > >>>> > fd2 = open(kpath, O_RDWR, 0644); >>>> > if (fd2 < 0) { >>>> > - perror("Can't open file"); >>>> > - ksft_exit_fail(); >>>> > + ksft_exit_fail_msg("Can't open file"); >>>> > } >>>> > >>>> > /* An example of output and arguments */ >>>> > - printf("pid1: %6d pid2: %6d FD: %2ld FILES: %2ld VM: %2ld " >>>> > + snprintf(&buf[0], sizeof(buf) ,"pid1: %6d pid2: %6d FD: %2ld >>>> > FILES: %2ld VM: %2ld " >>>> > "FS: %2ld SIGHAND: %2ld IO: %2ld SYSVSEM: %2ld " >>>> > - "INV: %2ld\n", >>>> > + "INV: %2ld", >>>> > pid1, pid2, >>>> > sys_kcmp(pid1, pid2, KCMP_FILE, fd1, fd2), >>>> > sys_kcmp(pid1, pid2, KCMP_FILES, 0, 0), >>>> > @@ -73,30 +71,27 @@ int main(int argc, char **argv) >>>> > >>>> > /* This one should return same fd */ >>>> > ret = sys_kcmp(pid1, pid2, KCMP_FILE, fd1, fd1); >>>> > + ksft_test_result_pass(buf); >>>> >>>> This uses a string with all kinds of dynamic data (particularly pid >>>> values) as the test description. Please use only non-dynamic >>>> strings for test descriptions. If it is desired to output information >>>> about >>>> the data used for the test, that should be in a YAML block. >>>> >>>> >>>> >>>> As now we don't have YAML block implementation yet, so I thought we >> can >>>> implement it in a later moment. >>> >>> TAP13 uses the YAML block for more structured output parsing, but also >>> supports the backwards compatible comment format, for test diagnostic >>> output. That is, you can always have the output look like this: >>> >>> ok 3 validate that sys_kcmp works >>> # pid1: 27897 pid2: 27898 FD: 1 FILES: 2 VM: 2 FS: 2 SIGHAND: 1 IO: 0 >> SYSVSEM: 0 INV: -1 >>> >>> I would recommend that we either: 1) implement some basic >>> ksft_output_yaml(), or 2) just use printfs to output commented >>> lines, so that this information text is not lost. >>> >>> However, I do think we need to migrate to non-dynamic strings in >>> the test description lines. >> >> We do have cases where there is need to include error string in >> the test output. For example the cases where we use perror() or >> strerror(). It is important to have that information and we will >> need to use dynamic strings. >> >> YAML requirement could become too restrictive for kselftest if >> it requires non-dynamic strings. > > The YAML lines can have any data you want (dynamic, static, whatever). > > TAP consist of 4 major elements: > the header > test description lines > additional data lines > the plan count > > The test description lines are the ones that start with 'ok' or 'not ok', > and include a number and a description of the test. It is these lines > that should have a static string that is the same whether the test > succeeds or fails. If there is any dynamic data in these lines (such as > pids, which are unique per run), then it will cause diffs of output > between two runs to fail (gratuitously), even when the result of > the test is the same. > > The additional data lines can either be in YAML format, which is > human-readable but structured, or in "diagnostic" format, which > is just commented strings (lines with leading '#' characters). > > Either of these support arbitrary strings, and can appropriately > be used to display dynamic data, such as reasons for failure, or > data about the test, such as pids, handles, memory sizes, commands, > etc. I think I am with you on what you meant by non-dynamic. For example, if test fails because of some xyz reason: not ok 3 File not found message: actual file name etc. details of the failure with any dynamic data So the "not ok ..." line has to the static in terms from run to run. The rest of the message that could change based on run-time conditions which dynamic should be a separate/additional information. > > I don't want kernel developers to have to learn YAML (or indeed > even to learn much about TAP), so I think the extra data lines should > be output with a helper routine. This could be as simple as: > ksft_output_data(msg); Yes. Agreed. > > This is what we would use to output lines that are currently going > to perror() or strerror(). > There is no need. I understand what you are saying. thanks, -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
