Message-ID: <66d4d97a4cac_3df182941a@willemb.c.googlers.com.notmuch>
Date: Sun, 01 Sep 2024 17:15:38 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>, 
 Jakub Kicinski <kuba@...nel.org>, 
 Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: Paolo Abeni <pabeni@...hat.com>, 
 netdev@...r.kernel.org, 
 davem@...emloft.net, 
 edumazet@...gle.com, 
 ncardwell@...gle.com, 
 shuah@...nel.org, 
 linux-kselftest@...r.kernel.org, 
 fw@...len.de, 
 Willem de Bruijn <willemb@...gle.com>, 
 "Matthieu Baerts (NGI0)" <matttbe@...nel.org>, 
 martineau@...nel.org
Subject: Re: [PATCH net-next RFC] selftests/net: integrate packetdrill with
 ksft

Willem de Bruijn wrote:
> Jakub Kicinski wrote:
> > On Fri, 30 Aug 2024 14:47:43 -0400 Willem de Bruijn wrote:
> > > > We have directories in net/lib, and it's a target, and it works, no?  
> > > 
> > > net/lib is not a TARGET in tools/testing/selftests/Makefile. Its
> > > Makefile only generates dependencies for other targets: TEST_FILES,
> > > TEST_GEN_FILES and TEST_INCLUDES.
> > 
> > Oh right, TEST_FILES vs TEST_INCLUDES :(
> > 
> > Looks like only x86 does some weird stuff and prepends $(OUTPUT) to all
> > test names. Otherwise the only TEST_NAME with a / in it is
> > 
> > x86_64/nx_huge_pages_test.sh
> 
> Oh interesting precedent. Let me take a look.
> 
> > But then again maybe we should give up on the idea of using directories?
> > Use some separator like --, I mean:
> > 
> > mv packetdrill/tcp/inq/client.pkt packetdrill/tcp--inq--client.pkt
> > 
> > Assuming we're moving forward with the interpreter idea we don't need
> > directories for multi-threading, just for organization. Which perhaps
> > isn't worth the time investment? Given that we'd mostly interact with
> > these tests via UI which will flatten it all back?
> 
> That's definitely simpler :)
> 
> I'd like to keep diffs between packetdrill scripts on github (and
> Google internal, we have more) and selftests to a minimum. This is
> invertible, as is rewriting source statements inside the pkt files.
> But that might be more work and more maintenance in the end.

Thanks again for the pointer and suggestion.

Changing kselftests to preserve directories turns out to be trivial.
Patch inline below.

But, existing TARGETS of course then start failing. Because they
depend on existing rsync without -R. In (at least) two ways:

amd-pstate fails because its TEST_FILES has includes from other
directories and it expects those files to land in the directory
with tests.

x86 prefixes all its output with $(OUTPUT) to form absolute paths,
which also creates absolute paths in kselftest-list.txt.

These two are examples, not necessarily the one instances of those
patterns. So switching to preserving directories for existing targets
like TEST_FILES seems intractable.

Plan B is to add a new TEST_PROGS_RECURSE, analogous to how
TEST_INCLUDES extended TEST_FILES with optional path preservation.
That is not much more complex.

---

+++ b/tools/testing/selftests/kselftest/runner.sh
@@ -101,7 +112,7 @@ run_one()
                echo "# timeout set to $kselftest_timeout" >> "$logfile"
        fi
 
-       TEST_HDR_MSG="selftests: $DIR: $BASENAME_TEST"
+       TEST_HDR_MSG="selftests: $DIR: $TEST"

+++ b/tools/testing/selftests/lib.mk
@@ -150,7 +150,7 @@ clean_mods_dir:

 define INSTALL_SINGLE_RULE
        $(if $(INSTALL_LIST),@mkdir -p $(INSTALL_PATH))
-       $(if $(INSTALL_LIST),rsync -a --copy-unsafe-links $(INSTALL_LIST) $(INSTALL_PATH)/)
+       $(if $(INSTALL_LIST),rsync -aR --copy-unsafe-links $(INSTALL_LIST) $(INSTALL_PATH)/)
 endef

 define INSTALL_MODS_RULE
@@ -180,8 +180,7 @@ endif
 
 emit_tests:
        for TEST in $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS); do \
-               BASENAME_TEST=`basename $$TEST`;        \
-               echo "$(COLLECTION):$$BASENAME_TEST";   \
+               echo "$(COLLECTION):$$TEST";    \
        done

Powered by blists - more mailing lists