[PATCH 2/2] Don't rely on strerror text when testing rmdir failure
AIX doesn't make a distiction between EEXIST and ENOTEMPTY so relying on the strerror string for the rmdir failure is fragile. Just test that the start of the string matches the Git controlled "failed to rmdir..." error. The exact text of the OS generated error string isn't important to the test. Signed-off-by: Charles Bailey --- t/t3600-rm.sh | 5 ++--- t/t7001-mv.sh | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index 3d30581..23eed17 100755 --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -709,10 +709,9 @@ test_expect_success 'checking out a commit after submodule removal needs manual git commit -m "submodule removal" submod && git checkout HEAD^ && git submodule update && - git checkout -q HEAD^ 2>actual && + git checkout -q HEAD^ 2>/dev/null && git checkout -q master 2>actual && - echo "warning: unable to rmdir submod: Directory not empty" >expected && - test_i18ncmp expected actual && + test_i18ngrep "^warning: unable to rmdir submod:" actual && git status -s submod >actual && echo "?? submod/" >expected && test_cmp expected actual && diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh index 215d43d..34fb1af 100755 --- a/t/t7001-mv.sh +++ b/t/t7001-mv.sh @@ -447,8 +447,7 @@ test_expect_success 'checking out a commit before submodule moved needs manual u git mv sub sub2 && git commit -m "moved sub to sub2" && git checkout -q HEAD^ 2>actual && - echo "warning: unable to rmdir sub2: Directory not empty" >expected && - test_i18ncmp expected actual && + test_i18ngrep "^warning: unable to rmdir sub2:" actual && git status -s sub2 >actual && echo "?? sub2/" >expected && test_cmp expected actual && -- 1.8.5.1.2.ge5d1dab -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Don't rely on strerror text when testing rmdir failure
Am 29.03.2014 16:39, schrieb Charles Bailey: > AIX doesn't make a distiction between EEXIST and ENOTEMPTY so relying on > the strerror string for the rmdir failure is fragile. Just test that the > start of the string matches the Git controlled "failed to rmdir..." > error. The exact text of the OS generated error string isn't important > to the test. Makes sense. > Signed-off-by: Charles Bailey > --- > t/t3600-rm.sh | 5 ++--- > t/t7001-mv.sh | 3 +-- > 2 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh > index 3d30581..23eed17 100755 > --- a/t/t3600-rm.sh > +++ b/t/t3600-rm.sh > @@ -709,10 +709,9 @@ test_expect_success 'checking out a commit after > submodule removal needs manual > git commit -m "submodule removal" submod && > git checkout HEAD^ && > git submodule update && > - git checkout -q HEAD^ 2>actual && > + git checkout -q HEAD^ 2>/dev/null && Isn't this unrelated to the strerror issue you are fixing here? Why not just drop the redirection completely? But maybe I'm just being to pedantic here ;-) > git checkout -q master 2>actual && > - echo "warning: unable to rmdir submod: Directory not empty" >expected && > - test_i18ncmp expected actual && > + test_i18ngrep "^warning: unable to rmdir submod:" actual && > git status -s submod >actual && > echo "?? submod/" >expected && > test_cmp expected actual && > diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh > index 215d43d..34fb1af 100755 > --- a/t/t7001-mv.sh > +++ b/t/t7001-mv.sh > @@ -447,8 +447,7 @@ test_expect_success 'checking out a commit before > submodule moved needs manual u > git mv sub sub2 && > git commit -m "moved sub to sub2" && > git checkout -q HEAD^ 2>actual && > - echo "warning: unable to rmdir sub2: Directory not empty" >expected && > - test_i18ncmp expected actual && > + test_i18ngrep "^warning: unable to rmdir sub2:" actual && > git status -s sub2 >actual && > echo "?? sub2/" >expected && > test_cmp expected actual && > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Don't rely on strerror text when testing rmdir failure
On Sat, Mar 29, 2014 at 04:48:44PM +0100, Jens Lehmann wrote: > Am 29.03.2014 16:39, schrieb Charles Bailey: > > diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh > > index 3d30581..23eed17 100755 > > --- a/t/t3600-rm.sh > > +++ b/t/t3600-rm.sh > > @@ -709,10 +709,9 @@ test_expect_success 'checking out a commit after > > submodule removal needs manual > > git commit -m "submodule removal" submod && > > git checkout HEAD^ && > > git submodule update && > > - git checkout -q HEAD^ 2>actual && > > + git checkout -q HEAD^ 2>/dev/null && > > Isn't this unrelated to the strerror issue you are fixing here? > Why not just drop the redirection completely? But maybe I'm just > being to pedantic here ;-) Well, it's a write to 'actual' and the next thing that tests the contents of 'actual' is the thing that I'm fixing so it's almost related. (See context kept below.) I changed the redirection here while investigating the bug. The redirected output was being overwritten immediately and this was a more explicit way to write "I don't care about whatever goes to stderr from this command" which confused me less that merely overwriting the file on the next line, but perhaps simply not redirecting is better. I really didn't give it much thought. > > > git checkout -q master 2>actual && > > - echo "warning: unable to rmdir submod: Directory not empty" >expected && > > - test_i18ncmp expected actual && > > + test_i18ngrep "^warning: unable to rmdir submod:" actual && Charles. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Don't rely on strerror text when testing rmdir failure
Jens Lehmann writes: > Am 29.03.2014 16:39, schrieb Charles Bailey: >> AIX doesn't make a distiction between EEXIST and ENOTEMPTY so relying on >> the strerror string for the rmdir failure is fragile. Just test that the >> start of the string matches the Git controlled "failed to rmdir..." >> error. The exact text of the OS generated error string isn't important >> to the test. > > Makes sense. > >> Signed-off-by: Charles Bailey >> --- >> t/t3600-rm.sh | 5 ++--- >> t/t7001-mv.sh | 3 +-- >> 2 files changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh >> index 3d30581..23eed17 100755 >> --- a/t/t3600-rm.sh >> +++ b/t/t3600-rm.sh >> @@ -709,10 +709,9 @@ test_expect_success 'checking out a commit after >> submodule removal needs manual >> git commit -m "submodule removal" submod && >> git checkout HEAD^ && >> git submodule update && >> -git checkout -q HEAD^ 2>actual && >> +git checkout -q HEAD^ 2>/dev/null && > > Isn't this unrelated to the strerror issue you are fixing here? > Why not just drop the redirection completely? But maybe I'm just > being to pedantic here ;-) No, that sounds like a very reasonable suggestion. Especially given that the redirection destination is overwritten immediately after. In general tests should not have to squelch their standard error output with 2>/dev/null; that is a job for the test harness, and they will be shown in the output of "./t3600-rm -v" to serve as anchor point while finding where a test goes wrong, which is a good thing. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
