Re: [PATCH nft 01/12] tests: shell: export DIFF to use it from feature scripts

[Thomas Haller <thaller@redhat.com>]

On Thu, 2023-11-09 at 20:14 +0100, Pablo Neira Ayuso wrote:
> On Thu, Nov 09, 2023 at 06:49:21PM +0100, Thomas Haller wrote:
> > On Thu, 2023-11-09 at 17:22 +0100, Pablo Neira Ayuso wrote:
> > > export DIFF so it can be used from feature scripts to probe the
> > > kernel.
> > > 
> > > +DIFF="$(which diff)"
> > > +if [ ! -x "$DIFF" ] ; then
> > > +	DIFF=true
> > > +fi
> > > +export DIFF
> > 
> > 
> > what is the purpose of having $DIFF variable at all?
> > Why not require to have `diff` installed?
> > 
> > Maybe that justification is somewhere in the history of the
> > project. If
> > so, could you drop one line in the commit message what the point
> > is?
> 
> It is all available in git annotate:
> 
> 68310ba0f9c2 ("tests: shell: Search diff tool once and for all")
> 7d93e2c2fbc7 ("tests: shell: autogenerate dump verification")

First use of $DIFF comes from 

  3fb3bb603374 ('tests/listing: add some listing tests')

which says:

    In order to ease debug in case of failure, if the diff tool is in the system,
    then a textual diff is printed.


With the `true` fallback, checks are skipped. It would be possible, to
use a fallback that still checks for equality (albeit without fancy
diff output).

But really. Just require everybody to install a diff program.


> 
> I just need to move it around so I can use it from feature scripts.
> If you prefer I can just use 'diff' instead from the feature scripts.

Sure. The patch is fine.


I think one day,

   sed 's/\$DIFF\>/diff/g' -i $(git grep -l DIFF tests/shell/)

should be done.



Thomas