[SUREFIRE-2277] Fix bug in RunResult serialisation/deserialisation to (from) failsafe-summary.xml #790
Conversation
...safe-plugin/src/main/java/org/apache/maven/plugin/failsafe/util/FailsafeSummaryXmlUtils.java
Show resolved
Hide resolved
| @@ -64,6 +69,49 @@ public void testSkipped() throws Exception { | |||
| writeReadCheck(new RunResult(3, 2, 1, 0, null, true)); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testFlakes() throws Exception { | |||
There was a problem hiding this comment.
can you be more specific about the exception thrown?
| } | ||
|
|
||
| @Test | ||
| public void testLegacyDeserialization() throws Exception { |
There was a problem hiding this comment.
can you be more specific about the exception thrown?
There was a problem hiding this comment.
Hey, yeah I could but it's a test method, I'm not convinced it's necessary to be specific here. I was following the existing code style in this particular Test class. I think in this case it may be better to be consistent with the original style.
There was a problem hiding this comment.
I 100% disagree. Consistency with imperfect code is not a virtue. New code should follow best practices, even when the old code does not.
maven-failsafe-plugin/src/test/java/org/apache/maven/plugin/failsafe/RunResultTest.java
Show resolved
Hide resolved
...safe-plugin/src/main/java/org/apache/maven/plugin/failsafe/util/FailsafeSummaryXmlUtils.java
Show resolved
Hide resolved
maven-surefire-plugin/src/site/resources/xsd/failsafe-summary.xsd
Outdated
Show resolved
Hide resolved
surefire-api/src/main/java/org/apache/maven/surefire/api/suite/RunResult.java
Show resolved
Hide resolved
|
I have corrected the line ending of all XSD files, please rebase and we will run ITs. |
…the equals method for RunResult where it did not account for the flakes field.
bingx1
force-pushed
the
fix-runresult-serialisation-bug
branch
from
ac0efac
to
9d2afac
Compare
…mary.xsd for the flakes element. Add a comment noting that flakes will be expected in the failsafe-summary in the future. Incorporate the value of flakes into RunResult.hashCode() to be consistent with the new implementation of toEquals
bingx1
force-pushed
the
fix-runresult-serialisation-bug
branch
from
9d2afac
to
73641e7
Compare
Done |
This PR fixes the bug described in https://issues.apache.org/jira/browse/SUREFIRE-2277
Problem
There is a bug in
RunResult.testAppendSerialization.This test creates a
RunResultobject in-memory, serialises it, writes it to disk and then again deserialises the same file into aRunResultin-memory. I have run the test with the debugger and found that the final in-memoryRunResultobject is not the same as the initial one.It shouldn't be passing on master, but is due to a bug in the
RunResult.equalsmethod which is used in an assertion in the test. The source of the bug is that the value ofRunResult.flakesisn't preserved during serialisation to and fromfailsafe-summary.xml. The bug is slipping through and the test passes because theRunResult.equalsmethod doesn't consider theRunResult.flakesfield.Fix
I have modified the
failsafe-summary.xsdto include an optional<flakes>element, which will allowRunResult.flakesto be persisted in thefailsafe-summary.xmlduring serialisation. I have also changed the serialisation and deserialisation methods forRunResultto account forflakes.I have also added a test,
RunResultTest.testLegacyDeserializationfor backwards compatibility. It tests that deserialising a legacyfailsafe-summary.xmlstill works. The behaviour is that when theflakesXML element is not present in thefailsafe-summary.xml, the in-memoryRunResultwill haveRunResult.flakesset to 0 after deserialisation.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-XXXwith 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 installto 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.
Our company, Atlassian, has a corporate CLA signed, and we added our team to it.
Contributors: Alex Courtis, Bing Xu and Hubert Grzeskowiak