RE: [PATCH 0/2] Added yaml reason to kselftest

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Shuah Khan on Friday, June 16, 2017 12:15 AM
> 
> On 06/15/2017 08:50 AM, Alice Ferrazz wrote:
> > From: Alice Ferrazzi <alice.ferrazzi@xxxxxxxxx>
> >
> > Added yaml output to kselftest.
> > Added reason yaml output to kcmp.
> >
> > Alice Ferrazzi (2):
> >   Added yaml output to kselftest.
> >   Used the reason on kcmp.
> >
> >  tools/testing/selftests/kcmp/kcmp_test.c | 12 ++++++------
> >  tools/testing/selftests/kselftest.h      | 12 +++++++++---
> >  2 files changed, 15 insertions(+), 9 deletions(-)
> >
> 
> Hi Alice,
> 
> Let's hold off on YAML block for a bit. I am concerned about
> the non-dynamic string requirement Tim brought up. So let's
> wait on this series until we understand clearly what are the
> YAML block requirements, before implementing support for it.

I agree with this.  I know I suggested this change, so I'm sorry
Alice for asking for something prematurely.  In further consideration
I think we need to review what we want to do here.

> 
> As I pointed out in my response to the earlier discussion on
> this [PATCH v3 5/5] kselftest: kcmp: Port kcmp test to TAP v13,
> there are few things I am concerned about:
> 
> 1. Being able to append strerror(errorno) information to the
>    fail/skip messages
Agreed.  I don't want to lose the strerror(errno) information.
It might be useful to have something like ksft_perror(msg);
as a straight-up replacement for where perror is used now.
To conform to TAP output, we would want to ensure these
were called AFTER the ksft_test_result_fail() call.  Right now,
perror() looks like it sometimes precedes the results output.

> 2. Parent/child logic - lots of tests run the actual test in a
>    child process.
Yes.  This raises  an interesting set of issues. Since the parent
and child output are interleaved, the final test log output is affected
by process scheduling.  This has obvious drawbacks for log
reproducibility.

IMHO we should come up with some synchronization mechanisms
between the child and parent, or maybe a convention to have
either one or the other (but not both) perform the test log output.

> 
> I also want to understand what is the value YAML brings to Kselftest
> as a whole, as opposed to saying let's add it because it is in TAP13.

Agreed.  The more I think about it, the more I feel like, for this round, 
we should just stick to the non-YAML format for the extra data lines.
That is, just use the comment-formatted lines, which in the spec is
referred to as 'diagnostic' lines.

# reason: foo was not bar
# pid1: 12646 pid2: 12647 
# expected: -1
# saw: 14

One benefit of this is that it doesn't require anyone to learn YAML.
And a helper function can be called multiple times without breaking
any required structure.  That is, you can append multiple sets of data
to the log output after a test, without worrying about the YAML
open and close delimiters, like so:

ksft_output_test_data("reason: foo was not bar");
ksft_output_test_data("expected: -1\nsaw: 14");

With YAML, this free-form printing becomes harder to do.

If we start to see patterns in the extra data, that we think would be
beneficial for test tools to utilize, we can then impose some schema
on the results and switch to some more structured output (in YAML).
But I think it's premature to do that now.
 -- Tim

P.S. I just want to comment on how great this work is, from my 
standpoint.  Even if we don't get some perfect, structured output
from this first pass, it will still be incredibly useful going forward to have
the output more regularized and machine parsable.

I've watched the DT binding schema work go in fits and starts over the years,
and it's never actually made it into the kernel because it tried to be too much
at first.  I'm very glad to see this effort not paralyzed by a desire to accomplish
too much in the first go.
��.n��������+%������w��{.n�����{��K����n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�

[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux