Re: [PATCH net 0/2] selftests: net: add missing tests to Makefile
From: Jakub Kicinski <kuba@kernel.org>
Date: 2022-05-17 19:45:27
Also in:
linux-kselftest
On Tue, 17 May 2022 14:23:22 +0800 Hangbin Liu wrote:
On Fri, Apr 29, 2022 at 05:56:04PM -0700, Jakub Kicinski wrote:quoted
On Thu, 28 Apr 2022 12:45:09 +0800 Hangbin Liu wrote:quoted
I think there need a way to notify the developer when they created a new file in selftests folder. Maybe a bot like bluez.test.bot or kernel test robot could help do that?Our netdev patch checks are here: https://github.com/kuba-moo/nipa/tree/master/tests/patch in case you're willing to code it up and post a PR.Hi Jakub, I checked the tools and write a draft patch. But I have a question before post the PR.
First off - thanks a log for doing this!
AFAIK, This bot is only used for checking patches and adding status in patchwork. But it doesn't support sending a reply to developer, right? For the selftest reminder, I think it would be good to let developer know via email if the file is missing in Makefile. What do you think?
Yes, we don't have the auto-reply. There's too much noise in some of the tests, but mostly it's because we don't want to encourage people posting patches just to build them. If it's a machine replying rather than a human some may think that it's okay. We already have jaw-droppingly expensive VM instance to keep up with the build volume. And the list is very busy. So we can't afford "post to run the CI" development model.
quoted hunk ↗ jump to hunk
Here is the draft patch:diff --git a/tests/patch/check_selftest/check_selftest.sh b/tests/patch/check_selftest/check_selftest.sh new file mode 100755 index 0000000..ad7c608 --- /dev/null +++ b/tests/patch/check_selftest/check_selftest.sh@@ -0,0 +1,28 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 + +rt=0 +if ! git show --name-status --oneline | \ + grep -P '^A\ttools/testing/selftests/net/' | \ + grep '\.sh$'; then + echo "No new net selftests script" >&$DESC_FD + exit 0 +fi + +files=$(git show --name-status --oneline | grep -P '^A\ttools/testing/selftests/net/' | grep '\.sh$' | sed 's@A\ttools/testing/selftests/net/@@') +for file in $files; do + if echo $file | grep forwarding; then + file=$(echo $file | sed 's/forwarding\///') + if ! grep -P "[\t| ]$file" tools/testing/selftests/net/forwarding/Makefile;then + echo "new test $file not in selftests/net/forwarding/Makefile" >&$DESC_FD + rc=1 + fi + else + if ! grep -P "[\t| ]$file" tools/testing/selftests/net/Makefile;then + echo "new test $file not in selftests/net/Makefile" >&$DESC_FD + rc=1 + fi
Does it matter which exact selftest makefile the changes are? Maybe as a first stab we should just check if there are changes to anything in tools/testing/selftests/.*/Makefile? We can see if there are false-negatives.
quoted hunk ↗ jump to hunk
+ fi +done + +exit $rcdiff --git a/tests/patch/check_selftest/info.json b/tests/patch/check_selftest/info.json new file mode 100644 index 0000000..615779f --- /dev/null +++ b/tests/patch/check_selftest/info.json@@ -0,0 +1,3 @@ +{ + "run": ["check_selftest.sh"] +}