| Message ID | 20200718094840.31269-1-martin.agren@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | t3200: don't grep for `strerror()` string | expand |
Martin Ågren <martin.agren@gmail.com> writes: > In 6b7093064a ("t3200: test for specific errors", 2020-06-15), we > learned to grep stderr to ensure that the failing `git branch` > invocations fail for the right reason. In two of these tests, we grep > for "File exists", expecting the string to show up there since config.c > calls `error_errno()`, which ends up including `strerror(errno)` in the > error message. > > But as we saw in 4605a73073 ("t1091: don't grep for `strerror()` > string", 2020-03-08), there exists at least one implementation where > `strerror()` yields a slightly different string than the one we're > grepping for. In particular, these tests fail on the NonStop platform. > > Similar to 4605a73073, grep for the beginning of the string instead to > avoid relying on `strerror()` behavior. > > Reported-by: Randall S. Becker <rsbecker@nexbridge.com> > Signed-off-by: Martin Ågren <martin.agren@gmail.com> > --- Yup, that looks like a good description and a fix to check only the parts of the message we write, ignoring what the system library may add. > Hi Randall, > > Does this fix the test for you? > > Martin Hopefully it would ;-)
On Sat, 18 Jul 2020 at 19:59, Junio C Hamano <gitster@pobox.com> wrote: > > Martin Ågren <martin.agren@gmail.com> writes: > > > Similar to 4605a73073, grep for the beginning of the string instead to > > avoid relying on `strerror()` behavior. > Yup, that looks like a good description and a fix to check only the > parts of the message we write, ignoring what the system library may > add. > > > Hi Randall, > > > > Does this fix the test for you? > > > > Martin > > Hopefully it would ;-) Heh, yeah, I feel pretty confident it should, but testing beats believing, and I don't have access to a NonStop box. ;-) Martin
On July 18, 2020 5:49 AM, Martin Ågren Wrote: > In 6b7093064a ("t3200: test for specific errors", 2020-06-15), we learned to > grep stderr to ensure that the failing `git branch` invocations fail for the right > reason. In two of these tests, we grep for "File exists", expecting the string > to show up there since config.c calls `error_errno()`, which ends up including > `strerror(errno)` in the error message. > > But as we saw in 4605a73073 ("t1091: don't grep for `strerror()` string", > 2020-03-08), there exists at least one implementation where `strerror()` > yields a slightly different string than the one we're grepping for. In particular, > these tests fail on the NonStop platform. > > Similar to 4605a73073, grep for the beginning of the string instead to avoid > relying on `strerror()` behavior. > > Reported-by: Randall S. Becker <rsbecker@nexbridge.com> > Signed-off-by: Martin Ågren <martin.agren@gmail.com> > --- > Hi Randall, > > Does this fix the test for you? > > Martin > > t/t3200-branch.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index > b6aa04bbec..4c0734157b 100755 > --- a/t/t3200-branch.sh > +++ b/t/t3200-branch.sh > @@ -870,7 +870,7 @@ test_expect_success '--set-upstream-to fails on > locked config' ' > >.git/config.lock && > git branch locked && > test_must_fail git branch --set-upstream-to locked 2>err && > - test_i18ngrep "could not lock config file .git/config: File exists" err > + test_i18ngrep "could not lock config file .git/config" err > ' > > test_expect_success 'use --set-upstream-to modify HEAD' ' > @@ -901,7 +901,7 @@ test_expect_success '--unset-upstream should fail if > config is locked' ' > git branch --set-upstream-to locked && > >.git/config.lock && > test_must_fail git branch --unset-upstream 2>err && > - test_i18ngrep "could not lock config file .git/config: File exists" err > + test_i18ngrep "could not lock config file .git/config" err > ' > > test_expect_success 'test --unset-upstream on HEAD' ' It should work, yes. You could go as far as the ':' if you were worried about the path of the .git/config file.
On July 19, 2020 1:09 PM, I wrote: > On July 18, 2020 5:49 AM, Martin Ågren Wrote: > > In 6b7093064a ("t3200: test for specific errors", 2020-06-15), we > > learned to grep stderr to ensure that the failing `git branch` > > invocations fail for the right reason. In two of these tests, we grep > > for "File exists", expecting the string to show up there since > > config.c calls `error_errno()`, which ends up including `strerror(errno)` in > the error message. > > > > But as we saw in 4605a73073 ("t1091: don't grep for `strerror()` > > string", 2020-03-08), there exists at least one implementation where > > `strerror()` yields a slightly different string than the one we're > > grepping for. In particular, these tests fail on the NonStop platform. > > > > Similar to 4605a73073, grep for the beginning of the string instead to > > avoid relying on `strerror()` behavior. > > > > Reported-by: Randall S. Becker <rsbecker@nexbridge.com> > > Signed-off-by: Martin Ågren <martin.agren@gmail.com> > > --- > > Hi Randall, > > > > Does this fix the test for you? > > > > Martin > > > > t/t3200-branch.sh | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index > > b6aa04bbec..4c0734157b 100755 > > --- a/t/t3200-branch.sh > > +++ b/t/t3200-branch.sh > > @@ -870,7 +870,7 @@ test_expect_success '--set-upstream-to fails on > > locked config' ' > > >.git/config.lock && > > git branch locked && > > test_must_fail git branch --set-upstream-to locked 2>err && > > - test_i18ngrep "could not lock config file .git/config: File exists" err > > + test_i18ngrep "could not lock config file .git/config" err > > ' > > > > test_expect_success 'use --set-upstream-to modify HEAD' ' > > @@ -901,7 +901,7 @@ test_expect_success '--unset-upstream should fail > > if config is locked' ' > > git branch --set-upstream-to locked && > > >.git/config.lock && > > test_must_fail git branch --unset-upstream 2>err && > > - test_i18ngrep "could not lock config file .git/config: File exists" err > > + test_i18ngrep "could not lock config file .git/config" err > > ' > > > > test_expect_success 'test --unset-upstream on HEAD' ' > > It should work, yes. You could go as far as the ':' if you were worried about > the path of the .git/config file. Yup, our Jenkins pipeline picked it up before I had a chance to the run the test myself. Passes. (We're up to t3311 on the current HEAD on master). Thanks, Randall
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index b6aa04bbec..4c0734157b 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -870,7 +870,7 @@ test_expect_success '--set-upstream-to fails on locked config' ' >.git/config.lock && git branch locked && test_must_fail git branch --set-upstream-to locked 2>err && - test_i18ngrep "could not lock config file .git/config: File exists" err + test_i18ngrep "could not lock config file .git/config" err ' test_expect_success 'use --set-upstream-to modify HEAD' ' @@ -901,7 +901,7 @@ test_expect_success '--unset-upstream should fail if config is locked' ' git branch --set-upstream-to locked && >.git/config.lock && test_must_fail git branch --unset-upstream 2>err && - test_i18ngrep "could not lock config file .git/config: File exists" err + test_i18ngrep "could not lock config file .git/config" err ' test_expect_success 'test --unset-upstream on HEAD' '
In 6b7093064a ("t3200: test for specific errors", 2020-06-15), we learned to grep stderr to ensure that the failing `git branch` invocations fail for the right reason. In two of these tests, we grep for "File exists", expecting the string to show up there since config.c calls `error_errno()`, which ends up including `strerror(errno)` in the error message. But as we saw in 4605a73073 ("t1091: don't grep for `strerror()` string", 2020-03-08), there exists at least one implementation where `strerror()` yields a slightly different string than the one we're grepping for. In particular, these tests fail on the NonStop platform. Similar to 4605a73073, grep for the beginning of the string instead to avoid relying on `strerror()` behavior. Reported-by: Randall S. Becker <rsbecker@nexbridge.com> Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- Hi Randall, Does this fix the test for you? Martin t/t3200-branch.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)