Linux-kselftest Archive on lore.kernel.org
 help / color / Atom feed
From: Frank Rowand <frowand.list@gmail.com>
To: "Bird, Tim" <Tim.Bird@sony.com>,
	Brendan Higgins <brendanhiggins@google.com>
Cc: Alan Maguire <alan.maguire@oracle.com>,
	Greg KH <gregkh@linuxfoundation.org>, shuah <shuah@kernel.org>,
	"open list:KERNEL SELFTEST FRAMEWORK" 
	<linux-kselftest@vger.kernel.org>,
	KUnit Development <kunit-dev@googlegroups.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
	Jonathan Corbet <corbet@lwn.net>, David Gow <davidgow@google.com>
Subject: Re: [PATCH v3 kunit-next 1/2] kunit: add debugfs /sys/kernel/debug/kunit/<suite>/results display
Date: Tue, 18 Feb 2020 19:18:52 -0600
Message-ID: <f0d102a8-6501-88d8-d47d-202fd36e55a5@gmail.com> (raw)
In-Reply-To: <MWHPR13MB0895A9AC64475539ECF99987FD110@MWHPR13MB0895.namprd13.prod.outlook.com>

On 2/18/20 2:49 PM, Bird, Tim wrote:
> 
> 
>> -----Original Message-----
>> From:  Brendan Higgins
>>
>> On Wed, Feb 12, 2020 at 7:25 PM Frank Rowand <frowand.list@gmail.com> wrote:
>>>
>>> On 2/7/20 10:58 AM, Alan Maguire wrote:
> 
> ...
> 
>>>> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
>>>> index 9242f93..aec607f 100644
>>>> --- a/lib/kunit/test.c
>>>> +++ b/lib/kunit/test.c
>>>> @@ -10,6 +10,7 @@
>>>>  #include <linux/kernel.h>
>>>>  #include <linux/sched/debug.h>
>>>>
>>>> +#include "debugfs.h"
>>>>  #include "string-stream.h"
>>>>  #include "try-catch-impl.h"
>>>>
>>>> @@ -28,73 +29,91 @@ static void kunit_print_tap_version(void)
>>>>       }
>>>>  }
>>>>
>>>> -static size_t kunit_test_cases_len(struct kunit_case *test_cases)
>>>> +size_t kunit_suite_num_test_cases(struct kunit_suite *suite)
>>>>  {
>>>>       struct kunit_case *test_case;
>>>>       size_t len = 0;
>>>>
>>>> -     for (test_case = test_cases; test_case->run_case; test_case++)
>>>> +     kunit_suite_for_each_test_case(suite, test_case)
>>>>               len++;
>>>>
>>>>       return len;
>>>>  }
>>>> +EXPORT_SYMBOL_GPL(kunit_suite_num_test_cases);
>>>>
>>>>  static void kunit_print_subtest_start(struct kunit_suite *suite)
>>>>  {
>>>>       kunit_print_tap_version();
>>>> -     pr_info("\t# Subtest: %s\n", suite->name);
>>>> -     pr_info("\t1..%zd\n", kunit_test_cases_len(suite->test_cases));
>>>> +     kunit_log(KERN_INFO, suite, "# Subtest: %s", suite->name);
>>>> +     kunit_log(KERN_INFO, suite, "1..%zd",
>>>> +               kunit_suite_num_test_cases(suite));
>>>
>>> The subtest 'is a TAP stream indented 4 spaces'.  (So the old code was
>>> also incorrect since it indented with a tab.)
>>
>> Whoops.
>>
>> I agree that fixing tabs to spaces is probably the easiest thing to do
>> here; nevertheless, I think this might be a good time to talk about
>> other deviations from the spec and what to do about it. This might
>> also be a good time to bring up Tim's comment at LPC last year about
>> forking TAP. Arguably I already have given that TAP14 is still under
>> review and is consequently subject to change.
>>
>> Additionally, the way I report expectation/assertion failures are my
>> own extension to the TAP spec. I did this because at the time I wasn't
>> ready to open the can of worms that was adding a YAML serializer to
>> the Linux kernel; I mentioned adding a YAML serializer at LPC and
>> people didn't seem super thrilled with the idea.
> 
> I'm not sure I follow.  Are you talking about writing YAML or interpreting
> YAML.  You don't need a serializer to write YAML.  It can be done 
> with straight text output.  I guess it depends on the scope of what you
> envision.  Even if you want to do more than trivial structured output,
> I don't think you'll need a full serializer.  (IOW, I think you could sneak
> something in and just call it a test output formatter.  Just don't call it YAML
> and most people won't notice. :-)

A serializer is a red herring.  Just drop the entire label and concept.
What is already in KUnit is the equivalent, printing out (eg through
printk() or however printk() is wrapped) simple text of the form (by
example) of

  label: value
  label: value
  label:
    label: value
    label: value
  label:
     label:
       label: value
       label: value

So basically
  - label/value pairs
  - label without value indicating a node or block (another level)
  - some rules about the format of value
  - indentation indicating a node or block

That is something really simple.  No need for any fancy coding to
encapsulate.

-Frank

> 
>>
>> Further both the TAP implementation here as well as what is in
>> kselftest have arbitrary kernel output mixed in with TAP output, which
>> seems to be a further deviation from the spec.
> Well that's a different kettle of worms, and really argues for staying
> with something that is strictly line-based.
> 
>>
>> In an effort to do this, and so that at the very least I could
>> document what I have done here, I have been looking into getting a
>> copy of TAP into the kernel. Unfortunately, TAP appears to have some
>> licensing issues. TAP says that it can be used/modified "under the
>> same terms as Perl itself" and then provides a dead link. I filed a
>> pull request to update the licence to the Perl Artistic Licence 1.0
>> since I believe that is what they are referencing; however, I have not
>> heard back from them yet.
> 
> When you say "getting a copy of TAP into the kernel", I presume you mean
> an existing implementation to produce TAP output?  Or are you talking about
> a TAP interpreter?  I'm not sure the former needs to use an existing implementation.
> 
> I previously volunteered (in Lisbon) to write up the TAP deviations,
> and never got around to it.   Sorry about that. I can try to work on it now if
> people are still interested.
>  -- Tim
> 
> [rest of patch omitted]
> 
> 


  parent reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-07 16:58 [PATCH v3 kunit-next 0/2] kunit: add debugfs representation to show results/run tests Alan Maguire
2020-02-07 16:58 ` [PATCH v3 kunit-next 1/2] kunit: add debugfs /sys/kernel/debug/kunit/<suite>/results display Alan Maguire
2020-02-11 21:58   ` Brendan Higgins
2020-02-13  3:25   ` Frank Rowand
2020-02-17 15:45     ` Alan Maguire
2020-02-17 17:04       ` Frank Rowand
2020-02-18 19:46     ` Brendan Higgins
2020-02-18 20:49       ` Bird, Tim
2020-02-18 22:03         ` Brendan Higgins
2020-02-18 22:28           ` Bird, Tim
2020-02-19  1:18         ` Frank Rowand [this message]
2020-02-19  1:10       ` Frank Rowand
2020-02-07 16:58 ` [PATCH v3 kunit-next 2/2] kunit: update documentation to describe debugfs representation Alan Maguire
2020-02-11 22:01   ` Brendan Higgins
2020-02-13  3:25   ` Frank Rowand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f0d102a8-6501-88d8-d47d-202fd36e55a5@gmail.com \
    --to=frowand.list@gmail.com \
    --cc=Tim.Bird@sony.com \
    --cc=alan.maguire@oracle.com \
    --cc=brendanhiggins@google.com \
    --cc=corbet@lwn.net \
    --cc=davidgow@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-kselftest Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-kselftest/0 linux-kselftest/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-kselftest linux-kselftest/ https://lore.kernel.org/linux-kselftest \
		linux-kselftest@vger.kernel.org
	public-inbox-index linux-kselftest

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kselftest


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git