Re: [PATCH net-next RFC] selftests/net: integrate packetdrill with ksft

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 28 Aug 2024 10:03:55 -0400 Willem de Bruijn wrote:
> > > A single script is much
> > > simpler, optionally with nested KTAP (not supported yet by ksft). But,
> > > I'm afraid that running time without intermediate output will be very
> > > long when we integrate all packetdrill scripts.  
> > 
> > If I read correctly, this runs the scripts in the given directory 
> > sequentially (as opposed to the default pktdrill run_all.py behavior 
> > that uses many concurrent threads).
> > 
> > I guess/fear that running all the pktdrill tests in a single batch would 
> > take quite a long time, which in turn could be not so good for CI 
> > integration. Currently there are a couple of CI test-cases with runtime   
> >  > 1h, but that is bad ;)  
> 
> Very good point, thanks! This is the third packetdrill runner that I'm
> writing. I should know this by now.. Let me see whether I can use
> run_all.py rather than reinvent the wheel here.

Do we have to worry about this now? If you're planning to add a runner
specifically for packetdrill... IDK if we should. We already have a few
runners, not to mention that run_kselftest.sh itself can run all the
test cases in parallel in separate network namespaces!

What I was wondering is whether we can use shebang to direct the .pkt
files to be "executed" by the python script. Alternatively we can add
support to specifying "interpreter" for a given test in ksft infra
(kinda like we can pass cmd line arguments to a test). Completely
untested but it should give better idea what I mean than a description:

diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
index 74954f6a8f94..429c279e9c6e 100644
--- a/tools/testing/selftests/kselftest/runner.sh
+++ b/tools/testing/selftests/kselftest/runner.sh
@@ -56,6 +56,7 @@ run_one()
 	export kselftest_timeout="$kselftest_default_timeout"
 
 	# Safe default if tr not available
+	kselftest_interp_ref="KSELFTEST_INTERP"
 	kselftest_cmd_args_ref="KSELFTEST_ARGS"
 
 	# Optional arguments for this command, possibly defined as an
@@ -78,6 +79,14 @@ run_one()
 					$TR_CMD [:lower:] [:upper:])
 		kselftest_cmd_args_ref="KSELFTEST_${BASENAME_SANITIZED}_ARGS"
 	fi
+	# Optional interpreter to run the test case
+	if [ -n "$TR_CMD" ]; then
+		SUFFIX_SANITIZED=$(echo "${BASENAME_TEST#*.}" | \
+				       $TR_CMD -d "[:blank:][:cntrl:]" | \
+				       $TR_CMD -c "[:alnum:]_" "_" | \
+				       $TR_CMD [:lower:] [:upper:])
+		kselftest_interp_ref="KSELFTEST_${SUFFIX_SANITIZED}_INTERP"
+	fi
 
 	# Load per-test-directory kselftest "settings" file.
 	settings="$BASE_DIR/$DIR/settings"
@@ -110,8 +119,12 @@ run_one()
 		if [ -x /usr/bin/stdbuf ]; then
 			stdbuf="/usr/bin/stdbuf --output=L "
 		fi
+		eval kselftest_interp="\$${kselftest_interp_ref:-}"
+		# Add a space at the end if interpreter is set to work in $cmd
+		[ -n "$kselftest_interp" ] && \
+		    kselftest_interp="$kselftest_interp "
 		eval kselftest_cmd_args="\$${kselftest_cmd_args_ref:-}"
-		cmd="$stdbuf ./$BASENAME_TEST $kselftest_cmd_args"
+		cmd="$stdbuf ./$kselftest_interp$BASENAME_TEST $kselftest_cmd_args"
 		if [ ! -x "$TEST" ]; then
 			echo "# Warning: file $TEST is not executable"