threads / patch / 51307

patch, 5 partst9602-cvsimport-branches-tags: exclude test if cvs is not installed

Subject: [Patch 3/5] t9602-cvsimport-branches-tags: exclude test if cvs is not installed

## tl;dr

9 messages between Jun 13, 2019 and Jun 13, 2019. Diffs are folded; open one to read it.

replies: 8people: 3as markdown or json

randall.s.becker@rogers.com· Jun 13, 2019, 18:53 UTC · lore

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

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.

Randall S. Becker (5):
  t9600-cvsimport: exclude test if cvs is not installed
  t9601-cvsimport-vendor-branch: exclude test if cvs is not installed
  t9602-cvsimport-branches-tags: exclude test if cvs is not installed
  t9603-cvsimport-patchsets: exclude test if cvs is not installed
  t9604-cvsimport-timestamps: exclude test if cvs is not installed
 t/t9600-cvsimport.sh               | 7 +++++++
 t/t9601-cvsimport-vendor-branch.sh | 8 ++++++++
 t/t9602-cvsimport-branches-tags.sh | 7 +++++++
 t/t9603-cvsimport-patchsets.sh     | 7 +++++++
 t/t9604-cvsimport-timestamps.sh    | 7 +++++++
 5 files changed, 36 insertions(+)
-- 
2.22.0
randall.s.becker@rogers.com· Jun 13, 2019, 18:53 UTC · re: randall.s.becker@rogers.com · lore
From: "Randall S. Becker" <rsbecker@nexbridge.com>

The t9602-cvsimport-branches-tags test requires the cvs package to be installed on the system on which the test is being run. The test will fail if cvs is not installed. The patch checks that cvs is installed by running the object without arguments, which should complete successfully if available.

Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
---
 t/t9602-cvsimport-branches-tags.sh | 7 +++++++
 1 file changed, 7 insertions(+)
Show changes to t/t9602-cvsimport-branches-tags.sh +7 −0
diff --git a/t/t9602-cvsimport-branches-tags.sh b/t/t9602-cvsimport-branches-tags.sh
index e1db323f54..52e8507725 100755
--- a/t/t9602-cvsimport-branches-tags.sh
+++ b/t/t9602-cvsimport-branches-tags.sh
@@ -6,6 +6,13 @@
 test_description='git cvsimport handling of branches and tags'
 . ./lib-cvs.sh
 
+cvs >/dev/null 2>&1
+if test $? -ne 1
+then
+	skip_all='skipping git-cvsimport tests, cvs not found'
+	test_done
+fi
+
 setup_cvs_test_repository t9602
 
 test_expect_success PERL 'import module' '

-- 
2.22.0
randall.s.becker@rogers.com· Jun 13, 2019, 18:53 UTC · re: randall.s.becker@rogers.com · lore

[Patch 5/5] t9604-cvsimport-timestamps: exclude test if cvs is not installed

From: "Randall S. Becker" <rsbecker@nexbridge.com>

The t9604-cvsimport-timestamps test requires the cvs package to be installed on the system on which the test is being run. The test will fail if cvs is not installed. The patch checks that cvs is installed by running the object without arguments, which should complete successfully if available.

Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
---
 t/t9604-cvsimport-timestamps.sh | 7 +++++++
 1 file changed, 7 insertions(+)
Show changes to t/t9604-cvsimport-timestamps.sh +7 −0
diff --git a/t/t9604-cvsimport-timestamps.sh b/t/t9604-cvsimport-timestamps.sh
index 2ff4aa932d..1fbbd179c1 100755
--- a/t/t9604-cvsimport-timestamps.sh
+++ b/t/t9604-cvsimport-timestamps.sh
@@ -3,6 +3,13 @@
 test_description='git cvsimport timestamps'
 . ./lib-cvs.sh
 
+cvs >/dev/null 2>&1
+if test $? -ne 1
+then
+	skip_all='skipping git-cvsimport tests, cvs not found'
+	test_done
+fi
+
 setup_cvs_test_repository t9604
 
 test_expect_success PERL 'check timestamps are UTC (TZ=CST6CDT)' '

-- 
2.22.0
randall.s.becker@rogers.com· Jun 13, 2019, 18:53 UTC · re: randall.s.becker@rogers.com · lore

[Patch 4/5] t9603-cvsimport-patchsets: exclude test if cvs is not installed

From: "Randall S. Becker" <rsbecker@nexbridge.com>

