[BUG] test_must_fail: does not correctly detect failures - Was Git 2.16.0-rc2 Test Summary on NonStop

[BUG] test_must_fail: does not correctly detect failures - Was Git 2.16.0-rc2 Test Summary on NonStop

[Randall S. Becker]

On January 13, 2018 2:31 PM, I wrote:
> On January 13, 2018 1:08 PM, I wrote:
> > Here’s where things are. This is probably the best git release so far
> (ever).
> > After applying a4cdf02, I had 6 total breakages. 3 existing, 3 new.
> > Many reduced. The test took about 24 hours to run on platform, which
> > is about 2 hours shorter than 2.13.5.
> >
> > t1308-config-set.sh (2 already discussed and expecting a fix, both
> > appear
> to
> > be issues in the test script, not code) t1404-update-ref-errors.sh #
> > 52 – reported but not discussed:
> >    not ok 52 - delete fails cleanly if packed-refs file is locked.
> >      The lock detection worked, but the test assumed the detection
> > would occur in a different spot.
> > t9001-send-email.sh (2 have existed for 2 years. 1 is new. We have not
> used
> > send-email on platform to this point).
> >    not ok 31 - reject long lines
> >      This is a new fail since 2.8.5
> >   not ok 106 - sendemail.transferencoding=7bit fails on 8bit data
> >      Still to be investigated. This may be a tooling issue on Platform.
> >   not ok 107 - --transfer-encoding overrides
> > sendemail.transferEncoding
> >      Still to be investigated. This may be a tooling issue on Platform.
> 
> I missed one:
> not ok 134 - --dump-aliases must be used alone #
> #               test_must_fail git send-email --dump-aliases
> --to=janice@example
> .com -1 refs/heads/accounting

Running the tests in debug, I found that they all (1308, 1404, 9001) use
test_must_fail, and hit similar situations:

expecting success:
        test_must_fail git send-email --dump-aliases --to=janice@example.com
-1 refs/heads/accounting
--dump-aliases incompatible with other options
test_must_fail: died by signal 34: git send-email --dump-aliases
--to=janice@example.com -1 refs/heads/accounting
not ok 134 - --dump-aliases must be used alone
#
#               test_must_fail git send-email --dump-aliases
--to=janice@example.com -1 refs/heads/accounting
#

