Annotate pytest failures with GitHub Actions #1707
Conversation
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
|
Code Climate has analyzed commit 03ae57e and detected 2 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
huonw
force-pushed
the
feature/junit-annotations
branch
from
6efa2fc
to
1ad1fda
Compare
|
Only the changes to |
| invalid = [] | ||
| for testcase in root.findall(".//testcase"): | ||
| for child in testcase: | ||
| if child.tag in ("error", "failure"): |
There was a problem hiding this comment.
does this skip annotating warnings?
There was a problem hiding this comment.
Yeah it doesn't. Unfortunately, I don't think warnings are explicitly captured as a separate thing associated within each test. For instance, the summary of the tags and their attributes and the counts ([...]) in a random Pytest file I have lying around is:
<testsuites> [1]
<testsuite> [1] errors, failures, hostname, name, skipped, tests, time, timestamp
<testcase> [890] classname, name, time
<skipped> [12] message, type
<system-out> [138]
<failure> [18] message
<system-err> [31]
<error> [4] message
Also, at the moment we end up with a lot of warnings from tests, so we'd have a lot of noise here.
The duplicates are either different parameterisations of the test (which are vaguely useful to repeat?), or the seeming GitHub bugs where a single annotation is shown more than once. |
This reverts commit 58e783e.
This replaces the JUnit test summary annotation used on Buildkite with annotations on individual tests about the failures. In particular, a custom script walks through the JUnit XML file to find any failures or errors, and adds a summary of the failures to each test via the
::error file=...::...command https://help.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-error-messageFor reference, a JUnit XML file emitted by pytest looks something like:
Note that
<testcase>errors and failures are indicated by<failure message=...>...</failure>and<error message=...>...</error>child elements.The JUnit files emitted by pytest don't include the test location at all, so we have to estimate them, by searching the stack trace (text contents of the
<failure>/<error>element) for the first match for a<file>:<line>:pattern. This should generally be StellarGraph code, and will usually be the call orassertwithin the test function that had the error.This is an optional extra, and it only triggers when tests fail, so this heuristic/approximate approach are okay. The code detects failures and highlight them, so that things can be fixed in future.
For instance, for https://github.com/stellargraph/stellargraph/pull/1707/checks?check_run_id=798011967 :
The annotations also are included for files that aren't modified. (Unfortunately, it seems there's more bugs here, because only a small number of the total number of failure annotations are actually listed.) For instance, if a change to code breaks an existing test:
(It's unclear to me why the cards are listed twice, even for a single OS/Python-version pair. I think it's a GitHub bug, because it's definitely only in the logs once: https://github.com/stellargraph/stellargraph/pull/1707/checks?check_run_id=798011967#step:8:31 )
Finally, as warned by codeclimate, the
xml.etree.ElementTreemodule is apparently unsafe, for untrusted XML data. This doesn't concern us particularly, because (a) the JUnit XML files are being generated by us and so are trusted, and (b) the problems of an attack aren't new (if someone is able to attack this part of the CI, they can already run arbitrary code in it).See: #1687