make check still brittle with doctests

I was wondering if this is still active, but I think it is. The makefile has

check-nodocs: extensions
 set -e
 # Generate a stream for PQM to watch.
 -$(RM) -f selftest.log
 $(PYTHON) -Werror -O ./bzr selftest --subunit $(tests) | tee selftest.log
 # An empty log file should catch errors in the $(PYTHON)
 # command above (the '|' swallow any errors since 'make'
 # sees the 'tee' exit code for the whole line
 if [ ! -s selftest.log ] ; then exit 1 ; fi
 # Check that there were no errors reported.
 subunit-stats < selftest.log

There are a couple of problems with this:

* The shell will ignore non-zero return codes from everything but the last command in a pipeline. (You can tell zsh to do otherwise, but I don't think there's any portable way.) So anything that causes bzr to abort won't be seen.
* The subunit file having something in it is no guarantee that all the tests ran to completion.

If this bites us again I would be strongly inclined to just take out subunit until we have a way to run it that's inherently safe.

This can still bite us but switching to tarmac should avoid the problem (by getting away from sh and avoiding the 'return code lost in pipe' issue).

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/2/2011 1:35 AM, Vincent Ladeuil wrote:
> This can still bite us but switching to tarmac should avoid the problem
> (by getting away from sh and avoiding the 'return code lost in pipe'
> issue).
>

I don't think tarmac gets us away from "running sh", since it just runs
a command to run tests as well.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk1J9FQACgkQJdeBCYSNAANiswCfXQu3XPDJPIVvFG1uHcJm+0bS
zY4An0A+5fi0LKyOUlkUlb7KWYft7MLw
=7mof
-----END PGP SIGNATURE-----

> I don't think tarmac gets us away from "running sh", since it just runs a command to run tests as well.

I was considering that tarmac may allow us to run python code instead of sh and better control the intermediate steps, sorry for the confusion

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/3/2011 1:26 AM, Vincent Ladeuil wrote:
>> I don't think tarmac gets us away from "running sh", since it just
> runs a command to run tests as well.
>
> I was considering that tarmac may allow us to run python code instead of
> sh and better control the intermediate steps, sorry for the confusion
>

We could do that today with:

build:
  $(PYTHON) script.py

If you think things will actually be better that way, we can look into it.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk1LHVgACgkQJdeBCYSNAAMhGQCgt157lMriHX8GGpst9il8bJGK
m54AoIvho6r4OA0hcWZzT1K6PhyJsayL
=SqU7
-----END PGP SIGNATURE-----

On 4 February 2011 08:25, John A Meinel <email address hidden> wrote:

> We could do that today with:
>
> build:
>  $(PYTHON) script.py
>
> If you think things will actually be better that way, we can look into
> it.

It seems a bit silly to write a Python script to wrap a script we
ourselves control.

What it needs is, I think:
 * selftest itself should fail if any tests fail, even with --subunit

and actually that's it, isn't it? The only reason we need to write to
a file as well as to stdout is so we can check if there are any
errors, but this is in fact a bit inherently dangerous.

Perhaps you could just set pipefile in Makefile ?

'pipefile' (or was it pipefail ?) was mentioned in the past but didn't make it, I can't remember precisely why...

Portability issues shouldn't really matter since the target is the pqm instance only...

On 4 February 2011 10:34, Robert Collins <email address hidden> wrote:
> Perhaps you could just set pipefile in Makefile ?

s//pipefail

This would make it bash-specific, and we're probably generally not
running bash, and it would need to be done on the same line, since
Make runs each line in a separate process. But otherwise yes.

The thing is if we make it exit nonzero on failure, we should not need
to have a pipeline at all, as far as I can see. Just emit subunit to
stdout and pqm should be able to use that both for the progress
display and to send to the user if it fails.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/4/2011 1:27 AM, Martin Pool wrote:
> On 4 February 2011 10:34, Robert Collins <email address hidden> wrote:
>> Perhaps you could just set pipefile in Makefile ?
>
> s//pipefail
>
> This would make it bash-specific, and we're probably generally not
> running bash, and it would need to be done on the same line, since
> Make runs each line in a separate process. But otherwise yes.
>
> The thing is if we make it exit nonzero on failure, we should not need
> to have a pipeline at all, as far as I can see. Just emit subunit to
> stdout and pqm should be able to use that both for the progress
> display and to send to the user if it fails.
>

As I recall, wasn't the original issue that the subunit-foo that we were
running at the end of the pipe didn't exit non-zero for failing tests vs
corrupted stream?

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk1MFxsACgkQJdeBCYSNAANr+gCbBDDhwaw4Ea3XRm9iZ03abzfm
XOAAnRRe/XNvB+OKznws/uxw9MI5jqQi
=uF45
-----END PGP SIGNATURE-----

On 5 February 2011 02:11, John Arbash Meinel <email address hidden> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 2/4/2011 1:27 AM, Martin Pool wrote:
>> On 4 February 2011 10:34, Robert Collins <email address hidden> wrote:
>>> Perhaps you could just set pipefile in Makefile ?
>>
>> s//pipefail
>>
>> This would make it bash-specific, and we're probably generally not
>> running bash, and it would need to be done on the same line, since
>> Make runs each line in a separate process.  But otherwise yes.
>>
>> The thing is if we make it exit nonzero on failure, we should not need
>> to have a pipeline at all, as far as I can see.  Just emit subunit to
>> stdout and pqm should be able to use that both for the progress
>> display and to send to the user if it fails.
>>
>
> As I recall, wasn't the original issue that the subunit-foo that we were
> running at the end of the pipe didn't exit non-zero for failing tests vs
> corrupted stream?

That's right: a file with no errors in it isn't detected as being a
test failure. But it might have no errors because for whatever reason
we didn't end up running the whole test suite. That's why I think
it's inherently more risky to ignore the error code from bzr, and for
bzr to exit 0 even though it knows some tests failed.