RE: [Patch 0/5] Add exclusions for tests requiring cvs where cvs is not installed

["Randall S. Becker" <rsbecker@nexbridge.com>]

On June 13, 2019 3:31 PM, I wrote:
> On June 13, 2019 3:07 PM, Peff wrote:
> > On Thu, Jun 13, 2019 at 02:53:08PM -0400, randall.s.becker@rogers.com
> > wrote:
> >
> > > From: "Randall S. Becker" <rsbecker@nexbridge.com>
> > >
> > > t9600 to t9604 currently depend on cvs to function correctly,
> > > otherwise all of those tests fail. This patch follows an existing
> > > pattern of from the t9400 series by attempting to run cvs without
> > > arguments, which succeeds if installed, and skipping the test if the
> > > command fails.
> >
> > Hrm. I don't have cvs installed, and those tests are properly skipped for me.
> > That's because they include lib-cvs.sh, which has:
> >
> >   if ! type cvs >/dev/null 2>&1
> >   then
> >           skip_all='skipping cvsimport tests, cvs not found'
> >           test_done
> >   fi
> >
> > Why doesn't that work for you? Does the "type" check not work (e.g.,
> > you have something called "cvs" but it does not behave as we expect)?
> > If so, then it sounds like we just need to harmonize that with the other
> checks.
> >
> > It also sounds like the t9400 tests could be using lib-cvs to avoid
> > duplicating logic, though it might need some refactoring (they don't
> > need cvsps, for example).
> 
> The t9400 tests use the same technique as I used - and I mistakenly trusted it.
> The type check does not fail.
> 
> if ! type cvs >/dev/null 2>&1
> then
> 	echo "oops"
> fi
> 
> does not print "oops". type is reporting $?=0 and a legitimate file in
> /usr/local/bin/cvs. Confusingly, t9400 skips, but type reports a valid path. I
> think the test done in the t9400 series is not correct.
> 
> cvs >/dev/null 2>&1 on the platform causes $?=255, while a blah >/dev/null
> 2>&1 reports $?=127.
> 
> There is something else going on causing the cvs-related tests to fail that this
> patch might be hiding. We do have cvsps so I'm now much more confused by
> the whole thing.
> 
> Let's drop this patch for now. I was premature on this patch and need to dig
> deeper as to what is going on.

We do not need the patch. The situation was caused by an old version of CVS (pre 1.11)  that was causing t9600... to fail. The message was buried under --verbose. I ported CVS 1.11.23 and CVS tests are now working. My bad.

Cheers,
Randall