Merge lp:~vila/bzr/626667-check-no-docs-swallow-errors into lp:bzr

[Vincent Ladeuil]

prevent 'make check' from succeeding when no tests are run

This fix a amazingly long standing bug (recently found though #626667)
in our Makefile that could lead pqm to accept bogus submissions.

Namely, errors in 'make check' were swallowed.

I went for a pretty conservative fix that requires that selftest produces
a non-empty result.

[John A Meinel (jameinel)]

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

On 8/30/2010 5:18 AM, Vincent Ladeuil wrote:
> Vincent Ladeuil has proposed merging lp:~vila/bzr/626667-check-no-docs-swallow-errors into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
>
> This fix a amazingly long standing bug (recently found though #626667)
> in our Makefile that could lead pqm to accept bogus submissions.
>
> Namely, errors in 'make check' were swallowed.
>
> I went for a pretty conservative fix that requires that selftest produces
> a non-empty result.

Can we just to 'set -e' and have any non-zero exit code stop the script?

Otherwise:
  merge: approve

John
=:->

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

iEYEARECAAYFAkx7vZgACgkQJdeBCYSNAAOq/gCfXYf9QbceQEgR3Csw84FXfy35
wPYAoJVt0y76sh49I7fkaS9aNmr2HyF3
=61gT
-----END PGP SIGNATURE-----

review: Approve

[John A Meinel (jameinel)]

sent to pqm by email

[Martin Pool (mbp)]

It's a bit strange and dangerous that pqm needs us to save a file,
even though it also consumes the output itself, but this is probably
the sensible fix anyhow.

[John A Meinel (jameinel)]

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

On 8/30/2010 9:31 PM, Martin Pool wrote:
> It's a bit strange and dangerous that pqm needs us to save a file,
> even though it also consumes the output itself, but this is probably
> the sensible fix anyhow.

right, it sends the stdout back to the user (if the command fails) but
we need to run something to analyze that output to determine if there
were any failures.

Arguably we should just have a pipe filter that would do this, but that
would require writing it :).

I believe it might be a way to get better filtered output (stripping the
log from all the 'successful' tests.

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

iEYEARECAAYFAkx8e2kACgkQJdeBCYSNAAOB0QCgwIHZFnS+VgG0VVgjux0k6U8/
NeQAnA/gZTgrf0gmos6qc9XplzrJV2l5
=79do
-----END PGP SIGNATURE-----

=== modified file 'Makefile'
--- Makefile	2010-04-06 06:59:03 +0000
+++ Makefile	2010-08-30 10:18:42 +0000
@@ -40,7 +40,12 @@
 
 check-nodocs: extensions
 	# Generate a stream for PQM to watch.
+	-$(RM) 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
 

=== modified file 'NEWS'
--- NEWS	2010-08-30 07:47:38 +0000
+++ NEWS	2010-08-30 10:18:42 +0000
@@ -128,6 +128,9 @@
 * `PathNotChild` should not give a traceback.
   (Martin Pool, #98735)
 
+* ``PQM`` will no longer ignore syntax errors in submissions.
+  (Vincent Ladeuil, #626667)
+
 * Prevent ``CHKMap.apply_delta`` from generating non-canonical CHK maps,
   which can result in "missing referenced chk root keys" errors when
   fetching from repositories with affected revisions.