-
Notifications
You must be signed in to change notification settings - Fork 542
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SUREFIRE-1345] Support flakyFailure and flakyError in TestSuiteXmlParser #700
Conversation
...report-parser/src/main/java/org/apache/maven/plugins/surefire/report/TestSuiteXmlParser.java
Outdated
Show resolved
Hide resolved
Yet another question: How do you want to utilize the data? In the Maven Surefire Report Plugin or somewhere else? |
The plan is to consume them in the Quarkus GitHub Bot for this feature: https://github.com/quarkusio/quarkus-github-bot#workflow-run-report . It analyses all the build reports from our CI workflow and post a comment in the pull request. We wanted to try to rerun tests but we need the feedback to contain the flakiness information. |
I see, let's focus on the issue and handle std streams separately, if you don't mind. |
...ire-report-parser/src/main/java/org/apache/maven/plugins/surefire/report/ReportTestCase.java
Outdated
Show resolved
Hide resolved
...ire-report-parser/src/main/java/org/apache/maven/plugins/surefire/report/ReportTestCase.java
Show resolved
Hide resolved
...report-parser/src/main/java/org/apache/maven/plugins/surefire/report/TestSuiteXmlParser.java
Outdated
Show resolved
Hide resolved
@kriegaex Any objections before I go over to your PR? |
@michael-o, thanks for considering that the original version of this PR collided with #692. But it is good to get confirmation from a third party that my idea of capturing stdOut and stdErr is useful to other users, too. No objections. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged.
Thanks for the review and for merging! |
Hey there,
In the Quarkus project, we make good use of the Surefire reports to provide a full report of a pull request run.
We will soon enable the ability to retry tests but we need to make sure we can easily identify flaky tests thus why we included the ability to get flakyFailures and flakyErrors from the report.
This is related to https://issues.apache.org/jira/browse/SUREFIRE-1345 but does not entirely implement it as I haven't implemented the reruns. I could be convinced to if you think it's crucial to get this PR in but, in our case, we don't see it as useful.
Tests are included to make sure the data is properly extracted.
I think I already signed the Apache CLA a long time ago but can fill it again if needed.
Happy to iterate on this to get it merged soon.
Thanks for having a look!
Following this checklist to help us incorporate your
contribution quickly and easily:
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
[SUREFIRE-XXX] - Fixes bug in ApproximateQuantiles
,where you replace
SUREFIRE-XXX
with the appropriate JIRA issue. Best practiceis to use the JIRA issue title in the pull request title and in the first line of the
commit message.
mvn clean install
to make sure basic checks pass. A more thorough check willbe performed on your pull request automatically.
mvn -Prun-its clean install
).If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.
I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
In any other case, please file an Apache Individual Contributor License Agreement.