Thanks for the feedback, Tomi! On Sat 2019-05-25 22:41:58 +0300, Tomi Ollila wrote: > On Sat, May 25 2019, Daniel Kahn Gillmor wrote: > >> In certain cases of test suite failure, the summary report was not >> being printed. In particular, any failure on the parallel test suite, >> and any aborted test in the serialized test suite would end up hiding >> the summary. >> >> It's better to always show the summary where we can (while preserving >> the return code). > > This is an improvement, but it looks like it is possible > aggregate-results.sh prints everything good (like all 3 tests passed) > since it may be that there are only good logs in test-results/. > > That might, in some cases, be confusing (exit value of notmuch-test > is not always visible).
Yes, i share your concern about confusion, and i tried to stave off that
confusion by making sure that there was something printed in each of the
fallthrough cases. But you're right that i didn't do that well
enough. v2 of my patch also moves the printed warning to after
aggregate-results.sh output, and prefixes it with "ERROR:", so that it's
more obvious that this is why we are terminating non-zero.
> One option could be giving $RES to aggregate-results.sh and if it is
> not zero then that could print different text -- just that it requires
> a bit more coding...
I'm not convinced that's a worthwhile tradeoff. i don't want to add an
option parser to aggregate-results.sh, and it uses all of "$@" already.
So in the simplest implementation we'd be passing some sort of
environment variable to it -- but that variable wouldn't do anything
other than print what notmuch-test is already printing. certainly if we
just passed $RES we'd have even less information about the context of
where/how the meta-failure happened. My v2 patch does a better job of
this, hopefully.
So if someone has strong feelings about this and wants to update
aggregate-results.sh instead, feel free to do so; i wouldn't block it,
but hopefully the current proposal is acceptable.
thanks for the review,
--dkg
signature.asc
Description: PGP signature
_______________________________________________ notmuch mailing list [email protected] https://notmuchmail.org/mailman/listinfo/notmuch