It is looking like git is doing what it is supposed to be doing, but the
test scripts are not detecting failures properly. The test_must_fail routine
is interestingly used in all of the above test cases that are failing. The
actual exit_code reported by git was 162, (a.k.a. signal 34 - which is not
thrown on the platform. The max signal is 31 (SIGABEND). test_must_fail has
a weird combination of some errors pass and others don't, but I can't
correlate the intent of its use in these tests particularly with no
acceptable signals passed in. Adding a return 1 if 162 caused other tests to
fail as well, so that's not the fix.

Any clues?
Thanks,
Randall

RE: [BUG] test_must_fail: does not correctly detect failures - Was Git 2.16.0-rc2 Test Summary on NonStop

[Randall S. Becker]

On January 13, 2018 3:17 PM, I wrote:
> On January 13, 2018 2:31 PM, I wrote:
> > On January 13, 2018 1:08 PM, I wrote:
> > > Here’s where things are. This is probably the best git release so
> > > far
> > (ever).
> > > After applying a4cdf02, I had 6 total breakages. 3 existing, 3 new.
> > > Many reduced. The test took about 24 hours to run on platform, which
> > > is about 2 hours shorter than 2.13.5.
> > >
> > > t1308-config-set.sh (2 already discussed and expecting a fix, both
> > > appear
> > to
> > > be issues in the test script, not code) t1404-update-ref-errors.sh #
> > > 52 – reported but not discussed:
> > >    not ok 52 - delete fails cleanly if packed-refs file is locked.
> > >      The lock detection worked, but the test assumed the detection
> > > would occur in a different spot.
> > > t9001-send-email.sh (2 have existed for 2 years. 1 is new. We have
> > > not
> > used
> > > send-email on platform to this point).
> > >    not ok 31 - reject long lines
> > >      This is a new fail since 2.8.5
> > >   not ok 106 - sendemail.transferencoding=7bit fails on 8bit data
> > >      Still to be investigated. This may be a tooling issue on
Platform.
> > >   not ok 107 - --transfer-encoding overrides
> > > sendemail.transferEncoding
> > >      Still to be investigated. This may be a tooling issue on
Platform.
> >
> > I missed one:
> > not ok 134 - --dump-aliases must be used alone #
> > #               test_must_fail git send-email --dump-aliases
> > --to=janice@example
> > .com -1 refs/heads/accounting
> 
> Running the tests in debug, I found that they all (1308, 1404, 9001) use
> test_must_fail, and hit similar situations:
> 
> expecting success:
>         test_must_fail git send-email --dump-aliases
--to=janice@example.com
> -1 refs/heads/accounting
> --dump-aliases incompatible with other options
> test_must_fail: died by signal 34: git send-email --dump-aliases --
> to=janice@example.com -1 refs/heads/accounting not ok 134 - --dump-
> aliases must be used alone #
> #               test_must_fail git send-email --dump-aliases
> --to=janice@example.com -1 refs/heads/accounting #
> 
> It is looking like git is doing what it is supposed to be doing, but the
test
> scripts are not detecting failures properly. The test_must_fail routine is
> interestingly used in all of the above test cases that are failing. The
actual
> exit_code reported by git was 162, (a.k.a. signal 34 - which is not thrown
on
> the platform. The max signal is 31 (SIGABEND). test_must_fail has a weird
> combination of some errors pass and others don't, but I can't correlate
the
> intent of its use in these tests particularly with no acceptable signals
passed
> in. Adding a return 1 if 162 caused other tests to fail as well, so that's
not the
> fix.

Follow-up: This looks like the completion code from perl on NonStop is not
the same as expected by git in the case of failures. I need to debug this to
get more details to the team. We have had completion issues before relating
to interpretation problems between perl, bash, and git in this platform, so
I'm assuming this to be likely again but need to track down the specifics.
Can anyone point me to where the detection is within git or the execv?

Thanks,
Randall
 

Re: [BUG] test_must_fail: does not correctly detect failures - Was Git 2.16.0-rc2 Test Summary on NonStop

[Johannes Sixt]

Am 14.01.2018 um 17:50 schrieb Randall S. Becker:
> Follow-up: This looks like the completion code from perl on NonStop is not
> the same as expected by git in the case of failures. I need to debug this to
> get more details to the team. We have had completion issues before relating
> to interpretation problems between perl, bash, and git in this platform, so
> I'm assuming this to be likely again but need to track down the specifics.
> Can anyone point me to where the detection is within git or the execv?

Perhaps you are looking for wait_or_whine() in run-command.c? But this 
function cannot do anything if perl alread exits with an exotic code (> 
128 even though no signal was received).

-- Hannes

RE: [BUG] test_must_fail: does not correctly detect failures - Was Git 2.16.0-rc2 Test Summary on NonStop

[Randall S. Becker]

> -----Original Message-----
> From: Johannes Sixt [mailto:j6t@kdbg.org]
> Sent: On January 14, 2018 1:41 PM wrote:
> Am 14.01.2018 um 17:50 schrieb Randall S. Becker:
> > Follow-up: This looks like the completion code from perl on NonStop is
> > not the same as expected by git in the case of failures. I need to
> > debug this to get more details to the team. We have had completion
> > issues before relating to interpretation problems between perl, bash,
> > and git in this platform, so I'm assuming this to be likely again but need to
> track down the specifics.
> > Can anyone point me to where the detection is within git or the execv?
> 
> Perhaps you are looking for wait_or_whine() in run-command.c? But this
> function cannot do anything if perl alread exits with an exotic code (>
> 128 even though no signal was received).

The exotic error code coming back from perl is 162. I can muck with it, if there was a value more useful to git.
*  553          } else if (WIFEXITED(status)) {
*  554                  code = WEXITSTATUS(status);
(eInspect 3,887):p code
$4 = 162

The perl code uses die to terminate with a non-specific non-zero error code. What it seems is that there is an assumed value that the git tests depend on, but perl.org describes the following:

"die raises an exception. Inside an eval the error message is stuffed into $@ and the eval is terminated with the undefined value. If the exception is outside of all enclosing evals, then the uncaught exception prints LIST to STDERR and exits with a non-zero value. If you need to exit the process with a specific exit code, see exit."

So it seems that we might need to either not use die or change the tests not to care about the exit code for specific tests involving perl. Suggestions?

Sincerely,
Randall

-- Brief whoami:
  NonStop developer since approximately NonStop(211288444200000000)
  UNIX developer since approximately 421664400
-- In my real life, I talk too much.

RE: [BUG] test_must_fail: does not correctly detect failures - Was Git 2.16.0-rc2 Test Summary on NonStop

[Randall S. Becker]

On January 14, 2018 4:33 PM, I wrote:
> On January 14, 2018 1:41 PM Johannes Sixt wrote:
> > Sent: > > Am 14.01.2018 um 17:50 schrieb Randall S. Becker:
> > > Follow-up: This looks like the completion code from perl on NonStop
> > > is not the same as expected by git in the case of failures. I need
> > > to debug this to get more details to the team. We have had
> > > completion issues before relating to interpretation problems between
> > > perl, bash, and git in this platform, so I'm assuming this to be
> > > likely again but need to
> > track down the specifics.
> > > Can anyone point me to where the detection is within git or the execv?
> >
> > Perhaps you are looking for wait_or_whine() in run-command.c? But this
> > function cannot do anything if perl alread exits with an exotic code
> > (>
> > 128 even though no signal was received).
> 
> The exotic error code coming back from perl is 162. I can muck with it, if
> there was a value more useful to git.
> *  553          } else if (WIFEXITED(status)) {
> *  554                  code = WEXITSTATUS(status);
> (eInspect 3,887):p code
> $4 = 162
> 
> The perl code uses die to terminate with a non-specific non-zero error code.
> What it seems is that there is an assumed value that the git tests depend on,
> but perl.org describes the following:
> 
> "die raises an exception. Inside an eval the error message is stuffed into $@
> and the eval is terminated with the undefined value. If the exception is
> outside of all enclosing evals, then the uncaught exception prints LIST to
> STDERR and exits with a non-zero value. If you need to exit the process with
> a specific exit code, see exit."
> 
> So it seems that we might need to either not use die or change the tests not
> to care about the exit code for specific tests involving perl. Suggestions?

Sadly, while changing the funky 162 completion code to 255 works fine for t9001, it causes a bunch of other tests to fail (parts of 1308 and most of 1404). I can't tall whether this is test suite or code related but I'm caught in the middle. Going to the platform developers may eventually get the fix for t9001 (fixing perl), but otherwise, I'm not sure why changing 162 to 255 causes 1308 and 1404 to blow so badly. In any event, I'm putting this away for a few days due to $DAYJOB.

Cheers,
Randall

Re: [BUG] test_must_fail: does not correctly detect failures - Was Git 2.16.0-rc2 Test Summary on NonStop

[Johannes Sixt]

Am 15.01.2018 um 03:37 schrieb Randall S. Becker:
> On January 14, 2018 4:33 PM, I wrote:
>> The exotic error code coming back from perl is 162. I can muck with it, if
>> there was a value more useful to git.
>> *  553          } else if (WIFEXITED(status)) {
>> *  554                  code = WEXITSTATUS(status);
>> (eInspect 3,887):p code
>> $4 = 162
>>
>> The perl code uses die to terminate with a non-specific non-zero error code.
>> What it seems is that there is an assumed value that the git tests depend on,
>> but perl.org describes the following:
>>
>> "die raises an exception. Inside an eval the error message is stuffed into $@
>> and the eval is terminated with the undefined value. If the exception is
>> outside of all enclosing evals, then the uncaught exception prints LIST to
>> STDERR and exits with a non-zero value. If you need to exit the process with
>> a specific exit code, see exit."

I take "die exits with non-zero" as a piece of information for the 
*users* so that they can write "if perl foo.pl; then something; fi" in 
shell scripts. I do *not* interpret it as leeway for implementers of 
perl to choose any random value as exit code. Choosing 162 just to be 
funky would be short-sighted. [I'm saying all this without knowing how 
perl specifies 'die' beyond the paragraph you cited. Perhaps there's 
more about 'die' that justifies exit code 162.] I'd say that the perl 
port is broken.

>>
>> So it seems that we might need to either not use die or change the tests not
>> to care about the exit code for specific tests involving perl. Suggestions?
> 
> Sadly, while changing the funky 162 completion code to 255 works
> fine for t9001, it causes a bunch of other tests to fail (parts of
> 1308 and most of 1404). I can't tall whether this is test suite or
> code related but I'm caught in the middle. Going to the platform
> developers may eventually get the fix for t9001 (fixing perl), but
> otherwise, I'm not sure why changing 162 to 255 causes 1308 and 1404
> to blow so badly. In any event, I'm putting this away for a few days
> due to $DAYJOB.

Why do you not choose 1? He, on my Linux box perl -e die exits with 255. 
So...

-- Hannes

RE: [BUG] test_must_fail: does not correctly detect failures - Was Git 2.16.0-rc2 Test Summary on NonStop

[Randall S. Becker]

On January 15, 2018 2:06 AM, Johannes Sixt wrote:
> Am 15.01.2018 um 03:37 schrieb Randall S. Becker:
> > On January 14, 2018 4:33 PM, I wrote:
> >> The exotic error code coming back from perl is 162. I can muck with
> >> it, if there was a value more useful to git.
> >> *  553          } else if (WIFEXITED(status)) {
> >> *  554                  code = WEXITSTATUS(status);
> >> (eInspect 3,887):p code
> >> $4 = 162
> >>
> >> The perl code uses die to terminate with a non-specific non-zero error
> code.
> >> What it seems is that there is an assumed value that the git tests
> >> depend on, but perl.org describes the following:
> >>
> >> "die raises an exception. Inside an eval the error message is stuffed
> >> into $@ and the eval is terminated with the undefined value. If the
> >> exception is outside of all enclosing evals, then the uncaught
> >> exception prints LIST to STDERR and exits with a non-zero value. If
> >> you need to exit the process with a specific exit code, see exit."
> 
> I take "die exits with non-zero" as a piece of information for the
> *users* so that they can write "if perl foo.pl; then something; fi" in shell
> scripts. I do *not* interpret it as leeway for implementers of perl to choose
> any random value as exit code. Choosing 162 just to be funky would be
> short-sighted. [I'm saying all this without knowing how perl specifies 'die'
> beyond the paragraph you cited. Perhaps there's more about 'die' that
> justifies exit code 162.] I'd say that the perl port is broken.

I agree that 162 is wrong. Its interpretation is 128+signal, which clearly does not happen in this case. On the platform, if the perl script is via stdin, 162 or 169 are returned. If via file (perl file.pl), 255 comes back. The port has issues. I have an opened a bug report with the platform developers. Usual non-Open Source timeframes to fix apply. ☹

> >>
> >> So it seems that we might need to either not use die or change the
> >> tests not to care about the exit code for specific tests involving perl.
> Suggestions?
> >
> > Sadly, while changing the funky 162 completion code to 255 works fine
> > for t9001, it causes a bunch of other tests to fail (parts of
> > 1308 and most of 1404). I can't tall whether this is test suite or
> > code related but I'm caught in the middle. Going to the platform
> > developers may eventually get the fix for t9001 (fixing perl), but
> > otherwise, I'm not sure why changing 162 to 255 causes 1308 and 1404
> > to blow so badly. In any event, I'm putting this away for a few days
> > due to $DAYJOB.
> 
> Why do you not choose 1? He, on my Linux box perl -e die exits with 255.
> So...

Choosing 1 is an option. I can try that, but I'm not sure it's going to help the test situation more than choosing 255 to replace 162. 

Cheers,
Randall

Re: [BUG] test_must_fail: does not correctly detect failures - Was Git 2.16.0-rc2 Test Summary on NonStop

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1503 bytes --]

On Mon, Jan 15, 2018 at 09:25:37AM -0500, Randall S. Becker wrote:
> On January 15, 2018 2:06 AM, Johannes Sixt wrote:
> > I take "die exits with non-zero" as a piece of information for the
> > *users* so that they can write "if perl foo.pl; then something; fi" in shell
> > scripts. I do *not* interpret it as leeway for implementers of perl to choose
> > any random value as exit code. Choosing 162 just to be funky would be
> > short-sighted. [I'm saying all this without knowing how perl specifies 'die'
> > beyond the paragraph you cited. Perhaps there's more about 'die' that
> > justifies exit code 162.] I'd say that the perl port is broken.
> 
> I agree that 162 is wrong. Its interpretation is 128+signal, which
> clearly does not happen in this case. On the platform, if the perl
> script is via stdin, 162 or 169 are returned. If via file (perl
> file.pl), 255 comes back. The port has issues. I have an opened a bug
> report with the platform developers. Usual non-Open Source timeframes
> to fix apply. ☹

I believe the standard behavior for Perl with die is the following:

exit $! if $!;
exit $? >> 8 if $? >> 8;
exit 255; # otherwise

Is there an errno value on your port that matches 162?  Maybe EBADF?

On Linux, I get the following:

genre ok % printf die | perl -; echo $?
Died at - line 1.
9
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

RE: [BUG] test_must_fail: does not correctly detect failures - Was Git 2.16.0-rc2 Test Summary on NonStop

[Randall S. Becker]

On January 15, 2018 10:01 PM, brian m. Carlson wrote:
> On Mon, Jan 15, 2018 at 09:25:37AM -0500, Randall S. Becker wrote:
> > On January 15, 2018 2:06 AM, Johannes Sixt wrote:
> > > I take "die exits with non-zero" as a piece of information for the
> > > *users* so that they can write "if perl foo.pl; then something; fi"
> > > in shell scripts. I do *not* interpret it as leeway for implementers
> > > of perl to choose any random value as exit code. Choosing 162 just
> > > to be funky would be short-sighted. [I'm saying all this without knowing
> how perl specifies 'die'
> > > beyond the paragraph you cited. Perhaps there's more about 'die'
> > > that justifies exit code 162.] I'd say that the perl port is broken.
> >
> > I agree that 162 is wrong. Its interpretation is 128+signal, which
> > clearly does not happen in this case. On the platform, if the perl
> > script is via stdin, 162 or 169 are returned. If via file (perl
> > file.pl), 255 comes back. The port has issues. I have an opened a bug
> > report with the platform developers. Usual non-Open Source timeframes
> > to fix apply. ☹
> 
> I believe the standard behavior for Perl with die is the following:
> 
> exit $! if $!;
> exit $? >> 8 if $? >> 8;
> exit 255; # otherwise
> 
> Is there an errno value on your port that matches 162?  Maybe EBADF?
> 
> On Linux, I get the following:
> 
> genre ok % printf die | perl -; echo $?
> Died at - line 1.
> 9

Nah. Worse. Assume a perl script that is simply 'die "hello world"'. If it's in a file, perl reports 255. If from stdin, perl reports 162/169. Doh. That's supposed to be 128+signum, and max sig is 31 (SIGABEND) on the platform. The difficulty at this point is that if I fix wait_or_whine to map either code to 255 or 1, then many more other tests fail, so I'm stuck with either 6 reasonably acceptable breaks or about 60 unacceptable ones, based on assumptions in test_must_fail or other fail detections in the git suite. I'd rather not mess with git code if the test breaks themselves are explainable.

Sign.

Randall

-- Brief whoami:
  NonStop developer since approximately NonStop(211288444200000000)
  UNIX developer since approximately 421664400
-- In my real life, I talk too much.