Re: [PATCH] tests/cp/cp-reply: --reply only accepts valid arguments

[Bob Proulx]

Brock Noland wrote:
> tests is an area where I may be of service. However, I am not overly
> familiar with git or how the tests should be written. Before I write a
> bunch, I wanted to submit a single test and make sure I am `doing it
> right.' As such, I wrote the relatively trivial test below as this
> code appeared to be uncovered in:

Nice!

>  tests/cp/cp-reply |   33 +++++++++++++++++++++++++++++++++

I think for deprecated things it would be nice if deprecated showed up
in the name.  Not a big deal though.

> +cp --reply a 2>&1 | grep -q 'Valid arguments are' || fail=1
> +cp --reply yes a b 2>&1 | grep -q 'reply option is deprecated' || fail=1

So far none of the other tests make use of 'grep -q' yet.  Coreutils
may be using a native grep which does not support -q.  Instead the
older form of redirecting to /dev/null has been used for more
portability.

  cp --reply a 2>&1 | grep 'Valid arguments are' >/dev/null || fail=1

Using the return code of the pipeline seems prone to issues with some
/bin/sh implementations.  It has been a while and perhaps it isn't a
problem any more and if so I apologize for spreading FUD on this
point.  Personally I would avoid it and use temporary files.  This
eliminates any questions about it.  And as we will see in a moment it
is useful for other reasons.

  cp --reply a >out 2>err
  grep 'Valid arguments are' err >/dev/null || fail=1

Also in this case there shouldn't be any stdout at all.  We can test
for that too this way.  If the command outputs to stdout this becomes
a failure now.

  cp --reply a >out 2>err
  grep 'Valid arguments are' err >/dev/null || fail=1
  test -s out && fail=1

When debugging this later and looking only at the error logs how will
this be debugged when it fails?  That is if this fails then an error
log would show only this following:

  + touch a
  + cp --reply a
  + grep 'Valid arguments are' err
  + fail=1

Hmm...  What could be the problem?  What message would it have
produced other than the one being tested for?  The person reporting
the error may not be very good at being able to debug this and so it
helps us to think about that case now to make it easier for us later.
I like to dump the contents of the file if it fails.

  cp --reply a >out 2>err
  grep 'Valid arguments are' err >/dev/null || { cat err; fail=1 ;}
  test -s out && { cat out; fail=1 ;}

Then we can see something like this (contrived) example:

  + cp a
  + grep 'Valid arguments are' err
  + cat err
  cp: missing destination file operand after `a'
  Try `cp --help' for more information.
  + fail=1

This should be slightly easier to debug from the logs in the future.

Hopefully this helps,
Bob