mikepenz/action-junit-report

Source of file and line attributes

alandefreitas opened this issue · 5 comments

I had a problem identifying the proper file and line number in my annotations so I went through the code and found this:

const pos = await resolveFileAndLine(
testcase._attributes.file || (testsuite._attributes !== undefined ? testsuite._attributes.file : null),
testcase._attributes.line || (testsuite._attributes !== undefined ? testsuite._attributes.line : null),
testcase._attributes.classname ? testcase._attributes.classname : testcase._attributes.name,
stackTrace
)

Then I had a quick look at the spec and it seems like the file and line attributes should come from the testcase.failure._attributes rather than the testcase._attributes, which is why my annotations were wrong.

@alandefreitas thank you for the report.

there seems to be a lot of different variants of test output. and normally (at least based on the reports I have) the information is on the testcase instead. See the example here: https://github.com/mikepenz/action-junit-report/blob/main/test_results/junit-web-test/expected.xml#L19-L22

Which junit export do you use? in which language? Do you have example test result output?

Yes. That makes sense. I'm sure you wouldn't have used the testcase element unless it worked for most use cases or at least some relevant use case. I've been using Catch2 to generate reports. The report files are OK but it's definitely not the most common generator. (In any case, I wrote a small tool to adjust the reports so it's all fine.)

So I quickly did some independent research and found out the failure element is the one that canonically allows the file and line elements. That might be wrong at some level, but it did make a lot of sense to me since each failure can be at a different line. In fact, if I recall correctly, the code that generates the annotations is iterating the failures and not the testcases.

The current code already tries to get it from the testcase and then the testsuite. I guess the testcase is the most common place for the filename then. On the other hand, if the failure element has a filename, this is even more specific than the testcase and probably what the report means. In that case, I think I would try the attribute from failure first, then fallback to the testcase, then the testsuite, because it generalizes well to what the code already does.

@alandefreitas thank you very much for the additional details.

It surely makes sense if it is part of some standards, and doesn't seem as it would affect current functionality either. As such it would be great to get an example export (can be greatly simplified) to add to the test-cases, as I want to make sure such cases won't break with future updates

Yes. That makes sense.

Here's a very small example generated with Catch2: https://gist.github.com/alandefreitas/2c92b53acadd4af446103985c79df47b

Thank you