ksummit.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Shuah Khan <shuahkh@osg.samsung.com>
Cc: ksummit-discuss@lists.linuxfoundation.org,
	Carlos O'Donell <carlos@redhat.com>,
	linux-api@vger.kernel.org,
	Thorsten Leemhuis <linux@leemhuis.info>,
	Shuah Khan <shuah@kernel.org>
Subject: Re: [Ksummit-discuss] [MAINTAINERS SUMMIT] & [TECH TOPIC] Improve regression tracking
Date: Wed, 02 Aug 2017 13:04:30 -0500	[thread overview]
Message-ID: <87r2wtluc1.fsf@xmission.com> (raw)
In-Reply-To: <61093c3f-6ad0-c033-a524-f2624f8d55ba@osg.samsung.com> (Shuah Khan's message of "Wed, 2 Aug 2017 11:46:40 -0600")

Shuah Khan <shuahkh@osg.samsung.com> writes:

> On 08/02/2017 11:33 AM, Eric W. Biederman wrote:
>> Shuah Khan <shuahkh@osg.samsung.com> writes:
>> 
>>> On 07/31/2017 10:54 AM, Eric W. Biederman wrote:
>>>> Steven Rostedt <rostedt@goodmis.org> writes:
>>>>
>>>>> On Wed, 5 Jul 2017 09:48:31 -0700
>>>>> Guenter Roeck <linux@roeck-us.net> wrote:
>>>>>
>>>>>> On 07/05/2017 08:27 AM, Steven Rostedt wrote:
>>>>>>> On Wed, 5 Jul 2017 08:16:33 -0700
>>>>>>> Guenter Roeck <linux@roeck-us.net> wrote:  
>>>>>> [ ... ]
>>>>>>>>
>>>>>>>> If we start shaming people for not providing unit tests, all we'll accomplish is
>>>>>>>> that people will stop providing bug fixes.  
>>>>>>>
>>>>>>> I need to be clearer on this. What I meant was, if there's a bug
>>>>>>> where someone has a test that easily reproduces the bug, then if
>>>>>>> there's not a test added to selftests for said bug, then we should
>>>>>>> shame those into doing so.
>>>>>>>   
>>>>>>
>>>>>> I don't think that public shaming of kernel developers is going to work
>>>>>> any better than public shaming of children or teenagers.
>>>>>>
>>>>>> Maybe a friendlier approach would be more useful ?
>>>>>
>>>>> I'm a friendly shamer ;-)
>>>>>
>>>>>>
>>>>>> If a test to reproduce a problem exists, it might be more beneficial to suggest
>>>>>> to the patch submitter that it would be great if that test would be submitted
>>>>>> as unit test instead of shaming that person for not doing so. Acknowledging and
>>>>>> praising kselftest submissions might help more than shaming for non-submissions.
>>>>>>
>>>>>>> A bug that is found by inspection or hard to reproduce test cases are
>>>>>>> not applicable, as they don't have tests that can show a regression.
>>>>>>>   
>>>>>>
>>>>>> My concern would be that once the shaming starts, it won't stop.
>>>>>
>>>>> I think this is a communication issue. My word for "shaming" was to
>>>>> call out a developer for not submitting a test. It wasn't about making
>>>>> fun of them, or anything like that. I was only making a point
>>>>> about how to teach people that they need to be more aware of the
>>>>> testing infrastructure. Not about actually demeaning people.
>>>>>
>>>>> Lets take a hypothetical sample. Say someone posted a bug report with
>>>>> an associated reproducer for it. The developer then runs the reproducer
>>>>> sees the bug, makes a fix and sends it to Linus and stable. Now the
>>>>> developer forgets this and continues on their merry way. Along comes
>>>>> someone like myself and sees a reproducing test case for a bug, but
>>>>> sees no test added to kselftests. I would send an email along the lines
>>>>> of "Hi, I noticed that there was a reproducer for this bug you fixed.
>>>>> How come there was no test added to the kselftests to make sure it
>>>>> doesn't appear again?" There, I "shamed" them ;-)
>>>>
>>>> I just want to point out that kselftests are hard to build and run.
>>>>
>>>> As I was looking at another issue I found a bug in one of the tests.  It
>>>> had defined a constant wrong.  I have a patch.  It took me a week of
>>>> poking at the kselftest code and trying one thing or another (between
>>>> working on other things) before I could figure out which combination of
>>>> things would let the test build and run.
>>>>
>>>> Until kselftests get easier to run I don't think they are something we
>>>> want to push to hard.
>>>>
>>>
>>> I would say it is easy to run ksefltests - "make kseflttest" from the
>>> main Makefile does this for you. You can also run individual tests:
>> 
>> On 4.13-rc1  That doesn't work.
>> 
>> $ make O=$PWD-build -j8 kselftests
>> make[1]: Entering directory 'linux-build'
>> make[1]: *** No rule to make target 'kselftests'.  Stop.
>> make[1]: Leaving directory 'linux-build'
>> Makefile:145: recipe for target 'sub-make' failed
>> make: *** [sub-make] Error 2
>
> It is "make kselftest"

If I include the standard O= to keep my source tree pristine
it still fails.  Which is a practical issue.  Especially because
that "make kselftest" needs to be followed by I think "make mrproper"
to get back to my normal development workflow.

I don't remember exactly what the issue was but I could not get:
tools/testing/selftests/x86/mpx-mini-test.c
tools/testing/selftests/x86/protection_keys.c

to build let alone run when I did "make kselftest"

>> And why I have to use some esoteric command and not just the
>> traditional "make path/to/test/output" to run an individual
>> test is beyond me.
>> 
>
> make kselftest from top level Makefile is a way to run all the tests.
> As I mentioned in my previous email 
>
> "You can also run individual tests:
>
> "make -C tools/testing/selftests/sync" for example to run sync tests.
>
> or you can also run:
>
> make -C tools/testing/selftests/ run_tests

As I said complicated.  That is definitely not the ordinary way of
building things in the kernel tree.

Given that some of those other tests take a little while to run, running
individual tests is actually quite important during development.

Eric

  parent reply	other threads:[~2017-08-02 18:12 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-02 17:51 Thorsten Leemhuis
2017-07-03 16:30 ` Steven Rostedt
2017-07-03 18:50   ` Dan Williams
2017-07-04 19:03   ` Thorsten Leemhuis
2017-07-05 12:45     ` Steven Rostedt
2017-07-05 13:09       ` Carlos O'Donell
2017-07-05 13:27         ` Steven Rostedt
2017-07-05 14:06           ` Greg KH
2017-07-05 14:28             ` Carlos O'Donell
2017-07-05 14:33             ` Steven Rostedt
2017-07-05 14:52               ` Mark Brown
2017-07-05 15:08               ` Carlos O'Donell
2017-07-05 16:10                 ` Steven Rostedt
2017-07-06 11:34                   ` Laurent Pinchart
2017-07-09 13:46               ` Thorsten Leemhuis
2017-07-05 14:33             ` Mark Brown
2017-07-05 14:36               ` Steven Rostedt
2017-07-05 14:50                 ` James Bottomley
2017-07-05 14:56                   ` Steven Rostedt
2017-07-05 15:09                     ` James Bottomley
2017-07-05 15:20                       ` Mark Brown
2017-07-05 15:40                         ` Geert Uytterhoeven
2017-07-05 15:20                       ` Steven Rostedt
2017-07-05 15:32                         ` James Bottomley
2017-07-05 15:43                           ` Steven Rostedt
2017-07-05 18:24                       ` Daniel Vetter
2017-07-05 18:17                 ` Daniel Vetter
2017-07-05 15:16             ` Guenter Roeck
2017-07-05 15:27               ` Steven Rostedt
2017-07-05 15:36                 ` James Bottomley
2017-07-05 16:04                   ` Steven Rostedt
2017-07-05 16:58                     ` James Bottomley
2017-07-05 17:07                       ` Steven Rostedt
2017-07-05 16:48                 ` Guenter Roeck
2017-07-05 16:58                   ` Dan Williams
2017-07-05 17:02                   ` Steven Rostedt
2017-07-06  9:28                     ` Mark Brown
2017-07-06  9:41                       ` Daniel Vetter
2017-07-06 14:53                         ` Theodore Ts'o
2017-07-06 21:28                           ` Daniel Vetter
2017-07-06 14:48                       ` James Bottomley
2017-07-07 10:03                         ` Mark Brown
2017-07-31 16:54                     ` Eric W. Biederman
2017-07-31 20:11                       ` Steven Rostedt
2017-07-31 20:12                         ` Eric W. Biederman
2017-08-02 16:53                       ` Shuah Khan
2017-08-02 17:33                         ` Eric W. Biederman
2017-08-02 17:46                           ` Shuah Khan
2017-08-02 17:58                             ` Shuah Khan
2017-08-02 18:04                             ` Eric W. Biederman [this message]
2017-08-02 18:23                               ` Randy Dunlap
2017-08-02 18:42                               ` Shuah Khan
2017-08-03  3:03                                 ` Theodore Ts'o
2017-08-03 17:42                                   ` Bird, Timothy
2017-08-03 22:11                                     ` Shuah Khan
2017-08-03 18:51                                   ` Shuah Khan
2017-08-04  1:15                                     ` Theodore Ts'o
2017-07-07  3:33                 ` Fengguang Wu
2017-07-07  4:52                   ` Frank Rowand
2017-07-05 15:32               ` Greg KH
2017-07-05 15:36                 ` Carlos O'Donell
2017-07-05 15:52                 ` Steven Rostedt
2017-07-05 18:42                   ` Greg KH
2017-07-05 18:29                 ` Daniel Vetter
2017-07-06 22:24                 ` Shuah Khan
2017-07-06 22:32                   ` Steven Rostedt
2017-07-06 22:40                     ` Shuah Khan
2017-07-05 16:54             ` Dan Williams
2017-07-05 18:45               ` Greg KH
2017-07-05 19:47                 ` Dan Williams
2017-07-05 14:06           ` Carlos O'Donell
2017-07-05 15:47         ` Mark Brown
2017-07-07  6:15 ` Andrei Vagin

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=87r2wtluc1.fsf@xmission.com \
    --to=ebiederm@xmission.com \
    --cc=carlos@redhat.com \
    --cc=ksummit-discuss@lists.linuxfoundation.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux@leemhuis.info \
    --cc=shuah@kernel.org \
    --cc=shuahkh@osg.samsung.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox