Linux-kselftest Archive on lore.kernel.org
 help / color / Atom feed
From: Ilya Leoshkevich <iii@linux.ibm.com>
To: shuah <shuah@kernel.org>
Cc: linux-kselftest@vger.kernel.org,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>
Subject: Re: [PATCH RESEND v3 0/2] selftests: fix prepending $(OUTPUT) to $(TEST_PROGS)
Date: Fri, 8 Nov 2019 13:31:54 +0100
Message-ID: <EF7A3DF5-D73D-4AD2-A836-CA48B3C03401@linux.ibm.com> (raw)
In-Reply-To: <623856be-1bd3-54b9-6336-fd8daec2dfe2@kernel.org>

> Am 07.11.2019 um 23:52 schrieb shuah <shuah@kernel.org>:
> 
> On 10/24/19 6:13 AM, Ilya Leoshkevich wrote:
>> Hello,
>> Is there anything blocking this from getting merged?
> 
> Please see below:
> 
>> This patch series fixes the following problem:
>> linux# make kselftest TARGETS=bpf O=/mnt/linux-build
>> # selftests: bpf: test_libbpf.sh
>> # ./test_libbpf.sh: line 23: ./test_libbpf_open: No such file or directory
>> # test_libbpf: failed at file test_l4lb.o
>> # selftests: test_libbpf [FAILED]
>> Patch 1 appends / to $(OUTPUT) in order to make it more uniform with the
>> rest of the tree.
> 
> It isn't clear what this fixes. In addition, this patch appends an extra
> "/"
> 
> Excerpts from make kselftest-all with this patch applied:
> 
> linux_5.4/tools/testing/selftests/bpf//test_tcpnotify_user
> 
> I am not seeing any reason to take this patch.

The goal of patch 1 was not to fix a practical problem, but rather make
kselftest code follow the same convention as the rest of the build code
in tools/, namely: that $(OUTPUT) is assumed to always end with /.
However, if this commonality is not valuable, I don't think I should
pursue this. So I'd like to withdraw patch 1.

>> Patch 2 fixes the problem by prepending $(OUTPUT) to all members of
>> $(TEST_PROGS).
>> v1->v2:
>> - Append / to $(OUTPUT).
>> - Use $(addprefix) instead of $(foreach).
> 
> I can't reproduce this problem - I ran
> 
> make kselftest O=/tmp/linux-build

May I ask you, how did you check the test results? I'm asking because
some test failures, including this one, may not affect make exit code.
So you need to grep for e.g. test_libbpf.sh in make output to see the
problem. For example, in my setup a lot of eBPF tests fail without this
patch, and yet:

[root@bpf linux]# make kselftest TARGETS=bpf O=/mnt/linux-build
<snip>
ok 28 selftests: bpf: test_kmod.sh
# selftests: bpf: test_libbpf.sh
# ./test_libbpf.sh: line 23: ./test_libbpf_open: No such file or directory
# test_libbpf: failed at file test_l4lb.o
# selftests: test_libbpf [FAILED]
not ok 29 selftests: bpf: test_libbpf.sh # exit=127
<snip>
make[1]: Leaving directory '/mnt/linux-build'

[root@bpf linux]# echo $?
0

> This might be fixing the problem in bpf case. However, I don't think
> this is the right fix.

It is weird in general that the existing code prepends $(OUTPUT) only to
the first test prog. The first test prog is by no means special. The
commit message of patch 2 explains in more detail how this discrepancy
causes a practical problem:

> The current logic prepends $(OUTPUT) only to the first member of
> $(TEST_PROGS). After that, run_one() does
>
>   cd `dirname $TEST`
>
> For all tests except the first one, `dirname $TEST` is ., which means
> they cannot access the files generated in $(OUTPUT).

Is there a problem with this reasoning?

> I would like to see this problem reproduced on another test first.

Best regards,
Ilya


      reply index

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-24 12:13 Ilya Leoshkevich
2019-10-24 12:13 ` [PATCH RESEND v3 1/2] selftests: append / to $(OUTPUT) Ilya Leoshkevich
2019-10-24 12:13 ` [PATCH RESEND v3 2/2] selftests: fix prepending $(OUTPUT) to $(TEST_PROGS) Ilya Leoshkevich
2019-11-07 22:52 ` [PATCH RESEND v3 0/2] " shuah
2019-11-08 12:31   ` Ilya Leoshkevich [this message]

Reply instructions:

You may reply publically 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=EF7A3DF5-D73D-4AD2-A836-CA48B3C03401@linux.ibm.com \
    --to=iii@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=heiko.carstens@de.ibm.com \
    --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