The t9603-cvsimport-patchsets test requires the cvs package to be installed on the system on which the test is being run. The test will fail if cvs is not installed. The patch checks that cvs is installed by running the object without arguments, which should complete successfully if available.

Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
---
 t/t9603-cvsimport-patchsets.sh | 7 +++++++
 1 file changed, 7 insertions(+)
Show changes to t/t9603-cvsimport-patchsets.sh +7 −0
diff --git a/t/t9603-cvsimport-patchsets.sh b/t/t9603-cvsimport-patchsets.sh
index 3e64b11eac..354cd66400 100755
--- a/t/t9603-cvsimport-patchsets.sh
+++ b/t/t9603-cvsimport-patchsets.sh
@@ -14,6 +14,13 @@
 test_description='git cvsimport testing for correct patchset estimation'
 . ./lib-cvs.sh
 
+cvs >/dev/null 2>&1
+if test $? -ne 1
+then
+	skip_all='skipping git-cvsimport tests, cvs not found'
+	test_done
+fi
+
 setup_cvs_test_repository t9603
 
 test_expect_failure PERL 'import with criss cross times on revisions' '

-- 
2.22.0
randall.s.becker@rogers.com· Jun 13, 2019, 18:53 UTC · re: randall.s.becker@rogers.com · lore

[Patch 2/5] t9601-cvsimport-vendor-branch: exclude test if cvs is not installed

From: "Randall S. Becker" <rsbecker@nexbridge.com>

The t9601-cvsimport-vendor-branch test requires the cvs package to be installed on the system on which the test is being run. The test will fail if cvs is not installed. The patch checks that cvs is installed by running the object without arguments, which should complete successfully if available.

Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
---
 t/t9601-cvsimport-vendor-branch.sh | 8 ++++++++
 1 file changed, 8 insertions(+)
Show changes to t/t9601-cvsimport-vendor-branch.sh +8 −0
diff --git a/t/t9601-cvsimport-vendor-branch.sh b/t/t9601-cvsimport-vendor-branch.sh
index 827d39f5bf..a473f07d2d 100755
--- a/t/t9601-cvsimport-vendor-branch.sh
+++ b/t/t9601-cvsimport-vendor-branch.sh
@@ -32,8 +32,16 @@
 #       tag has been removed.
 
 test_description='git cvsimport handling of vendor branches'
+
 . ./lib-cvs.sh
 
+cvs >/dev/null 2>&1
+if test $? -ne 1
+then
+	skip_all='skipping git-cvsimport tests, cvs not found'
+	test_done
+fi
+
 setup_cvs_test_repository t9601
 
 test_expect_success PERL 'import a module with a vendor branch' '

-- 
2.22.0
randall.s.becker@rogers.com· Jun 13, 2019, 18:53 UTC · re: randall.s.becker@rogers.com · lore

[Patch 1/5] t9600-cvsimport: exclude test if cvs is not installed

From: "Randall S. Becker" <rsbecker@nexbridge.com>

The t9600-cvsimport test requires the cvs package to be installed on the system on which the test is being run. The test will fail if cvs is not installed. The patch checks that cvs is installed by running the object without arguments, which should complete successfully if available.

Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
---
 t/t9600-cvsimport.sh | 7 +++++++
 1 file changed, 7 insertions(+)
Show changes to t/t9600-cvsimport.sh +7 −0
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 251fdd66c4..d6bf38918b 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -3,6 +3,13 @@
 test_description='git cvsimport basic tests'
 . ./lib-cvs.sh
 
+cvs >/dev/null 2>&1
+if test $? -ne 1
+then
+	skip_all='skipping git-cvsimport tests, cvs not found'
+	test_done
+fi
+
 if ! test_have_prereq NOT_ROOT; then
 	skip_all='When cvs is compiled with CVS_BADROOT commits as root fail'
 	test_done

-- 
2.22.0
Jeff King· Jun 13, 2019, 19:06 UTC · re: randall.s.becker@rogers.com · lore

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

On Thu, Jun 13, 2019 at 02:53:08PM -0400, randall.s.becker@rogers.com wrote:
Show 7 quoted lines
> 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).

-Peff
Randall S. Becker· Jun 13, 2019, 19:30 UTC · re: Jeff King · lore

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

On June 13, 2019 3:07 PM, Peff wrote:
Show 27 quoted lines
> 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.
Randall
Randall S. Becker· Jun 13, 2019, 22:46 UTC · re: Randall S. Becker · lore

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

On June 13, 2019 3:31 PM, I wrote:
Show 51 quoted lines
> 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

← back to recent threads