linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Mark Brown <broonie@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Shuah Khan <shuah@kernel.org>,
	linux-mm@kvack.org, linux-kselftest@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] selftests/mm: Fix test result reporting in gup_longterm
Date: Fri, 16 May 2025 14:55:24 +0200	[thread overview]
Message-ID: <58dbef73-6e37-46de-9092-365456306b27@redhat.com> (raw)
In-Reply-To: <aCcvxaFc6DE_Mhr1@finisterre.sirena.org.uk>

On 16.05.25 14:29, Mark Brown wrote:
> On Fri, May 16, 2025 at 10:02:16AM +0200, David Hildenbrand wrote:
>> On 15.05.25 10:57, Mark Brown wrote:
> 
>>> The kselftest framework uses the string logged when a test result is
>>> reported as the unique identifier for a test, using it to track test
>>> results between runs. The gup_longterm test completely fails to follow
>>> this pattern, it runs a single test function repeatedly with various
>>> parameters but each result report is a string logging an error message
>>> which is fixed between runs.
> 
>> As the person who wrote that test (that you apparently didn't CC for some
> 
> I just CCed whoever get_maintainers told me to CC for the patch.

For the future, it's a good idea to look for the author of the 
problematic bits.

> 
>> reason), what exactly is the problem with that?
> 
>> We run tests. If all pass, we're happy, if one fails, we investigate.
> 
> None of the tooling is able to either distinguish between the multiple
> tests that are being run in gup_longterm, nor compare the results of
> multiple runs effectively.  If all the tests run they report themselves
> as being duplicates of the same test name, if one of them starts failing
> the effect is that one of the duplicates disappears and we get an
> entirely new test that's never passed reported.  If multiple tests fail
> it's even worse.  This means that UIs displaying test results end up
> reporting things unclearly (Was there a regression or was a new test
> that never worked added?  What was the test?).  Since it's difficult
> to track the tests between runs tooling that does reporting of things
> like "This last worked in X" in the UI doesn't work properly, and tool
> driven bisection of issues similarly struggles since it can't tell
> what's going on with any of the tests between runs.

Okay, so this is purely to make tooling happy. Humans are smart enough 
to figure it out.


What mechanism do we have in place to reliably prevent that from 
happening? And is this at least documented somewhere ("unique identifier 
for a test")>

I guess when using kselftest_harness, we get a single identifier per 
tests (and much less output) just automatically.

 > > Basically, the output is garbled and vastly less useful for people
> running this as a matter of routine or as part of a broader kselftest
> run.  For example with my own automation I probably won't notice that a
> previously working test failed unless every single test fails, and newly
> added tests that never worked are a much lower priority to the point
> where I may never look at them depending on where they are.
> 
> If a selftest is reporting multiple tests it should report them with
> names that are stable and unique.

I'm afraid we have other such tests that report duplicate conditions. 
cow.c is likely another candidate (written by me ;) ).

Probably, the affected tests should be converted to use 
kselftest_harness, where we just report the result for a single tests, 
and not the individual assertions.

That would reduce the output of these tests drastically as well.

So that is likely the way to clean this up properly and make tooling happy?

-- 
Cheers,

David / dhildenb



  reply	other threads:[~2025-05-16 12:55 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-15  8:57 Mark Brown
2025-05-15  9:35 ` Dev Jain
2025-05-15  9:41   ` Mark Brown
2025-05-15  9:43     ` Dev Jain
2025-05-16  8:02 ` David Hildenbrand
2025-05-16 12:29   ` Mark Brown
2025-05-16 12:55     ` David Hildenbrand [this message]
2025-05-16 13:09       ` Mark Brown
2025-05-16 14:12         ` David Hildenbrand
2025-05-16 18:07           ` Mark Brown
2025-05-19 13:28             ` David Hildenbrand
2025-05-19 14:55               ` Mark Brown
2025-05-21 18:48               ` Mark Brown
2025-05-22  8:42                 ` David Hildenbrand
2025-05-22  9:23                   ` Mark Brown

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=58dbef73-6e37-46de-9092-365456306b27@redhat.com \
    --to=david@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=broonie@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.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
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