Where to report the filename and line number in JUnit?

[carlescufi]

The JUnit spec doesn't mention file name and line number where an error occurred (eg. as attributes of failure).
Since this action seems to display annotations inline with the PR files changed (i.e. the diff), how can one provide the file name and line number where an error was detected (for example by a linter program).

[EnricoMi]

You are right, file name and line number is not part of any of the unofficial specs.

However, some test tools use file and line in <testcase> for this (e.g. pytest):

publish-unit-test-result-action/python/test/files/junit-xml/pytest/junit.fail.xml

Lines 5 to 6 in 43acb3d

	<testcase classname="test.test_spark.SparkTests" file="test/test_spark.py" line="1412"
	name="test_check_shape_compatibility" time="6.435"></testcase>

Note: this is not the line where an error (test failure) occurred but where the failing test is defined.

[carlescufi]

@EnricoMi thanks very much for the reply. Unfortunately, as you mention, this is defined in the testcase element and not in the failure one, which makes it hard to implement the usecase I have, which is to have a single test case with multiple failures, sort of like this:

<?xml version="1.0" encoding="utf-8"?>
<testsuites tests="1" failures="5" errors="0" time="0.0">
        <testsuite name="Compliance" tests="1" errors="0" failures="5" skipped="0" time="0">
                <testcase name="Checkpatch" classname="Guidelines">
                        <failure message="subsys/bluetooth/host/hci_core.c:269  do not use C99 // comments" type="error">
C99_COMMENTS: do not use C99 // comments
File:subsys/bluetooth/host/hci_core.c
Line:269</failure>
                        <failure message="subsys/bluetooth/host/hci_core.c:270  that open brace { should be on the previous line" type="error">
OPEN_BRACE: that open brace { should be on the previous line
File:subsys/bluetooth/host/hci_core.c
Line:270</failure>
                        <failure message="subsys/bluetooth/host/hci_core.c:275  space prohibited between function name and open parenthesis '('" type="warning">
SPACING: space prohibited between function name and open parenthesis '('
File:subsys/bluetooth/host/hci_core.c
Line:275</failure>
                </testcase>
        </testsuite>
</testsuites>

This is a usecase that seems to be supported by the JUnit/xunit spec.

See pytest-dev/pytest#5891

which then links to:

https://github.com/jenkinsci/xunit-plugin/blob/ce58c5eb1db91d0d181935889a20cc42d2697b03/src/main/resources/org/jenkinsci/plugins/xunit/types/model/xsd/junit-10.xsd#L85-L105

or

https://github.com/pytest-dev/pytest/blob/f54ec30a6d7969eba6a2003b4409d33828cd406d/testing/example_scripts/junit-10.xsd#L88-L98

Which seems to allow multiple failures in a single testcase.

[carlescufi]

@EnricoMi note that my goal is to reuse your action for one of the CI checks that is currently annotating manually, unlike others in the open source project I work on, which already use your action

[EnricoMi]

I think multiple failures should work with the action. When the messages are different, it should show multiple failures for the same testcase.

Have you tried above file with the action?

[carlescufi]

I think multiple failures should work with the action. When the messages are different, it should show multiple failures for the same testcase.

That's excellent to hear, but the problem is the filename/line number in each failure (hence the original question in this thread). I would need a way to convey to your action that the failure includes this "metadata" so it can place the GitHub annotations correctly as per GitHub's format. Each failure would match to an error/warning. I am currently doing this sort of manually in the action that handles this today:

I store the failures in a "special" class that derives from junitparser.Failure:
https://github.com/zephyrproject-rtos/zephyr/blob/c67666ae1bdfcb570457ab528b8d7de34fcaf29e/scripts/ci/check_compliance.py#L161-L169

and then print those as GitHub annotations:
https://github.com/zephyrproject-rtos/zephyr/blob/c67666ae1bdfcb570457ab528b8d7de34fcaf29e/scripts/ci/check_compliance.py#L1114-L1121

Have you tried above file with the action?

No, I haven't yet, but it's great to know it should work, thanks!

[EnricoMi]

Nothing stops you from adding attributes to the <failure> tags:

<testcase name="Checkpatch" classname="Guidelines">
  <failure file="subsys/bluetooth/host/hci_core.c" line="269" message="do not use C99 // comments" type="error">
C99_COMMENTS: do not use C99 // comments
File:subsys/bluetooth/host/hci_core.c
Line:269</failure>
</testcase>

And then extend this section of junit.py:

publish-unit-test-result-action/python/publish/junit.py

Lines 219 to 240 in 71ca27c

	# junit allows for multiple results for a single test case (e.g. success and failure for the same test)
	# we pick the most severe result, which could still be multiple results, so we aggregate those, which is messy
	cases = [
	UnitTestCase(
	result_file=result_file,
	test_file=case._elem.get('file'),
	line=int_opt(case._elem.get('line')),
	class_name=case.classname,
	test_name=case.name,
	result=get_result(results),
	message=get_message(results),
	content=get_content(results),
	stdout=case.system_out,
	stderr=case.system_err,
	time=case.time * time_factor if case.time is not None else case.time
	)
	for result_file, suite in suites
	for case in get_cases(suite)
	if case.classname is not None or case.name is not None
	# junit allows for multiple results in one test case, pick the most severe results
	for results in [get_results(case.result, case.status)]
	]

It looks like this action "supports" multiple failures per testcase by picking the most severe, and from those the first only. So your use case (multiple results per test case) is not fully supported.

What you want here is to create one UnitTestCase per result in case.result. Then, get_result, get_message, and get_content are not needed anymore:

        result=get_result(results),
        message=get_message(results),
        content=get_content(results),

Finally,

        test_file=case._elem.get('file'),
        line=int_opt(case._elem.get('line')),

has to use the file and line from result if it exists, and fall back to case if not.

Except for the last step, this looks required to fully support the spec.

But I am hesitating to do the last bit as that is a bespoke JUnit XML spec.

[EnricoMi]

@carlescufi have you continued to work on this?

[carlescufi]

@carlescufi have you continued to work on this?

Not yet @EnricoMi sorry. I will try to find some time for this during the summer and otherwise let you know.