Re: [PATCH 1/1] Improve missing mods error message and make shell script executable

[Jakub Kicinski <kuba@kernel.org>]

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
-- 
pw-bot: cr