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: > > On Thu, 28 Apr 2022 12:45:09 +0800 Hangbin Liu wrote: > > > 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. > 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. > + fi > +done > + > +exit $rc > diff --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"] > +}