Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 11 Aug 2022 09:41:03 +0800
From:   Hangbin Liu <liuhangbin@gmail.com>
To:     Siddh Raman Pant <code@siddh.me>
Cc:     Piyush Thange <pthange19@gmail.com>, davem <davem@davemloft.net>,
        edumazet <edumazet@google.com>, kuba <kuba@kernel.org>,
        pabeni <pabeni@redhat.com>, shuah <shuah@kernel.org>,
        "vladimir.oltean" <vladimir.oltean@nxp.com>,
        idosch <idosch@nvidia.com>, petrm <petrm@nvidia.com>,
        troglobit <troglobit@gmail.com>, amcohen <amcohen@nvidia.com>,
        tobias <tobias@waldekranz.com>,
        "po-hsu.lin" <po-hsu.lin@canonical.com>,
        netdev <netdev@vger.kernel.org>,
        linux-kselftest <linux-kselftest@vger.kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        linux-kernel-mentees 
        <linux-kernel-mentees@lists.linuxfoundation.org>
Subject: Re: [PATCH] selftests:net:forwarding: Included install command
Message-ID: <YvReL0HkZt639BoO@Laptop-X1>
References: <20220810093508.33790-1-pthange19@gmail.com>
 <182872c2de1.4461d55242061.8862004854197621952@siddh.me>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <182872c2de1.4461d55242061.8862004854197621952@siddh.me>
Precedence: bulk

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