Re: [PATCH 2/2] Don't rely on strerror text when testing rmdir failure

2014-03-31 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de 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 cbaile...@bloomberg.net
 ---
  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^ 2actual 
 +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


Re: [PATCH 2/2] Don't rely on strerror text when testing rmdir failure

2014-03-29 Thread Jens Lehmann
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 cbaile...@bloomberg.net
 ---
  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^ 2actual 
 + 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 2actual 
 - 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^ 2actual 
 - 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

2014-03-29 Thread Charles Bailey
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^ 2actual 
  +   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 2actual 
  -   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