Re: [PATCH v4 13/15] perf test: Add -j/--junit option for JUnit XML test reports

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Crashed or abnormally exiting tests are reported as successful in the JUnit XML report.
--

commit 97a61309aed955d3a130fc7d4fb58abfdb0d12f1
Author: Ian Rogers <irogers@google.com>

perf test: Add -j/--junit option for JUnit XML test reports

This commit adds a -j/--junit command line option to generate standard
JUnit XML test reports. The XML report captures individual test suite
and subtest execution latency, alongside failure logs and skip reasons.

> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
[ ... ]
> @@ -418,6 +450,34 @@ static int print_test_result(struct test_suite *t, int curr_suite, int curr_test
[ ... ]
> +	if (junit_filename && result != TEST_RUNNING) {
> +		const char *classname = t->desc;
> +		const char *testname = test_description(t, curr_test_case);
> +		char *escaped_err = xml_escape(err_output);
> +		char *escaped_class = xml_escape(classname);
> +		char *escaped_test = xml_escape(testname);
> +
> +		strbuf_addf(&junit_xml_buf,
> +			    "    <testcase classname=\"%s\" name=\"%s\" time=\"%.2f\">\n",
> +			    escaped_class, escaped_test, elapsed);
> +		if (result == TEST_FAIL) {

[Severity: Medium]
Will tests that crash or time out be incorrectly reported as successful
in the XML?

When a test terminates abnormally (e.g., due to a signal like SIGSEGV or
a timeout), execution monitors like finish_command() can return other negative
error codes (such as -11) instead of exactly TEST_FAIL (-1).

Since this strict equality check bypasses the <failure> block for those
other negative codes, it seems an empty <testcase> tag will be generated,
which standard JUnit CI parsers interpret as a passing test.

Should this check account for other negative return values, perhaps using
if (result != TEST_OK && result != TEST_SKIP), to ensure crashes are 
properly recorded as failures?

> +			strbuf_addf(&junit_xml_buf,
> +				    "      <failure message=\"FAILED\">\n%s\n      </failure>\n",
> +				    escaped_err);
> +		} else if (result == TEST_SKIP) {
> +			const char *reason = skip_reason(t, curr_test_case);
> +			char *escaped_reason = xml_escape(reason ? reason : "Skip");
> +
> +			strbuf_addf(&junit_xml_buf, "      <skipped message=\"%s\"/>\n",
> +				    escaped_reason);
> +			free(escaped_reason);
> +		}
> +		strbuf_addstr(&junit_xml_buf, "    </testcase>\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260531082235.940833-1-irogers@google.com?part=13