{"thread":{"id":"51307","subject":"[Patch 3/5] t9602-cvsimport-branches-tags: exclude test if cvs is not installed","startedAt":"2019-06-13T18:53:30Z","lastAt":"2019-06-13T22:46:28Z","messageCount":9,"participants":["randall.s.becker@rogers.com","Jeff King","Randall S. Becker"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"377170","messageId":"20190613185313.16120-4-randall.s.becker@rogers.com","threadId":"51307","inReplyTo":"20190613185313.16120-1-randall.s.becker@rogers.com","subject":"[Patch 3/5] t9602-cvsimport-branches-tags: exclude test if cvs is not installed","fromName":"","fromEmail":"randall.s.becker@rogers.com","sentAt":"2019-06-13T18:53:11Z","receivedAt":"2019-06-13T18:53:30Z","isPatch":true,"sender":{"key":"randall.s.becker@rogers.com","avatar":null},"body":"From: \"Randall S. Becker\" \n\nThe t9602-cvsimport-branches-tags test requires the cvs package to\nbe installed on the system on which the test is being run. The test\nwill fail if cvs is not installed. The patch checks that cvs is\ninstalled by running the object without arguments, which should\ncomplete successfully if available.\n\nSigned-off-by: Randall S. Becker \n---\n t/t9602-cvsimport-branches-tags.sh | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/t/t9602-cvsimport-branches-tags.sh b/t/t9602-cvsimport-branches-tags.sh\nindex e1db323f54..52e8507725 100755\n--- a/t/t9602-cvsimport-branches-tags.sh\n+++ b/t/t9602-cvsimport-branches-tags.sh\n@@ -6,6 +6,13 @@\n test_description='git cvsimport handling of branches and tags'\n . ./lib-cvs.sh\n \n+cvs >/dev/null 2>&1\n+if test $? -ne 1\n+then\n+\tskip_all='skipping git-cvsimport tests, cvs not found'\n+\ttest_done\n+fi\n+\n setup_cvs_test_repository t9602\n \n test_expect_success PERL 'import module' '\n-- \n2.22.0\n\n"},{"id":"377171","messageId":"20190613185313.16120-6-randall.s.becker@rogers.com","threadId":"51307","inReplyTo":"20190613185313.16120-1-randall.s.becker@rogers.com","subject":"[Patch 5/5] t9604-cvsimport-timestamps: exclude test if cvs is not installed","fromName":"","fromEmail":"randall.s.becker@rogers.com","sentAt":"2019-06-13T18:53:13Z","receivedAt":"2019-06-13T18:53:40Z","isPatch":true,"sender":{"key":"randall.s.becker@rogers.com","avatar":null},"body":"From: \"Randall S. Becker\" \n\nThe t9604-cvsimport-timestamps test requires the cvs package to\nbe installed on the system on which the test is being run. The test\nwill fail if cvs is not installed. The patch checks that cvs is\ninstalled by running the object without arguments, which should\ncomplete successfully if available.\n\nSigned-off-by: Randall S. Becker \n---\n t/t9604-cvsimport-timestamps.sh | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/t/t9604-cvsimport-timestamps.sh b/t/t9604-cvsimport-timestamps.sh\nindex 2ff4aa932d..1fbbd179c1 100755\n--- a/t/t9604-cvsimport-timestamps.sh\n+++ b/t/t9604-cvsimport-timestamps.sh\n@@ -3,6 +3,13 @@\n test_description='git cvsimport timestamps'\n . ./lib-cvs.sh\n \n+cvs >/dev/null 2>&1\n+if test $? -ne 1\n+then\n+\tskip_all='skipping git-cvsimport tests, cvs not found'\n+\ttest_done\n+fi\n+\n setup_cvs_test_repository t9604\n \n test_expect_success PERL 'check timestamps are UTC (TZ=CST6CDT)' '\n-- \n2.22.0\n\n"},{"id":"377172","messageId":"20190613185313.16120-5-randall.s.becker@rogers.com","threadId":"51307","inReplyTo":"20190613185313.16120-1-randall.s.becker@rogers.com","subject":"[Patch 4/5] t9603-cvsimport-patchsets: exclude test if cvs is not installed","fromName":"","fromEmail":"randall.s.becker@rogers.com","sentAt":"2019-06-13T18:53:12Z","receivedAt":"2019-06-13T18:53:42Z","isPatch":true,"sender":{"key":"randall.s.becker@rogers.com","avatar":null},"body":"From: \"Randall S. Becker\" \n\nThe t9603-cvsimport-patchsets test requires the cvs package to\nbe installed on the system on which the test is being run. The test\nwill fail if cvs is not installed. The patch checks that cvs is\ninstalled by running the object without arguments, which should\ncomplete successfully if available.\n\nSigned-off-by: Randall S. Becker \n---\n t/t9603-cvsimport-patchsets.sh | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/t/t9603-cvsimport-patchsets.sh b/t/t9603-cvsimport-patchsets.sh\nindex 3e64b11eac..354cd66400 100755\n--- a/t/t9603-cvsimport-patchsets.sh\n+++ b/t/t9603-cvsimport-patchsets.sh\n@@ -14,6 +14,13 @@\n test_description='git cvsimport testing for correct patchset estimation'\n . ./lib-cvs.sh\n \n+cvs >/dev/null 2>&1\n+if test $? -ne 1\n+then\n+\tskip_all='skipping git-cvsimport tests, cvs not found'\n+\ttest_done\n+fi\n+\n setup_cvs_test_repository t9603\n \n test_expect_failure PERL 'import with criss cross times on revisions' '\n-- \n2.22.0\n\n"},{"id":"377173","messageId":"20190613185313.16120-3-randall.s.becker@rogers.com","threadId":"51307","inReplyTo":"20190613185313.16120-1-randall.s.becker@rogers.com","subject":"[Patch 2/5] t9601-cvsimport-vendor-branch: exclude test if cvs is not installed","fromName":"","fromEmail":"randall.s.becker@rogers.com","sentAt":"2019-06-13T18:53:10Z","receivedAt":"2019-06-13T18:53:43Z","isPatch":true,"sender":{"key":"randall.s.becker@rogers.com","avatar":null},"body":"From: \"Randall S. Becker\" \n\nThe t9601-cvsimport-vendor-branch test requires the cvs package to\nbe installed on the system on which the test is being run. The test\nwill fail if cvs is not installed. The patch checks that cvs is\ninstalled by running the object without arguments, which should\ncomplete successfully if available.\n\nSigned-off-by: Randall S. Becker \n---\n t/t9601-cvsimport-vendor-branch.sh | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/t/t9601-cvsimport-vendor-branch.sh b/t/t9601-cvsimport-vendor-branch.sh\nindex 827d39f5bf..a473f07d2d 100755\n--- a/t/t9601-cvsimport-vendor-branch.sh\n+++ b/t/t9601-cvsimport-vendor-branch.sh\n@@ -32,8 +32,16 @@\n # tag has been removed.\n \n test_description='git cvsimport handling of vendor branches'\n+\n . ./lib-cvs.sh\n \n+cvs >/dev/null 2>&1\n+if test $? -ne 1\n+then\n+\tskip_all='skipping git-cvsimport tests, cvs not found'\n+\ttest_done\n+fi\n+\n setup_cvs_test_repository t9601\n \n test_expect_success PERL 'import a module with a vendor branch' '\n-- \n2.22.0\n\n"},{"id":"377174","messageId":"20190613185313.16120-2-randall.s.becker@rogers.com","threadId":"51307","inReplyTo":"20190613185313.16120-1-randall.s.becker@rogers.com","subject":"[Patch 1/5] t9600-cvsimport: exclude test if cvs is not installed","fromName":"","fromEmail":"randall.s.becker@rogers.com","sentAt":"2019-06-13T18:53:09Z","receivedAt":"2019-06-13T18:53:53Z","isPatch":true,"sender":{"key":"randall.s.becker@rogers.com","avatar":null},"body":"From: \"Randall S. Becker\" \n\nThe t9600-cvsimport test requires the cvs package to be installed on\nthe system on which the test is being run. The test will fail if cvs\nis not installed. The patch checks that cvs is installed by running\nthe object without arguments, which should complete successfully if\navailable.\n\nSigned-off-by: Randall S. Becker \n---\n t/t9600-cvsimport.sh | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh\nindex 251fdd66c4..d6bf38918b 100755\n--- a/t/t9600-cvsimport.sh\n+++ b/t/t9600-cvsimport.sh\n@@ -3,6 +3,13 @@\n test_description='git cvsimport basic tests'\n . ./lib-cvs.sh\n \n+cvs >/dev/null 2>&1\n+if test $? -ne 1\n+then\n+\tskip_all='skipping git-cvsimport tests, cvs not found'\n+\ttest_done\n+fi\n+\n if ! test_have_prereq NOT_ROOT; then\n \tskip_all='When cvs is compiled with CVS_BADROOT commits as root fail'\n \ttest_done\n-- \n2.22.0\n\n"},{"id":"377175","messageId":"20190613185313.16120-1-randall.s.becker@rogers.com","threadId":"51307","inReplyTo":null,"subject":"[Patch 0/5] Add exclusions for tests requiring cvs where cvs is not installed","fromName":"","fromEmail":"randall.s.becker@rogers.com","sentAt":"2019-06-13T18:53:08Z","receivedAt":"2019-06-13T18:54:45Z","isPatch":true,"sender":{"key":"randall.s.becker@rogers.com","avatar":null},"body":"From: \"Randall S. Becker\" \n\nt9600 to t9604 currently depend on cvs to function correctly, otherwise\nall of those tests fail. This patch follows an existing pattern of\nfrom the t9400 series by attempting to run cvs without arguments,\nwhich succeeds if installed, and skipping the test if the command\nfails.\n\nRandall S. Becker (5):\n t9600-cvsimport: exclude test if cvs is not installed\n t9601-cvsimport-vendor-branch: exclude test if cvs is not installed\n t9602-cvsimport-branches-tags: exclude test if cvs is not installed\n t9603-cvsimport-patchsets: exclude test if cvs is not installed\n t9604-cvsimport-timestamps: exclude test if cvs is not installed\n\n t/t9600-cvsimport.sh | 7 +++++++\n t/t9601-cvsimport-vendor-branch.sh | 8 ++++++++\n t/t9602-cvsimport-branches-tags.sh | 7 +++++++\n t/t9603-cvsimport-patchsets.sh | 7 +++++++\n t/t9604-cvsimport-timestamps.sh | 7 +++++++\n 5 files changed, 36 insertions(+)\n\n-- \n2.22.0\n\n"},{"id":"377178","messageId":"20190613190644.GC27217@sigill.intra.peff.net","threadId":"51307","inReplyTo":"20190613185313.16120-1-randall.s.becker@rogers.com","subject":"Re: [Patch 0/5] Add exclusions for tests requiring cvs where cvs is not installed","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-06-13T19:06:44Z","receivedAt":"2019-06-13T19:06:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 13, 2019 at 02:53:08PM -0400, randall.s.becker@rogers.com wrote:\n\n> From: \"Randall S. Becker\" \n> \n> t9600 to t9604 currently depend on cvs to function correctly, otherwise\n> all of those tests fail. This patch follows an existing pattern of\n> from the t9400 series by attempting to run cvs without arguments,\n> which succeeds if installed, and skipping the test if the command\n> fails.\n\nHrm. I don't have cvs installed, and those tests are properly skipped\nfor me. That's because they include lib-cvs.sh, which has:\n\n if ! type cvs >/dev/null 2>&1\n then\n skip_all='skipping cvsimport tests, cvs not found'\n test_done\n fi\n\nWhy doesn't that work for you? Does the \"type\" check not work (e.g., you\nhave something called \"cvs\" but it does not behave as we expect)? If so,\nthen it sounds like we just need to harmonize that with the other\nchecks.\n\nIt also sounds like the t9400 tests could be using lib-cvs to avoid\nduplicating logic, though it might need some refactoring (they don't\nneed cvsps, for example).\n\n-Peff\n"},{"id":"377182","messageId":"000101d5221e$88aa67d0$99ff3770$@nexbridge.com","threadId":"51307","inReplyTo":"20190613190644.GC27217@sigill.intra.peff.net","subject":"RE: [Patch 0/5] Add exclusions for tests requiring cvs where cvs is not installed","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2019-06-13T19:30:55Z","receivedAt":"2019-06-13T19:31:12Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On June 13, 2019 3:07 PM, Peff wrote:\n> On Thu, Jun 13, 2019 at 02:53:08PM -0400, randall.s.becker@rogers.com\n> wrote:\n> \n> > From: \"Randall S. Becker\" \n> >\n> > t9600 to t9604 currently depend on cvs to function correctly,\n> > otherwise all of those tests fail. This patch follows an existing\n> > pattern of from the t9400 series by attempting to run cvs without\n> > arguments, which succeeds if installed, and skipping the test if the\n> > command fails.\n> \n> Hrm. I don't have cvs installed, and those tests are properly skipped for me.\n> That's because they include lib-cvs.sh, which has:\n> \n> if ! type cvs >/dev/null 2>&1\n> then\n> skip_all='skipping cvsimport tests, cvs not found'\n> test_done\n> fi\n> \n> Why doesn't that work for you? Does the \"type\" check not work (e.g., you\n> have something called \"cvs\" but it does not behave as we expect)? If so, then\n> it sounds like we just need to harmonize that with the other checks.\n> \n> It also sounds like the t9400 tests could be using lib-cvs to avoid duplicating\n> logic, though it might need some refactoring (they don't need cvsps, for\n> example).\n\nThe t9400 tests use the same technique as I used - and I mistakenly trusted it. The type check does not fail.\n\nif ! type cvs >/dev/null 2>&1\nthen\n\techo \"oops\"\nfi\n\ndoes 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.\n\ncvs >/dev/null 2>&1 on the platform causes $?=255, while a blah >/dev/null 2>&1 reports $?=127.\n\nThere 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.\n\nLet's drop this patch for now. I was premature on this patch and need to dig deeper as to what is going on.\n\nRandall\n\n"},{"id":"377205","messageId":"001b01d52239$cefaa270$6cefe750$@nexbridge.com","threadId":"51307","inReplyTo":"000101d5221e$88aa67d0$99ff3770$@nexbridge.com","subject":"RE: [Patch 0/5] Add exclusions for tests requiring cvs where cvs is not installed","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2019-06-13T22:46:10Z","receivedAt":"2019-06-13T22:46:28Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On June 13, 2019 3:31 PM, I wrote:\n> On June 13, 2019 3:07 PM, Peff wrote:\n> > On Thu, Jun 13, 2019 at 02:53:08PM -0400, randall.s.becker@rogers.com\n> > wrote:\n> >\n> > > From: \"Randall S. Becker\" \n> > >\n> > > t9600 to t9604 currently depend on cvs to function correctly,\n> > > otherwise all of those tests fail. This patch follows an existing\n> > > pattern of from the t9400 series by attempting to run cvs without\n> > > arguments, which succeeds if installed, and skipping the test if the\n> > > command fails.\n> >\n> > Hrm. I don't have cvs installed, and those tests are properly skipped for me.\n> > That's because they include lib-cvs.sh, which has:\n> >\n> > if ! type cvs >/dev/null 2>&1\n> > then\n> > skip_all='skipping cvsimport tests, cvs not found'\n> > test_done\n> > fi\n> >\n> > Why doesn't that work for you? Does the \"type\" check not work (e.g.,\n> > you have something called \"cvs\" but it does not behave as we expect)?\n> > If so, then it sounds like we just need to harmonize that with the other\n> checks.\n> >\n> > It also sounds like the t9400 tests could be using lib-cvs to avoid\n> > duplicating logic, though it might need some refactoring (they don't\n> > need cvsps, for example).\n> \n> The t9400 tests use the same technique as I used - and I mistakenly trusted it.\n> The type check does not fail.\n> \n> if ! type cvs >/dev/null 2>&1\n> then\n> \techo \"oops\"\n> fi\n> \n> does not print \"oops\". type is reporting $?=0 and a legitimate file in\n> /usr/local/bin/cvs. Confusingly, t9400 skips, but type reports a valid path. I\n> think the test done in the t9400 series is not correct.\n> \n> cvs >/dev/null 2>&1 on the platform causes $?=255, while a blah >/dev/null\n> 2>&1 reports $?=127.\n> \n> There is something else going on causing the cvs-related tests to fail that this\n> patch might be hiding. We do have cvsps so I'm now much more confused by\n> the whole thing.\n> \n> Let's drop this patch for now. I was premature on this patch and need to dig\n> deeper as to what is going on.\n\nWe 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.\n\nCheers,\nRandall\n\n\n"}]}