linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] selftests: settings: tests can be in subsubdirs
@ 2019-10-22 17:12 Matthieu Baerts
  2019-11-21 16:32 ` Matthieu Baerts
  0 siblings, 1 reply; 6+ messages in thread
From: Matthieu Baerts @ 2019-10-22 17:12 UTC (permalink / raw)
  To: Shuah Khan; +Cc: Matthieu Baerts, Kees Cook, linux-kselftest, linux-kernel

Commit 852c8cbf34d3 (selftests/kselftest/runner.sh: Add 45 second
timeout per test) adds support for a new per-test-directory "settings"
file. But this only works for tests not in a sub-subdirectories, e.g.

 - tools/testing/selftests/rtc (rtc) is OK,
 - tools/testing/selftests/net/mptcp (net/mptcp) is not.

We have to increase the timeout for net/mptcp tests which are not
upstreamed yet but this fix is valid for other tests if they need to add
a "settings" file, see the full list with:

  tools/testing/selftests/*/*/**/Makefile

Note that this patch changes the text header message printed at the end
of the execution but this text is modified only for the tests that are
in sub-subdirectories, e.g.

  ok 1 selftests: net/mptcp: mptcp_connect.sh

Before we had:

  ok 1 selftests: mptcp: mptcp_connect.sh

But showing the full target name is probably better, just in case a
subsubdir has the same name as another one in another subdirectory.

Fixes: 852c8cbf34d3 (selftests/kselftest/runner.sh: Add 45 second timeout per test)
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 tools/testing/selftests/kselftest/runner.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
index 84de7bc74f2c..0d7a89901ef7 100644
--- a/tools/testing/selftests/kselftest/runner.sh
+++ b/tools/testing/selftests/kselftest/runner.sh
@@ -90,7 +90,7 @@ run_one()
 run_many()
 {
 	echo "TAP version 13"
-	DIR=$(basename "$PWD")
+	DIR="${PWD#${BASE_DIR}/}"
 	test_num=0
 	total=$(echo "$@" | wc -w)
 	echo "1..$total"
-- 
2.20.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] selftests: settings: tests can be in subsubdirs
  2019-10-22 17:12 [PATCH] selftests: settings: tests can be in subsubdirs Matthieu Baerts
@ 2019-11-21 16:32 ` Matthieu Baerts
  2019-11-21 18:52   ` Kees Cook
  0 siblings, 1 reply; 6+ messages in thread
From: Matthieu Baerts @ 2019-11-21 16:32 UTC (permalink / raw)
  To: Shuah Khan; +Cc: Kees Cook, linux-kselftest, linux-kernel

Hi Shuah,

First, thank you for maintaining the Kernel Selftest framework!

On 22/10/2019 19:12, Matthieu Baerts wrote:
> Commit 852c8cbf34d3 (selftests/kselftest/runner.sh: Add 45 second
> timeout per test) adds support for a new per-test-directory "settings"
> file. But this only works for tests not in a sub-subdirectories, e.g.
> 
>   - tools/testing/selftests/rtc (rtc) is OK,
>   - tools/testing/selftests/net/mptcp (net/mptcp) is not.
> 
> We have to increase the timeout for net/mptcp tests which are not
> upstreamed yet but this fix is valid for other tests if they need to add
> a "settings" file, see the full list with:
> 
>    tools/testing/selftests/*/*/**/Makefile
> 
> Note that this patch changes the text header message printed at the end
> of the execution but this text is modified only for the tests that are
> in sub-subdirectories, e.g.
> 
>    ok 1 selftests: net/mptcp: mptcp_connect.sh
> 
> Before we had:
> 
>    ok 1 selftests: mptcp: mptcp_connect.sh
> 
> But showing the full target name is probably better, just in case a
> subsubdir has the same name as another one in another subdirectory.
> 
> Fixes: 852c8cbf34d3 (selftests/kselftest/runner.sh: Add 45 second timeout per test)
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Sorry to bother you again with this but by chance, did you have a look 
at the patch below? :)

It doesn't only fix an issue with MPTCP, not in the kernel yet. But it 
also fixes the issue of taking the right "settings" file (if available) 
for any other tests in a sub-directory, e.g.:

   drivers/dma-buf
   filesystems/binderfs
   net/forwarding
   networking/timestamping

But I guess all tests in powerpc/* dirs and others.

Cheers,
Matt

> ---
>   tools/testing/selftests/kselftest/runner.sh | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
> index 84de7bc74f2c..0d7a89901ef7 100644
> --- a/tools/testing/selftests/kselftest/runner.sh
> +++ b/tools/testing/selftests/kselftest/runner.sh
> @@ -90,7 +90,7 @@ run_one()
>   run_many()
>   {
>   	echo "TAP version 13"
> -	DIR=$(basename "$PWD")
> +	DIR="${PWD#${BASE_DIR}/}"
>   	test_num=0
>   	total=$(echo "$@" | wc -w)
>   	echo "1..$total"
> 

-- 
Matthieu Baerts | R&D Engineer
matthieu.baerts@tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] selftests: settings: tests can be in subsubdirs
  2019-11-21 16:32 ` Matthieu Baerts
