js/ci-github-workflow-markup output regression (was: [PATCH] ls-tree: test for the regression in 9c4d58ff2c3)

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jun 3, 2022, 09:54 UTC
Message-ID: <220603.86fskmxd43.gmgdl@evledraar.gmail.com>
In-Reply-To: <nycvar.QRO.7.76.6.2206021703110.349@tvgsbejvaqbjf.bet>

On Thu, Jun 02 2022, Johannes Schindelin wrote:

Show 6 quoted lines

> On Tue, 31 May 2022, Ævar Arnfjörð Bjarmason wrote:
> [...]
>> +test_ls_tree_format_mode_output () {
>> +	local opts=$1 &&
>
> This line does not quote `$1`, but...

Well spotted, thanks, I don't know how I missed that, will fix it & re-roll, but per changed $subject...

Show 10 quoted lines

> [...]
> ... this line passes a first argument that contains spaces. Hence the
> tests fail in CI:
> https://github.com/git/git/runs/6703333447?check_suite_focus=true
>
> Further, since this failure is outside of any `test_expect_success` or
> `test_expect_failure`, the error message about this is not even included
> in the weblogs (but of course it is included in the full logs that are
> included in the build artifacts). For the record, here is the error
> message:

...this part of it though seems like a pretty bad regression in your merged-to-next js/ci-github-workflow-markup topic, which just happens to be unearthed by this CI failure.

On top of master this patch will get this CI failure:
https://github.com/avar/git/runs/6675920732?check_suite_focus=true#step:5:598;
Ending in ("[...]" edit is mine):
	
	 ok 35 - 'ls-tree --object-only -r' output (via subdir)
	+ git rev-parse --short HEAD:.gitmodules
	[...]
	+ HEAD_short_dir_sub_file=a150abd
	
	ok 36 - setup: HEAD_short_* variables
	t3105-ls-tree-output.sh: 20: local: --abbrev: bad variable name
	FATAL: Unexpected exit with code 2

So here we see that we got to test 36, and then got this error. All of
which is after the step we focused on to begin with would have shown:
	
	 Test Summary Report
	-------------------
	t3105-ls-tree-output.sh                          (Wstat: 256 Tests: 36 Failed: 0)
	  Non-zero exit status: 1
	  Parse errors: No plan found in TAP output

I.e. telling us we had a TAP parse error, so something like this was going on.

But now we'll instead get ("=>" edit is mine, it didn't copy/paste):

	=> Run tests
	=== Failed test: t3105-ls-tree-output ===
	The full logs are in the artifacts attached to this run.
	Error: Process completed with exit code 1.

Where expanding the "=>" in "Run tests" will get us the "Test summary report" from before, but now we have no "ci/print-test-failures.sh" step to emit the output we got from the test, and we just get a cryptic error about t3105 having failed *somewhere*.

As noted in the breadcrumb trail leading from "[2]" in [1] this is a fundamental limitation of the approach you picked for --github-workflow-markup. You're tasking the test-lib.sh to emit well-formed output, but as shown here we can die prematurely on "eval" failures.

But this does look easy to "solve" with a quicker fix, just bringing back the "ci/print-test-failures.sh" step so you can at least expand it, and not have to go to the "summary" and download the *.zip of the log itself. As that shows we still have the raw log there, it just didn't make it to the new GitHub Markdown formatting mechanism.

1. https://lore.kernel.org/git/220324.8635j7nyvw.gmgdl@evledraar.gmail.com/