| Message ID | 20240820202116.6124-1-david.hunter.linux@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [1/1] Improve missing mods error message and make shell script executable | expand |
On Tue, 20 Aug 2024 16:21:16 -0400 David Hunter wrote: > Subject: [PATCH 1/1] Improve missing mods error message and make shell script executable Would be good to add a prefix to the subject: selftests: net: > Make the test executable. Currently, tests in this shell script are not > executable, so the scipt file is skipped entirely. Could you clarify how it gets skipped? We use make [...] run_tests in our CI and it does seem to run. > Also, If you say "also" there's a good chance the commit should be split into two.. > the error message descirbing the required modules is inaccurate. > Currently, only "SKIP: Need act_mirred module" is shown. As a result, > users might only that module; however, three modules are actually > required and if any of them are missing, the build will fail with the > same message. > > Fix the error message to show any/all modules needed for the script file > upon failure. > > Signed-off-by: David Hunter <david.hunter.linux@gmail.com> > --- > .../testing/selftests/net/test_ingress_egress_chaining.sh | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > mode change 100644 => 100755 tools/testing/selftests/net/test_ingress_egress_chaining.sh > > diff --git a/tools/testing/selftests/net/test_ingress_egress_chaining.sh b/tools/testing/selftests/net/test_ingress_egress_chaining.sh > old mode 100644 > new mode 100755 > index 08adff6bb3b6..b1a3d68e0ec2 > --- a/tools/testing/selftests/net/test_ingress_egress_chaining.sh > +++ b/tools/testing/selftests/net/test_ingress_egress_chaining.sh > @@ -13,8 +13,14 @@ if [ "$(id -u)" -ne 0 ];then > fi > > needed_mods="act_mirred cls_flower sch_ingress" > +mods_missing="" > + > +for mod in $needed_mods; do > + modinfo $mod &>/dev/null || mods_missing="$mods_missing$mod " > +done > + > for mod in $needed_mods; do Do you have to loop again? Maybe just check if mods_missing is empty? > - modinfo $mod &>/dev/null || { echo "SKIP: Need act_mirred module"; exit $ksft_skip; } > + modinfo $mod &>/dev/null || { echo "SKIP: modules needed: $mods_missing"; exit $ksft_skip; } > done
> If you say "also" there's a good chance the commit should be split into two.. I am splitting original patch into 2 separate patches. I forgot to do the reply all command on kernel lore. Here is the link to version 2 for the improving the missing modules error message: https://lore.kernel.org/all/20240823054833.144612-1-david.hunter.linux@gmail.com/ Subject: [PATCH 1/1 V2] Selftests: net: Improve missing modules error message > Could you clarify how it gets skipped? We use make [...] run_tests in our CI and it does seem to run. Here is my set up: $ uname -a - Linux dshunter-HP-Laptop-15-dy5xxx 6.11.0-rc2+ #2 SMP PREEMPT_DYNAMIC Tue Aug 20 14:31:34 EDT 2024 x86_64 x86_64 x86_64 GNU/Linux Steps I took to produce the error: - use git clone to get the mainline source - run make -C tools/testing/selftests - make summary=1 -C tools/testing/selftests TARGETS=net run_tests Output: # selftests: net: test_ingress_egress_chaining.sh # Warning: file test_ingress_egress_chaining.sh is not executable After running chmod +x on the shell script. The tests were able to be run. Thanks, David
diff --git a/tools/testing/selftests/net/test_ingress_egress_chaining.sh b/tools/testing/selftests/net/test_ingress_egress_chaining.sh old mode 100644 new mode 100755 index 08adff6bb3b6..b1a3d68e0ec2 --- a/tools/testing/selftests/net/test_ingress_egress_chaining.sh +++ b/tools/testing/selftests/net/test_ingress_egress_chaining.sh @@ -13,8 +13,14 @@ if [ "$(id -u)" -ne 0 ];then fi needed_mods="act_mirred cls_flower sch_ingress" +mods_missing="" + +for mod in $needed_mods; do + modinfo $mod &>/dev/null || mods_missing="$mods_missing$mod " +done + for mod in $needed_mods; do - modinfo $mod &>/dev/null || { echo "SKIP: Need act_mirred module"; exit $ksft_skip; } + modinfo $mod &>/dev/null || { echo "SKIP: modules needed: $mods_missing"; exit $ksft_skip; } done ns="ns$((RANDOM%899+100))"
Make the test executable. Currently, tests in this shell script are not executable, so the scipt file is skipped entirely. Also, the error message descirbing the required modules is inaccurate. Currently, only "SKIP: Need act_mirred module" is shown. As a result, users might only that module; however, three modules are actually required and if any of them are missing, the build will fail with the same message. Fix the error message to show any/all modules needed for the script file upon failure. Signed-off-by: David Hunter <david.hunter.linux@gmail.com> --- .../testing/selftests/net/test_ingress_egress_chaining.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) mode change 100644 => 100755 tools/testing/selftests/net/test_ingress_egress_chaining.sh