From: shuah at kernel.org (shuah)
Subject: [PATCH 1/6] selftests: Extract single-test shell logic from lib.mk
Date: Tue, 23 Apr 2019 16:47:37 -0600 [thread overview]
Message-ID: <ca4605d5-9423-542e-5a10-2b681293baf9@kernel.org> (raw)
In-Reply-To: <CAGXu5jKLmX5wX1Qb0hYkn1oBhWNgJUYARoEn2JmioJUn5hiW4A@mail.gmail.com>
On 4/23/19 4:31 PM, Kees Cook wrote:
> On Tue, Apr 16, 2019 at 4:21 PM shuah <shuah at kernel.org> wrote:
>>
>> On 4/16/19 5:16 PM, Kees Cook wrote:
>>> On Tue, Apr 16, 2019 at 6:11 PM shuah <shuah at kernel.org> wrote:
>>>>
>>>> Hi Kees,
>>>>
>>>> Thanks for the patch.
>>>>
>>>> On 4/9/19 5:55 PM, Kees Cook wrote:
>>>>> In order to improve the reusability of the kselftest test running logic,
>>>>> this extracts the single-test logic from lib.mk into kselftest/runner.sh
>>>>> which lib.mk can call directly. No changes in output.
>>>>>
>>>>> As part of the change, this removes the unused "summary" Makefile variable
>>>>> (and tests). However, future merging with the "emit_tests" target needs
>>>>> to be able to redirect output, so a new "logfile" variable is introduced.
>>>>
>>>> Shouldn't the selftests/Makefile need update for "summary" removal??
>>>>
>>>
>>> I didn't see anything using "summary" except as a --summary argument
>>> to the run_kselftests.sh script. Maybe I missed it?
>>>
>>
>> It is in the selftests/Makefile install target.
>
> Right: it's used only by the run_kselftest.sh script:
>
> ALL_SCRIPT := $(INSTALL_PATH)/run_kselftest.sh
>
> install:
> ...
> echo "if [ \"\$$1\" = \"--summary\" ]; then" >> $(ALL_SCRIPT)
>
>
> So, I think this entire series can land. Is there other feedback I
> should incorporate? I'd like to see it get some -next testing...
>
> Thanks!
>
Kees,
Yes I was planning to get this into next and did some testing. I found
that the first patch breaks the summary option.
You can see it by running
make summary=1 TARGETS="breakpoints" kselftest
with and without your first patch. Ca you fix the regression and send
me revised patches. Sorry, I didn't get back to you sooner. A few bugs
I found kept me busy.
While I was testing your patch series, I found test build failures due
to circular dependency that needed fixing. In addition, the same
headers_install dependency, also broke O= and KBUILD_OUTPUT builds
and fixed those as well. Long story short, these fixes might conflict
with your work.
thanks,
-- Shuah
WARNING: multiple messages have this Message-ID (diff)
From: shuah@kernel.org (shuah)
Subject: [PATCH 1/6] selftests: Extract single-test shell logic from lib.mk
Date: Tue, 23 Apr 2019 16:47:37 -0600 [thread overview]
Message-ID: <ca4605d5-9423-542e-5a10-2b681293baf9@kernel.org> (raw)
Message-ID: <20190423224737.3bKV0Wv8x3v_mYKKkndDNIlUjKnGJaV5iRR68vZPCK0@z> (raw)
In-Reply-To: <CAGXu5jKLmX5wX1Qb0hYkn1oBhWNgJUYARoEn2JmioJUn5hiW4A@mail.gmail.com>
On 4/23/19 4:31 PM, Kees Cook wrote:
> On Tue, Apr 16, 2019@4:21 PM shuah <shuah@kernel.org> wrote:
>>
>> On 4/16/19 5:16 PM, Kees Cook wrote:
>>> On Tue, Apr 16, 2019@6:11 PM shuah <shuah@kernel.org> wrote:
>>>>
>>>> Hi Kees,
>>>>
>>>> Thanks for the patch.
>>>>
>>>> On 4/9/19 5:55 PM, Kees Cook wrote:
>>>>> In order to improve the reusability of the kselftest test running logic,
>>>>> this extracts the single-test logic from lib.mk into kselftest/runner.sh
>>>>> which lib.mk can call directly. No changes in output.
>>>>>
>>>>> As part of the change, this removes the unused "summary" Makefile variable
>>>>> (and tests). However, future merging with the "emit_tests" target needs
>>>>> to be able to redirect output, so a new "logfile" variable is introduced.
>>>>
>>>> Shouldn't the selftests/Makefile need update for "summary" removal??
>>>>
>>>
>>> I didn't see anything using "summary" except as a --summary argument
>>> to the run_kselftests.sh script. Maybe I missed it?
>>>
>>
>> It is in the selftests/Makefile install target.
>
> Right: it's used only by the run_kselftest.sh script:
>
> ALL_SCRIPT := $(INSTALL_PATH)/run_kselftest.sh
>
> install:
> ...
> echo "if [ \"\$$1\" = \"--summary\" ]; then" >> $(ALL_SCRIPT)
>
>
> So, I think this entire series can land. Is there other feedback I
> should incorporate? I'd like to see it get some -next testing...
>
> Thanks!
>
Kees,
Yes I was planning to get this into next and did some testing. I found
that the first patch breaks the summary option.
You can see it by running
make summary=1 TARGETS="breakpoints" kselftest
with and without your first patch. Ca you fix the regression and send
me revised patches. Sorry, I didn't get back to you sooner. A few bugs
I found kept me busy.
While I was testing your patch series, I found test build failures due
to circular dependency that needed fixing. In addition, the same
headers_install dependency, also broke O= and KBUILD_OUTPUT builds
and fixed those as well. Long story short, these fixes might conflict
with your work.
thanks,
-- Shuah
next prev parent reply other threads:[~2019-04-23 22:47 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-09 23:55 [PATCH 0/6] selftests: Move test output to diagnostic lines keescook
2019-04-09 23:55 ` Kees Cook
2019-04-09 23:55 ` [PATCH 1/6] selftests: Extract single-test shell logic from lib.mk keescook
2019-04-09 23:55 ` Kees Cook
2019-04-16 23:11 ` shuah
2019-04-16 23:11 ` shuah
2019-04-16 23:16 ` keescook
2019-04-16 23:16 ` Kees Cook
2019-04-16 23:21 ` shuah
2019-04-16 23:21 ` shuah
2019-04-23 22:31 ` keescook
2019-04-23 22:31 ` Kees Cook
2019-04-23 22:47 ` shuah [this message]
2019-04-23 22:47 ` shuah
2019-04-24 2:43 ` keescook
2019-04-24 2:43 ` Kees Cook
2019-04-24 2:46 ` shuah
2019-04-24 2:46 ` shuah
2019-04-09 23:55 ` [PATCH 2/6] selftests: Use runner.sh for emit targets keescook
2019-04-09 23:55 ` Kees Cook
2019-04-09 23:55 ` [PATCH 3/6] selftests: Extract logic for multiple test runs keescook
2019-04-09 23:55 ` Kees Cook
2019-04-09 23:55 ` [PATCH 4/6] selftests/runner: Add plan line and fix result line syntax keescook
2019-04-09 23:55 ` Kees Cook
2019-04-09 23:55 ` [PATCH 5/6] selftests/runner: Distinguish between missing and non-executable keescook
2019-04-09 23:55 ` Kees Cook
2019-04-09 23:55 ` [PATCH 6/6] selftests: Move test output to diagnostic lines keescook
2019-04-09 23:55 ` Kees Cook
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=ca4605d5-9423-542e-5a10-2b681293baf9@kernel.org \
--to=unknown@example.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.