[PATCH] test: report summary even when aborting
Daniel Kahn Gillmor
dkg at fifthhorseman.net
Sun May 26 08:06:16 PDT 2019
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
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,
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 227 bytes
Desc: not available
More information about the notmuch