Re: [PATCH 0/9] ci: make Git's GitHub workflow output much more helpful

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 7, 2022, 16:07 UTC
Message-ID: <nycvar.QRO.7.76.6.2203071657180.11118@tvgsbejvaqbjf.bet>
In-Reply-To: <30dbc8fb-a1db-05bc-3dcb-070e11cf4715@gmail.com>

Hi Phillip,

On Wed, 2 Mar 2022, Phillip Wood wrote:

Show 20 quoted lines

> On 25/02/2022 14:10, Johannes Schindelin wrote:
>
> > On Wed, 23 Feb 2022, Phillip Wood wrote:
> >
> > > With the first link above the initial page load is faster but to get
> > > to the output of the failing test case I have click on "Run
> > > ci/print_test_failures.sh" then wait for that to load and then
> > > search for "not ok" to actually get to the information I'm after.
> >
> > And that's only because you are familiar with what you have to do.
> >
> > Any new contributor would be stuck with the information presented on the
> > initial load, without any indication that more information _is_ available,
> > just hidden away in the next step's log (which is marked as "succeeding",
> > therefore misleading the inclined reader into thinking that this cannot
> > potentially contain any information pertinent to the _failure_ that needs
> > to be investigated).
>
> Yes it took we a while to realize how to get to the test output when I first
> started looking at the CI results.

Thank you for saying that. Since nobody else said it as clearly as you, I really started to doubt myself here.

Show 20 quoted lines

> One thing I forgot to mention was that when you expand a failing test it shows
> the test script twice before the output e.g.
>
> Error: failed: t7527.35 Matrix[uc:false][fsm:true] enable fsmonitor
> failure: t7527.35 Matrix[uc:false][fsm:true] enable fsmonitor
>   				git config core.fsmonitor true &&
>   				git fsmonitor--daemon start &&
>   				git update-index --fsmonitor
>
>   expecting success of 7527.35 'Matrix[uc:false][fsm:true] enable fsmonitor':
>   				git config core.fsmonitor true &&
>   				git fsmonitor--daemon start &&
>   				git update-index --fsmonitor
>
>  ++ git config core.fsmonitor true
>  ++ git fsmonitor--daemon start
>  ...
>
> I don't know how easy it would be to fix that so that we only show "expecting
> success of ..." without the test being printed first

It's not _super_ easy: right now, the patch series does not touch the code that prints the latter message. In fact, that latter message is generated _before_ the test fails, and redirected via `tee` into `GIT_TEST_TEE_OUTPUT_FILE`. Then, once the verdict is clear that this test case failed, the first message is printed (the one that is colored in the output via `::error::`), and the output from the `GIT_TEST_TEE_OUTPUT_FILE` file is pasted, starting at the offset marking the start of the test case.

The easiest workaround would probably to add a flag that suppresses the header `expecting success` in case we're running with the `--github-workflow-markup` option.

I'll put it on my ever-growing TODO list, but maybe in the interest of heeding the mantra "the perfect is the enemy of the good", this can be done on top of this series rather than blocking it?

Thanks, Dscho