@ 2019-11-21 18:52   ` Kees Cook
  2020-01-27 16:05     ` Matthieu Baerts
  0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2019-11-21 18:52 UTC (permalink / raw)
  To: Matthieu Baerts; +Cc: Shuah Khan, linux-kselftest, linux-kernel

On Thu, Nov 21, 2019 at 05:32:42PM +0100, Matthieu Baerts wrote:
> Hi Shuah,
> 
> First, thank you for maintaining the Kernel Selftest framework!
> 
> On 22/10/2019 19:12, Matthieu Baerts wrote:
> > Commit 852c8cbf34d3 (selftests/kselftest/runner.sh: Add 45 second
> > timeout per test) adds support for a new per-test-directory "settings"
> > file. But this only works for tests not in a sub-subdirectories, e.g.
> > 
> >   - tools/testing/selftests/rtc (rtc) is OK,
> >   - tools/testing/selftests/net/mptcp (net/mptcp) is not.
> > 
> > We have to increase the timeout for net/mptcp tests which are not
> > upstreamed yet but this fix is valid for other tests if they need to add
> > a "settings" file, see the full list with:
> > 
> >    tools/testing/selftests/*/*/**/Makefile
> > 
> > Note that this patch changes the text header message printed at the end
> > of the execution but this text is modified only for the tests that are
> > in sub-subdirectories, e.g.
> > 
> >    ok 1 selftests: net/mptcp: mptcp_connect.sh
> > 
> > Before we had:
> > 
> >    ok 1 selftests: mptcp: mptcp_connect.sh
> > 
> > But showing the full target name is probably better, just in case a
> > subsubdir has the same name as another one in another subdirectory.
> > 
> > Fixes: 852c8cbf34d3 (selftests/kselftest/runner.sh: Add 45 second timeout per test)
> > Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> Sorry to bother you again with this but by chance, did you have a look at
> the patch below? :)
> 
> It doesn't only fix an issue with MPTCP, not in the kernel yet. But it also
> fixes the issue of taking the right "settings" file (if available) for any
> other tests in a sub-directory, e.g.:
> 
>   drivers/dma-buf
>   filesystems/binderfs
>   net/forwarding
>   networking/timestamping
> 
> But I guess all tests in powerpc/* dirs and others.

Thanks for the ping! I missed this patch when you originally sent it.
Yes, this make sense to me:

Reviewed-by: Kees Cook <keescook@chromium.org>

As an improvement on this, I wonder if we need to walk all directories
between $BASEDIR and $DIR? Actually, let me write this and send it...

-Kees

> 
> Cheers,
> Matt
> 
> > ---
> >   tools/testing/selftests/kselftest/runner.sh | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
> > index 84de7bc74f2c..0d7a89901ef7 100644
> > --- a/tools/testing/selftests/kselftest/runner.sh
> > +++ b/tools/testing/selftests/kselftest/runner.sh
> > @@ -90,7 +90,7 @@ run_one()
> >   run_many()
> >   {
> >   	echo "TAP version 13"
> > -	DIR=$(basename "$PWD")
> > +	DIR="${PWD#${BASE_DIR}/}"
> >   	test_num=0
> >   	total=$(echo "$@" | wc -w)
> >   	echo "1..$total"
> > 
> 
> -- 
> Matthieu Baerts | R&D Engineer
> matthieu.baerts@tessares.net
> Tessares SA | Hybrid Access Solutions
> www.tessares.net
> 1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] selftests: settings: tests can be in subsubdirs
  2019-11-21 18:52   ` Kees Cook
@ 2020-01-27 16:05     ` Matthieu Baerts
  2020-01-27 17:16       ` shuah
  0 siblings, 1 reply; 6+ messages in thread
From: Matthieu Baerts @ 2020-01-27 16:05 UTC (permalink / raw)
  To: Kees Cook, Shuah Khan; +Cc: linux-kselftest, linux-kernel, mptcp

Hi Shuah, Kees,

On 21/11/2019 19:52, Kees Cook wrote:
> On Thu, Nov 21, 2019 at 05:32:42PM +0100, Matthieu Baerts wrote:
>> Hi Shuah,
>>
>> First, thank you for maintaining the Kernel Selftest framework!
>>
>> On 22/10/2019 19:12, Matthieu Baerts wrote:
>>> Commit 852c8cbf34d3 (selftests/kselftest/runner.sh: Add 45 second
>>> timeout per test) adds support for a new per-test-directory "settings"
>>> file. But this only works for tests not in a sub-subdirectories, e.g.
>>>
>>>    - tools/testing/selftests/rtc (rtc) is OK,
>>>    - tools/testing/selftests/net/mptcp (net/mptcp) is not.
>>>
>>> We have to increase the timeout for net/mptcp tests which are not
>>> upstreamed yet but this fix is valid for other tests if they need to add
>>> a "settings" file, see the full list with:
>>>
>>>     tools/testing/selftests/*/*/**/Makefile
>>>
>>> Note that this patch changes the text header message printed at the end
>>> of the execution but this text is modified only for the tests that are
>>> in sub-subdirectories, e.g.
>>>
>>>     ok 1 selftests: net/mptcp: mptcp_connect.sh
>>>
>>> Before we had:
>>>
>>>     ok 1 selftests: mptcp: mptcp_connect.sh
>>>
>>> But showing the full target name is probably better, just in case a
>>> subsubdir has the same name as another one in another subdirectory.
>>>
>>> Fixes: 852c8cbf34d3 (selftests/kselftest/runner.sh: Add 45 second timeout per test)
>>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>> Sorry to bother you again with this but by chance, did you have a look at
>> the patch below? :)
>>
>> It doesn't only fix an issue with MPTCP, not in the kernel yet. But it also
>> fixes the issue of taking the right "settings" file (if available) for any
>> other tests in a sub-directory, e.g.:
>>
>>    drivers/dma-buf
>>    filesystems/binderfs
>>    net/forwarding
>>    networking/timestamping
>>
>> But I guess all tests in powerpc/* dirs and others.
> 
> Thanks for the ping! I missed this patch when you originally sent it.
> Yes, this make sense to me:
> 
> Reviewed-by: Kees Cook <keescook@chromium.org>

Kees, Thank you for this review!

Shuah, I am sorry to send you this new request. It is just to inform you 
that the first selftests for MPTCP are now in "net-next" repo, ready for 
the future Linux 5.6.
We would then be very happy to see this patch here below for the 
kselftest framework accepted to avoid timeouts. Locally we apply this 
patch before running the selftests but we cannot ask everybody running 
MPTCP' selftests to do the same :)

> As an improvement on this, I wonder if we need to walk all directories
> between $BASEDIR and $DIR? Actually, let me write this and send it...

Thank you, Kees, for this improvement!

Cheers,
Matt

> 
> -Kees
> 
>>
>> Cheers,
>> Matt
>>
>>> ---
>>>    tools/testing/selftests/kselftest/runner.sh | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
>>> index 84de7bc74f2c..0d7a89901ef7 100644
>>> --- a/tools/testing/selftests/kselftest/runner.sh
>>> +++ b/tools/testing/selftests/kselftest/runner.sh
>>> @@ -90,7 +90,7 @@ run_one()
>>>    run_many()
>>>    {
>>>    	echo "TAP version 13"
>>> -	DIR=$(basename "$PWD")
>>> +	DIR="${PWD#${BASE_DIR}/}"
>>>    	test_num=0
>>>    	total=$(echo "$@" | wc -w)
>>>    	echo "1..$total"
>>> -- 
Matthieu Baerts | R&D Engineer
matthieu.baerts@tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] selftests: settings: tests can be in subsubdirs
  2020-01-27 16:05     ` Matthieu Baerts
@ 2020-01-27 17:16       ` shuah
  2020-01-27 20:34         ` Matthieu Baerts
  0 siblings, 1 reply; 6+ messages in thread
From: shuah @ 2020-01-27 17:16 UTC (permalink / raw)
  To: Matthieu Baerts, Kees Cook; +Cc: linux-kselftest, linux-kernel, mptcp, shuah

On 1/27/20 9:05 AM, Matthieu Baerts wrote:
> Hi Shuah, Kees,
> 
> On 21/11/2019 19:52, Kees Cook wrote:
>> On Thu, Nov 21, 2019 at 05:32:42PM +0100, Matthieu Baerts wrote:
>>> Hi Shuah,
>>>
>>> First, thank you for maintaining the Kernel Selftest framework!
>>>
>>> On 22/10/2019 19:12, Matthieu Baerts wrote:
>>>> Commit 852c8cbf34d3 (selftests/kselftest/runner.sh: Add 45 second
>>>> timeout per test) adds support for a new per-test-directory "settings"
>>>> file. But this only works for tests not in a sub-subdirectories, e.g.
>>>>
>>>>    - tools/testing/selftests/rtc (rtc) is OK,
>>>>    - tools/testing/selftests/net/mptcp (net/mptcp) is not.
>>>>
>>>> We have to increase the timeout for net/mptcp tests which are not
>>>> upstreamed yet but this fix is valid for other tests if they need to 
>>>> add
>>>> a "settings" file, see the full list with:
>>>>
>>>>     tools/testing/selftests/*/*/**/Makefile
>>>>
>>>> Note that this patch changes the text header message printed at the end
>>>> of the execution but this text is modified only for the tests that are
>>>> in sub-subdirectories, e.g.
>>>>
>>>>     ok 1 selftests: net/mptcp: mptcp_connect.sh
>>>>
>>>> Before we had:
>>>>
>>>>     ok 1 selftests: mptcp: mptcp_connect.sh
>>>>
>>>> But showing the full target name is probably better, just in case a
>>>> subsubdir has the same name as another one in another subdirectory.
>>>>
>>>> Fixes: 852c8cbf34d3 (selftests/kselftest/runner.sh: Add 45 second 
>>>> timeout per test)
>>>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>>> Sorry to bother you again with this but by chance, did you have a 
>>> look at
>>> the patch below? :)
>>>
>>> It doesn't only fix an issue with MPTCP, not in the kernel yet. But 
>>> it also
>>> fixes the issue of taking the right "settings" file (if available) 
>>> for any
>>> other tests in a sub-directory, e.g.:
>>>
>>>    drivers/dma-buf
>>>    filesystems/binderfs
>>>    net/forwarding
>>>    networking/timestamping
>>>
>>> But I guess all tests in powerpc/* dirs and others.
>>
>> Thanks for the ping! I missed this patch when you originally sent it.
>> Yes, this make sense to me:
>>
>> Reviewed-by: Kees Cook <keescook@chromium.org>
> 
> Kees, Thank you for this review!
> 
> Shuah, I am sorry to send you this new request. It is just to inform you 
> that the first selftests for MPTCP are now in "net-next" repo, ready for 
> the future Linux 5.6.
> We would then be very happy to see this patch here below for the 
> kselftest framework accepted to avoid timeouts. Locally we apply this 
> patch before running the selftests but we cannot ask everybody running 
> MPTCP' selftests to do the same :)
> 
>

I am sorry for the delay. My bad. Looks like I just missed it. I will
make sure it gets into 5.6-rc1

Thanks for your patience and ping.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] selftests: settings: tests can be in subsubdirs
  2020-01-27 17:16       ` shuah
@ 2020-01-27 20:34         ` Matthieu Baerts
  0 siblings, 0 replies; 6+ messages in thread
From: Matthieu Baerts @ 2020-01-27 20:34 UTC (permalink / raw)
  To: shuah; +Cc: Kees Cook, linux-kselftest, linux-kernel, mptcp

Hi Shuah,

On 27/01/2020 18:16, shuah wrote:
> On 1/27/20 9:05 AM, Matthieu Baerts wrote:

[...]

>> Kees, Thank you for this review!
>>
>> Shuah, I am sorry to send you this new request. It is just to inform 
>> you that the first selftests for MPTCP are now in "net-next" repo, 
>> ready for the future Linux 5.6.
>> We would then be very happy to see this patch here below for the 
>> kselftest framework accepted to avoid timeouts. Locally we apply this 
>> patch before running the selftests but we cannot ask everybody running 
>> MPTCP' selftests to do the same :)
>>
>>
> 
> I am sorry for the delay. My bad. Looks like I just missed it. I will
> make sure it gets into 5.6-rc1

That's alright, thank you for having applied this patch in your "next" 
branch!

Cheers,
Matt
-- 
Matthieu Baerts | R&D Engineer
matthieu.baerts@tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-01-27 20:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-22 17:12 [PATCH] selftests: settings: tests can be in subsubdirs Matthieu Baerts
2019-11-21 16:32 ` Matthieu Baerts
2019-11-21 18:52   ` Kees Cook
2020-01-27 16:05     ` Matthieu Baerts
2020-01-27 17:16       ` shuah
2020-01-27 20:34         ` Matthieu Baerts

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).