From: Phillip Wood Date: Mon, 07 Mar 2022 17:12:02 GMT Subject: Re: [PATCH 0/9] ci: make Git's GitHub workflow output much more helpful Message-ID: In-Reply-To: Hi Dscho On 07/03/2022 16:07, Johannes Schindelin wrote: > Hi Phillip, > > On Wed, 2 Mar 2022, Phillip Wood wrote: > >> 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. > >> 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? Sure, I mentioned it in case it was easy to fix but I don't think it should stop these patches moving forward Best Wishes Phillip > Thanks, > Dscho