Bug#926222: unblock: pbgenomicconsensus/2.3.2-2
Control: tags -1 moreinfo
Andreas Tille:
> Package: release.debian.org
> Severity: normal
> User: release.debian.org@packages.debian.org
> Usertags: unblock
>
> Please unblock package pbgenomicconsensus
>
>
> I admit the changes are more complex than I would have prefered them to
> be. The usage of the upstream Makefile inside the tests would have
> required to re-do the workaround for missing data for some tests as they
> were just done in debian/rules. Instead I moved the code from d/rules
> right into upstream makefile to keep the test consistent at package
> build time and in autopkgtest.
>
> Unfortunately further patches of the test suite are necessary to deal
> with several warnings bloating the output.
>
> Kind regards, Andreas.
>
>
> unblock pbgenomicconsensus/2.3.2-2
>
> [...]
>
Hi,
A possible reoccurring issue is that the changes to the Makefile
involves long shell sequences with ";" (i.e. "Ignore errors") without
use of "set -e". As an example is this:
> +--- a/Makefile
> ++++ b/Makefile
> +@@ -8,7 +8,12 @@ tests: unit-tests basic-tests
> +
> + unit-tests:
> + # Unit tests
> +- py.test --junit-xml=nosetests.xml tests/unit
> ++ # ignore tests requiring https://github.com/PacificBiosciences/PacBioTestData which is not packaged
> ++ TMPDIR=$$(mktemp -d /tmp/test_ignore_XXXXXX) ; \
> ++ mv tests/unit/test_tool_contract.py $${TMPDIR} ; \
> ++ py.test --junit-xml=nosetests.xml tests/unit ; \
> ++ mv $${TMPDIR}/* tests/unit ; \
> ++ rmdir $${TMPDIR}
> +
> + # Note: We need at least cram/0.7 for '--xunit-file'
> + # Note: The cram tests often need h5py.
AFAICT, if the py.test command fails the target might still succeed as
long as the final "rmdir" succeeds due to "; without set -e". Obviously,
it is not going to be any easier to correctly clean up *in case of* errors.
Similar issue appears here:
> +--- a/tests/cram/extra/plurality-fluidigm.t
> ++++ b/tests/cram/extra/plurality-fluidigm.t
> +@@ -7,7 +7,8 @@ Some tests of a "fluidigm amplicons" dat
> +
> + Set the QV threshold to 10.
> +
> +- $ variantCaller --algorithm=plurality -r $REFERENCE -q 10 -o variants.gff -o consensus.csv -o consensus.fastq $INPUT
> ++ $ variantCaller --algorithm=plurality -r $REFERENCE -q 10 -o variants.gff -o consensus.csv -o consensus.fastq $INPUT 2>&1 | tee | grep -v H5pyDeprecationWarning
> ++ [1]
> +
> + There are two true SNVs (and one diploid SNV that we miss right now).
> +
(ASSUMPTION: It is a command and run under "normal shell rules" and the
test actually checks the exit code rather than the out).
If the above assumption holds, this patch will hide the exit code of
variantCaller and replace it by the exit code of grep (a normal pipeline
*without* the "set -o pipefail"-bashism will have the exit code of the
last command in the pipeline).
If it is just code examples, then you can (mostly) disregard this remark.
Final remark:
> Test-Command:
> - cp -r Makefile tests $AUTOPKGTEST_TMP
> - && cd $AUTOPKGTEST_TMP
> - && make tests
> + make unit-tests || true # due to warnings caused by python-pbcore some non-zero value is returned despite all tests are passing - thus adding '|| true'
> Depends:
> @,
> python-nose,
> python-cram,
> make,
> + poa
> Restrictions: allow-stderr
If the autopktest cannot "fail" (both stderr and exit code is ignored),
then it should either not be run or at least be tagged as "superficial"
as a(n unconditional) "Success" is not really saying anything about
whether it worked or exploded in fire and flames.
Thanks,
~Niels
Reply to: