Hide skipped tests in CI

[iluuu1994]

Seeing all skipped tests named is generally not super useful but it hides the failed tests in a wall of text. We care more about the number of skipped tests which are still being displayed. The one useful things about seeing skipped tests is seeing that the CI is still doing something. However stuck CIs is normally not a problem we have.

If we want this output for older branches too I'll merge it into PHP-8.0 instead.

/cc @TimWolla

[TimWolla]

This is much better, thanks.

[iluuu1994]

I wonder if we should also hide the XFAIL, or at least the diff of it. I highly doubt anybody is actually looking at those anyway.

[cmb69]

Showing details about XFAILs is indeed not particularly useful, but I'd really like to have the option to see all these details in a junit.xml file. For AppVeyor this is done like (relevant are only the first and the last line):

php-src/appveyor/test_task.bat

Lines 136 to 145 in 8cf9c2f

	set TEST_PHP_JUNIT=c:\junit.out.xml
	cd "%APPVEYOR_BUILD_FOLDER%"
	nmake test TESTS="%OPCACHE_OPTS% -q --offline --show-diff --show-slow 1000 --set-timeout 120 --temp-source c:\tests_tmp --temp-target c:\tests_tmp --bless %PARALLEL%"
	set EXIT_CODE=%errorlevel%
	taskkill /f /im snmpd.exe
	appveyor PushArtifact %TEST_PHP_JUNIT%

If there is the need, one can download that file, and investigate.

[iluuu1994]

@cmb69 Ok, I'll give that a try tomorrow 🙂

[TimWolla]

Other than that remark this still LGTM.

[iluuu1994]

We can likely also ditch WARN since they're shown in the summary and don't have a diff.

[cmb69]

We can likely also ditch WARN since they're shown in the summary and don't have a diff.

ACK. Are there any other reasons for WARN except for XFAIL tests passing?

[iluuu1994]

@cmb69 There's this:

php-src/run-tests.php

Lines 2130 to 2132 in d002a0d

	} elseif (!strncasecmp('warn', $output, 4) && preg_match('/^warn\s+(.+)/i', $output, $m)) {
	$warn = true; /* only if there is a reason */
	$info = " (warn: $m[1])";

Where you could do <?php die('warn Foo'); ?> in SKIPIF but it seems to be borked as the test is still shown as passing. The only other one is (warn: XFAIL section but test passes) (and the same for XLEAK).

[iluuu1994]

Great, github.job is not fully qualified (it's the name without the matrix part)... So we'll have to pass it via params.

[cmb69]

Where you could do <?php die('warn Foo'); ?> in SKIPIF but it seems to be borked as the test is still shown as passing.

Ah, thank you! Then probably this should either be fixed, or be removed. And it should be documented also (unless removed). Also the "info" case.

[iluuu1994]

@cmb69 Ah, it only happens if the test fails. If so you'll get a WARN&FAIL. But I'm not sure what the purpose is here, since you can just use XFAIL. IMO this should be removed. I couldn't find any usages of this.

[cmb69]

@iluuu1994, I'll have a closer look ASAP.

[iluuu1994]

@cmb69 Btw XLEAK is also undocumented and unused.

@TimWolla It's very unfortunate that all the matrix information has to be repeated in testArtifacts since github.job only contains the unqualified name (e.g. LINUX_X64). Do you know a way around this?

[TimWolla]

It's very unfortunate that all the matrix information has to be repeated in testArtifacts since github.job only contains the unqualified name (e.g. LINUX_X64). Do you know a way around this?

@iluuu1994

I'm afraid not and a Google search just revealed other users encountering the same limitation. You might consider using the https://support.github.com/ panel to raise this as a suggestion. My past experiences with the GitHub support went very well. While they won't promise anything of course, they usually responded that they would add a +1 for the feature on their internal roadmap.

That said: You might be able to hack around with that by using toJSON() or join(). The resulting name likely will get very ugly, though.

[iluuu1994]

@TimWolla Bummer. Yeah I've tried ${{ join(matrix.*, ' - ') }} as suggested on StackOverflow but the result was very non-descriptive (e.g. true - false). Thank you for taking a look! I guess I'll just leave it like this then for now. Is the rest ok in your opinion?

[iluuu1994]

@cmb69 Since the junit file is quite big (over 2mb per test run) do you think it's enough to store it on failure?

Edit: Or we limit the time it's stored, probably the better option.

[cmb69]

Or we limit the time it's stored, probably the better option.

See https://github.com/actions/upload-artifact#retention-period; TL;DR: it's already limited to 90 days. 5 days or so should be sufficient, though.

[TimWolla]

Thank you for taking a look!

One more idea (that I didn't test): You might be able to generate the name in the first step of the build, using ::set-output and then reference this in both the job name and the artifact input. However I do not know, if the job name may reference outputs of the steps. I know that later step names may reference the outputs of earlier steps. I'm using this here:

https://github.com/TimWolla/action-install-haproxy/blob/3029c05b3af1dbc59e627dcc0ab975eea4b6e832/.github/workflows/test.yml#L43

Is the rest ok in your opinion?

I didn't see any obvious issues.

[TimWolla]

However I do not know, if the job name may reference outputs of the steps

Apparently not: https://github.com/TimWolla/action-install-haproxy/actions/runs/2762375632

[iluuu1994]

One more idea

Yes I thought about this approach too.

if the job name may reference outputs

From what I read it can't. And as I type you just confirmed it 🙂

[TimWolla]

@iluuu1994 The above might or might not be possible now with: https://github.blog/changelog/2022-09-26-github-actions-dynamic-names-for-workflow-runs/