diff mbox series

selftests:net:forwarding: Included install command

Message ID 20220810093508.33790-1-pthange19@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series selftests:net:forwarding: Included install command | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 11 of 11 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success net selftest script(s) already in Makefile
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 14 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Piyush Thange Aug. 10, 2022, 9:35 a.m. UTC
If the execution is skipped due to "jq not installed" message then
the installation methods on different OS's have been provided with
this message.

Signed-off-by: Piyush Thange <pthange19@gmail.com>
---
 tools/testing/selftests/net/forwarding/lib.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

--
2.37.1

Comments

Siddh Raman Pant Aug. 10, 2022, 9:53 a.m. UTC | #1
On Wed, 10 Aug 2022 15:05:08 +0530  Piyush Thange <pthange19@gmail.com>  wrote:
> If the execution is skipped due to "jq not installed" message then
> the installation methods on different OS's have been provided with
> this message.
> 
> Signed-off-by: Piyush Thange <pthange19@gmail.com>
> ---
>  tools/testing/selftests/net/forwarding/lib.sh | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> index 37ae49d47853..c4121856fe06 100755
> --- a/tools/testing/selftests/net/forwarding/lib.sh
> +++ b/tools/testing/selftests/net/forwarding/lib.sh
> @@ -152,6 +152,14 @@ require_command()
> 
>  	if [[ ! -x "$(command -v "$cmd")" ]]; then
>  		echo "SKIP: $cmd not installed"
> +		if [[ $cmd == "jq" ]]; then
> +			echo " Install on Debian based systems"
> +			echo "	sudo apt -y install jq"
> +			echo " Install on RHEL based systems"
> +			echo "	sudo yum -y install jq"
> +			echo " Install on Fedora based systems"
> +			echo "	sudo dnf -y install jq"
> +		fi
>  		exit $ksft_skip
>  	fi
>  }
> --
> 2.37.1

This is very specific to `jq` command. What's special with `jq` and not
others? If methods have to be shown, they should be shown for all the
programs which are not installed.

Further, this limits the information to specific package managers and
systems in the userspace. Tomorrow a new system may come, which will
cause this list to grow, not to mention other existing package managers.
The kernel also doesn't have a role in it, so we should try to be generic
as much as possible.

Thanks,
Siddh
Hangbin Liu Aug. 11, 2022, 1:41 a.m. UTC | #2
On Wed, Aug 10, 2022 at 03:23:15PM +0530, Siddh Raman Pant wrote:
> On Wed, 10 Aug 2022 15:05:08 +0530  Piyush Thange <pthange19@gmail.com>  wrote:
> > If the execution is skipped due to "jq not installed" message then
> > the installation methods on different OS's have been provided with
> > this message.
> > 
> > Signed-off-by: Piyush Thange <pthange19@gmail.com>
> > ---
> >  tools/testing/selftests/net/forwarding/lib.sh | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> > index 37ae49d47853..c4121856fe06 100755
> > --- a/tools/testing/selftests/net/forwarding/lib.sh
> > +++ b/tools/testing/selftests/net/forwarding/lib.sh
> > @@ -152,6 +152,14 @@ require_command()
> > 
> >  	if [[ ! -x "$(command -v "$cmd")" ]]; then
> >  		echo "SKIP: $cmd not installed"
> > +		if [[ $cmd == "jq" ]]; then
> > +			echo " Install on Debian based systems"
> > +			echo "	sudo apt -y install jq"
> > +			echo " Install on RHEL based systems"
> > +			echo "	sudo yum -y install jq"
> > +			echo " Install on Fedora based systems"
> > +			echo "	sudo dnf -y install jq"
> > +		fi
> >  		exit $ksft_skip
> >  	fi
> >  }
> > --
> > 2.37.1
> 
> This is very specific to `jq` command. What's special with `jq` and not
> others? If methods have to be shown, they should be shown for all the
> programs which are not installed.

Agree. The user could decide if jq should be install via REQUIRE_JQ. There are
also other cmds that vendor may not build by default. I didn't see any
selftests need to handle the installation. The users should takes care of it.

require_command() has takes care most of the needed cmds. If we want to
improve the user's experience for the needed cmds. I think add the needed cmds
to README file is better.

Thanks
Hangbin
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 37ae49d47853..c4121856fe06 100755
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -152,6 +152,14 @@  require_command()

 	if [[ ! -x "$(command -v "$cmd")" ]]; then
 		echo "SKIP: $cmd not installed"
+		if [[ $cmd == "jq" ]]; then
+			echo " Install on Debian based systems"
+			echo "	sudo apt -y install jq"
+			echo " Install on RHEL based systems"
+			echo "	sudo yum -y install jq"
+			echo " Install on Fedora based systems"
+			echo "	sudo dnf -y install jq"
+		fi
 		exit $ksft_skip
 	fi
 }