Re: Surefire Report Bug with Assumption after at least 1 Rerun Test

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Surefire Report Bug with Assumption after at least 1 Rerun Test

Tibor Digana
Hi Ryan,

I found this issue already reported in JIRA.
https://issues.apache.org/jira/browse/SUREFIRE-1556

There is no fix because we have not received any reproducible project.

IMO this section should not exist in the XML:

< message="This is a bug">

      <system-out><![CDATA[...]]></system-out>

      <system-err><![CDATA[...]]></system-err>

    </>

It looks like the message is XML attribute used as an element.

I checked the StatelessXmlReporter  right now but i could not find any
logical reason for this issue.
I have really no idea what code and how injected this section into the XML
file.

Here is the method which is responsible for writing the system-out and
system-err but I still do not see the root cause in the latest revision.
https://github.com/apache/maven-surefire/blob/master/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java#L479

Maybe you have older version where is this bug.
So the only thing we can do is to ask you to run the test with
3.0.0-SNAPSHOT.
And then somebody has to debug the code. ;-(

Pls write the test result in JIRA.
If the bug is still present with the snapshot version then I would ask you
to help us, dig in to the code and debug it.

Cheers
Tibor


On Mon, Nov 11, 2019 at 6:33 PM Ryan Thomas <[hidden email]> wrote:

> Hi Tibor,
>
>
>
> My Company BrainpOP is using Maven 3.6.2 with Junit4.7+ (Junit
> 4.13-beta-3, Junit 4.13-rc-1) and has recently come into an issue while
> trying to optimize flaky test run times.
>
>
>
> We are implementing assumptions to have Tests that are in a bad state to
> be skipped so that they do not add a ton of run time to the Test Suite in a
> case where the next Selenium run command fails with a long
> TimeOutException. However, we have come across some scenarios where
> sometime the Test might fail do to a failed Assertion, and on its next Run
> due to rerunFailingTestCount  criteria being met, an Assumption will fail
> resulting in further runs of the test being skipped. When this occurs, The
> surefire-report-plugin has been failing at parsing the generated XML.
>
>
>
> In analyzing the XML output for the scenario, I found the the XML
> generated was invalid, and that the message tied to where the Assumption
> failed in malformed. In this case, the Tag name (e.g. failure, skipped,
> rerunFailure, etc…) is missing and only the message attribute  and type
> remains. Looking at the surefire-test-report-3.0.xsd Schema, I can see that
> this is invalid.
>
>
>
> Here is the sample Test run in Junit through Maven
>
>
>
> @SuppressWarnings("deprecation")
>
> @Test
>
> public void testSomething3() {
>
>                 System.out.println("the end");
>
>                 i++;
>
>                 if(i == 3)
>
>                                 Assume.assumeTrue("This is a bug", false);
>
>                 else
>
>                                 Assert.assertTrue("Run: " + i,false);
>
> }
>
>
>
> Included here is a sample snippet of the malformed tag from the generated
> XML
>
>
>
> <testcase name="testSomething3" classname="com.brainpop.tests.LoggingTest"
> time="13.289">
>
>     <failure message="Run: 1"
> type="java.lang.AssertionError">java.lang.AssertionError: Run: 1
>
>                 at
> com.brainpop.tests.LoggingTest.testSomething3(LoggingTest.java:66)
>
> </failure>
>
>     <system-out><![CDATA[...]]></system-out>
>
>     <system-err><![CDATA[...]]></system-err>
>
>     <rerunFailure message="Run: 2" type="java.lang.AssertionError">
>
>       <stackTrace>java.lang.AssertionError: Run: 2
>
>                 at
> com.brainpop.tests.LoggingTest.testSomething3(LoggingTest.java:66)
>
> </stackTrace>
>
>       <system-out><![CDATA[...]]></system-out>
>
>       <system-err><![CDATA[...]]></system-err>
>
>     </rerunFailure>
>
>     < message="This is a bug">
>
>       <system-out><![CDATA[...]]></system-out>
>
>       <system-err><![CDATA[...]]></system-err>
>
>     </>
>
>   </testcase>
>
>
>
>
>
>
>
> Sent from Mail <https://go.microsoft.com/fwlink/?LinkId=550986> for
> Windows 10
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Surefire Report Bug with Assumption after at least 1 Rerun Test

Ryan Thomas
Hi Tibor,

Were you able to test this scenario with the code snippet I provided in
testSomething3()?
My company has a legitimate test that falls into this scenario but I
provided that snippet as a proof of concept.

What I have found is 2 things:
1. *There is a conflict in the reported type when there are Errors/Failures
before a Skip from an Assumption in
DefaultReporterFactory.getTestResultType()*
It is technically impossible for the code contained to return a skipped
status if there was previously an Error or Failure reported in the repeated
test Runs
https://github.com/apache/maven-surefire/blob/33360045e005b38b16eb2c6d1169f43de927bb3d/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactory.java#L253
Here we see that if there was an Error or a Failure and the
rerunFailingTestsCount> 0, if there was a success after we report flake,
but because the other scenario is an ELSE, it will always in this flow
return seenError? error: failure.  It will not be able to move onto the
outer if/else if/else to return a skipped status.

2. *There is an issue in StatelessXmlReporter when we are then handling
this misreported Skipped state as a Failure*
https://github.com/apache/maven-surefire/blob/master/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java#L250

Here we see that when writing to the XML for all of the subsequent runs
after the first run, we are getting the singleRunEntry.getReportEntryType().
getRerunXmlTag(), which in the case for the ReportType enum, SKIPPED has
its flakyXmlTag and rerunXmlTag defined as "", an empty string.
https://github.com/apache/maven-surefire/blob/7149edc6f24fa8bff06372e24a177e86d4960d8c/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/ReportEntryType.java#L31

This conflict arises because the individual entry IS reported as skipped,
but the getTestResultType check from the list or tries incorrectly reported
it as a Failure. So it goes through the incorrect reporting type for the
XML output, hence why it contains the system-out, and system-err sub
elements

Thank you for taking the time to look into this!

*Ryan Thomas*
*Sr. QA Engineer*
BrainPOP


On Mon, Nov 11, 2019 at 2:04 PM Tibor Digana <[hidden email]> wrote:

> Hi Ryan,
>
> I found this issue already reported in JIRA.
> https://issues.apache.org/jira/browse/SUREFIRE-1556
>
> There is no fix because we have not received any reproducible project.
>
> IMO this section should not exist in the XML:
>
> < message="This is a bug">
>
>       <system-out><![CDATA[...]]></system-out>
>
>       <system-err><![CDATA[...]]></system-err>
>
>     </>
>
> It looks like the message is XML attribute used as an element.
>
> I checked the StatelessXmlReporter  right now but i could not find any
> logical reason for this issue.
> I have really no idea what code and how injected this section into the XML
> file.
>
> Here is the method which is responsible for writing the system-out and
> system-err but I still do not see the root cause in the latest revision.
>
> https://github.com/apache/maven-surefire/blob/master/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java#L479
>
> Maybe you have older version where is this bug.
> So the only thing we can do is to ask you to run the test with
> 3.0.0-SNAPSHOT.
> And then somebody has to debug the code. ;-(
>
> Pls write the test result in JIRA.
> If the bug is still present with the snapshot version then I would ask you
> to help us, dig in to the code and debug it.
>
> Cheers
> Tibor
>
>
> On Mon, Nov 11, 2019 at 6:33 PM Ryan Thomas <[hidden email]> wrote:
>
>> Hi Tibor,
>>
>>
>>
>> My Company BrainpOP is using Maven 3.6.2 with Junit4.7+ (Junit
>> 4.13-beta-3, Junit 4.13-rc-1) and has recently come into an issue while
>> trying to optimize flaky test run times.
>>
>>
>>
>> We are implementing assumptions to have Tests that are in a bad state to
>> be skipped so that they do not add a ton of run time to the Test Suite in a
>> case where the next Selenium run command fails with a long
>> TimeOutException. However, we have come across some scenarios where
>> sometime the Test might fail do to a failed Assertion, and on its next Run
>> due to rerunFailingTestCount  criteria being met, an Assumption will fail
>> resulting in further runs of the test being skipped. When this occurs, The
>> surefire-report-plugin has been failing at parsing the generated XML.
>>
>>
>>
>> In analyzing the XML output for the scenario, I found the the XML
>> generated was invalid, and that the message tied to where the Assumption
>> failed in malformed. In this case, the Tag name (e.g. failure, skipped,
>> rerunFailure, etc…) is missing and only the message attribute  and type
>> remains. Looking at the surefire-test-report-3.0.xsd Schema, I can see that
>> this is invalid.
>>
>>
>>
>> Here is the sample Test run in Junit through Maven
>>
>>
>>
>> @SuppressWarnings("deprecation")
>>
>> @Test
>>
>> public void testSomething3() {
>>
>>                 System.out.println("the end");
>>
>>                 i++;
>>
>>                 if(i == 3)
>>
>>                                 Assume.assumeTrue("This is a bug", false);
>>
>>                 else
>>
>>                                 Assert.assertTrue("Run: " + i,false);
>>
>> }
>>
>>
>>
>> Included here is a sample snippet of the malformed tag from the generated
>> XML
>>
>>
>>
>> <testcase name="testSomething3"
>> classname="com.brainpop.tests.LoggingTest" time="13.289">
>>
>>     <failure message="Run: 1"
>> type="java.lang.AssertionError">java.lang.AssertionError: Run: 1
>>
>>                 at
>> com.brainpop.tests.LoggingTest.testSomething3(LoggingTest.java:66)
>>
>> </failure>
>>
>>     <system-out><![CDATA[...]]></system-out>
>>
>>     <system-err><![CDATA[...]]></system-err>
>>
>>     <rerunFailure message="Run: 2" type="java.lang.AssertionError">
>>
>>       <stackTrace>java.lang.AssertionError: Run: 2
>>
>>                 at
>> com.brainpop.tests.LoggingTest.testSomething3(LoggingTest.java:66)
>>
>> </stackTrace>
>>
>>       <system-out><![CDATA[...]]></system-out>
>>
>>       <system-err><![CDATA[...]]></system-err>
>>
>>     </rerunFailure>
>>
>>     < message="This is a bug">
>>
>>       <system-out><![CDATA[...]]></system-out>
>>
>>       <system-err><![CDATA[...]]></system-err>
>>
>>     </>
>>
>>   </testcase>
>>
>>
>>
>>
>>
>>
>>
>> Sent from Mail <https://go.microsoft.com/fwlink/?LinkId=550986> for
>> Windows 10
>>
>>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: Surefire Report Bug with Assumption after at least 1 Rerun Test

Tibor Digana
yes, i saw the code. So it is reproducible.
but we will release a new version, so no time for me in this bug fix.

IMHO this class should be written again.
We have this ambition in the version 3.0.0-M5.

I can see the principal problems with the design, and you can see the
detailed code level issue.
I think you should try to find a workaround if your company cannot wait and
open a pull request on GitHub.
Another option is to participate on rewriting this class and write missing
tests. Then the class will reach higher quality.

These issues will always exist because the class is stateful and it make
some guesses about the rerun feeture which is bad of course.
So i don't think that the logic is bullet proof.
IMO this class should handle all events and process them without making any
guess of what test was the first and second, and any guess of using the
loops.
We for instance it does not handle events in this class saying that this is
the normal run and this is the rerun, etc.
That's why this class makes some guess about the order and it comes with
errors.
This design will cause the bugs that we have regarding parallel executions
of JUnit5 methods. It is fine in JUnit47+ because there is a trick which
bypassed this problem.
The next issue is the efficiency of the report writer because this class
attempts to rewrite the content.

So, this all is due to legacy code and therefore I did not spend the time
with induvidual bugs because my time can be better utilized if I make one
commit and fix 10 Jira issues at once.
Now the cost makes sense for me only if we rewrite the class completely
because we can close all bugs for all users, and not only the one bug for
only few users.

Maybe you want to paricipate in 3.0.0-M5 development.


T

On Mon, Nov 11, 2019 at 8:20 PM Ryan Thomas <[hidden email]> wrote:

> Hi Tibor,
>
> Were you able to test this scenario with the code snippet I provided in
> testSomething3()?
> My company has a legitimate test that falls into this scenario but I
> provided that snippet as a proof of concept.
>
> What I have found is 2 things:
> 1. *There is a conflict in the reported type when there are
> Errors/Failures before a Skip from an Assumption in
> DefaultReporterFactory.getTestResultType()*
> It is technically impossible for the code contained to return a skipped
> status if there was previously an Error or Failure reported in the repeated
> test Runs
>
> https://github.com/apache/maven-surefire/blob/33360045e005b38b16eb2c6d1169f43de927bb3d/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactory.java#L253
> Here we see that if there was an Error or a Failure and the
> rerunFailingTestsCount> 0, if there was a success after we report flake,
> but because the other scenario is an ELSE, it will always in this flow
> return seenError? error: failure.  It will not be able to move onto the
> outer if/else if/else to return a skipped status.
>
> 2. *There is an issue in StatelessXmlReporter when we are then handling
> this misreported Skipped state as a Failure*
>
> https://github.com/apache/maven-surefire/blob/master/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java#L250
>
> Here we see that when writing to the XML for all of the subsequent runs
> after the first run, we are getting the singleRunEntry.
> getReportEntryType().getRerunXmlTag(), which in the case for the
> ReportType enum, SKIPPED has its flakyXmlTag and rerunXmlTag defined as "",
> an empty string.
>
> https://github.com/apache/maven-surefire/blob/7149edc6f24fa8bff06372e24a177e86d4960d8c/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/ReportEntryType.java#L31
>
> This conflict arises because the individual entry IS reported as skipped,
> but the getTestResultType check from the list or tries incorrectly reported
> it as a Failure. So it goes through the incorrect reporting type for the
> XML output, hence why it contains the system-out, and system-err sub
> elements
>
> Thank you for taking the time to look into this!
>
> *Ryan Thomas*
> *Sr. QA Engineer*
> BrainPOP
>
>
> On Mon, Nov 11, 2019 at 2:04 PM Tibor Digana <[hidden email]>
> wrote:
>
>> Hi Ryan,
>>
>> I found this issue already reported in JIRA.
>> https://issues.apache.org/jira/browse/SUREFIRE-1556
>>
>> There is no fix because we have not received any reproducible project.
>>
>> IMO this section should not exist in the XML:
>>
>> < message="This is a bug">
>>
>>       <system-out><![CDATA[...]]></system-out>
>>
>>       <system-err><![CDATA[...]]></system-err>
>>
>>     </>
>>
>> It looks like the message is XML attribute used as an element.
>>
>> I checked the StatelessXmlReporter  right now but i could not find any
>> logical reason for this issue.
>> I have really no idea what code and how injected this section into the
>> XML file.
>>
>> Here is the method which is responsible for writing the system-out and
>> system-err but I still do not see the root cause in the latest revision.
>>
>> https://github.com/apache/maven-surefire/blob/master/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java#L479
>>
>> Maybe you have older version where is this bug.
>> So the only thing we can do is to ask you to run the test with
>> 3.0.0-SNAPSHOT.
>> And then somebody has to debug the code. ;-(
>>
>> Pls write the test result in JIRA.
>> If the bug is still present with the snapshot version then I would ask
>> you to help us, dig in to the code and debug it.
>>
>> Cheers
>> Tibor
>>
>>
>> On Mon, Nov 11, 2019 at 6:33 PM Ryan Thomas <[hidden email]> wrote:
>>
>>> Hi Tibor,
>>>
>>>
>>>
>>> My Company BrainpOP is using Maven 3.6.2 with Junit4.7+ (Junit
>>> 4.13-beta-3, Junit 4.13-rc-1) and has recently come into an issue while
>>> trying to optimize flaky test run times.
>>>
>>>
>>>
>>> We are implementing assumptions to have Tests that are in a bad state to
>>> be skipped so that they do not add a ton of run time to the Test Suite in a
>>> case where the next Selenium run command fails with a long
>>> TimeOutException. However, we have come across some scenarios where
>>> sometime the Test might fail do to a failed Assertion, and on its next Run
>>> due to rerunFailingTestCount  criteria being met, an Assumption will fail
>>> resulting in further runs of the test being skipped. When this occurs, The
>>> surefire-report-plugin has been failing at parsing the generated XML.
>>>
>>>
>>>
>>> In analyzing the XML output for the scenario, I found the the XML
>>> generated was invalid, and that the message tied to where the Assumption
>>> failed in malformed. In this case, the Tag name (e.g. failure, skipped,
>>> rerunFailure, etc…) is missing and only the message attribute  and type
>>> remains. Looking at the surefire-test-report-3.0.xsd Schema, I can see that
>>> this is invalid.
>>>
>>>
>>>
>>> Here is the sample Test run in Junit through Maven
>>>
>>>
>>>
>>> @SuppressWarnings("deprecation")
>>>
>>> @Test
>>>
>>> public void testSomething3() {
>>>
>>>                 System.out.println("the end");
>>>
>>>                 i++;
>>>
>>>                 if(i == 3)
>>>
>>>                                 Assume.assumeTrue("This is a bug",
>>> false);
>>>
>>>                 else
>>>
>>>                                 Assert.assertTrue("Run: " + i,false);
>>>
>>> }
>>>
>>>
>>>
>>> Included here is a sample snippet of the malformed tag from the
>>> generated XML
>>>
>>>
>>>
>>> <testcase name="testSomething3"
>>> classname="com.brainpop.tests.LoggingTest" time="13.289">
>>>
>>>     <failure message="Run: 1"
>>> type="java.lang.AssertionError">java.lang.AssertionError: Run: 1
>>>
>>>                 at
>>> com.brainpop.tests.LoggingTest.testSomething3(LoggingTest.java:66)
>>>
>>> </failure>
>>>
>>>     <system-out><![CDATA[...]]></system-out>
>>>
>>>     <system-err><![CDATA[...]]></system-err>
>>>
>>>     <rerunFailure message="Run: 2" type="java.lang.AssertionError">
>>>
>>>       <stackTrace>java.lang.AssertionError: Run: 2
>>>
>>>                 at
>>> com.brainpop.tests.LoggingTest.testSomething3(LoggingTest.java:66)
>>>
>>> </stackTrace>
>>>
>>>       <system-out><![CDATA[...]]></system-out>
>>>
>>>       <system-err><![CDATA[...]]></system-err>
>>>
>>>     </rerunFailure>
>>>
>>>     < message="This is a bug">
>>>
>>>       <system-out><![CDATA[...]]></system-out>
>>>
>>>       <system-err><![CDATA[...]]></system-err>
>>>
>>>     </>
>>>
>>>   </testcase>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> Sent from Mail <https://go.microsoft.com/fwlink/?LinkId=550986> for
>>> Windows 10
>>>
>>>
>>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: Surefire Report Bug with Assumption after at least 1 Rerun Test

Ryan Thomas
Hi Tibor,

That would be great. How would I go about doing that?

*Ryan Thomas*
*Sr. QA Engineer*
BrainPOP


On Mon, Nov 11, 2019 at 2:57 PM Tibor Digana <[hidden email]> wrote:

> yes, i saw the code. So it is reproducible.
> but we will release a new version, so no time for me in this bug fix.
>
> IMHO this class should be written again.
> We have this ambition in the version 3.0.0-M5.
>
> I can see the principal problems with the design, and you can see the
> detailed code level issue.
> I think you should try to find a workaround if your company cannot wait
> and open a pull request on GitHub.
> Another option is to participate on rewriting this class and write missing
> tests. Then the class will reach higher quality.
>
> These issues will always exist because the class is stateful and it make
> some guesses about the rerun feeture which is bad of course.
> So i don't think that the logic is bullet proof.
> IMO this class should handle all events and process them without making
> any guess of what test was the first and second, and any guess of using the
> loops.
> We for instance it does not handle events in this class saying that this
> is the normal run and this is the rerun, etc.
> That's why this class makes some guess about the order and it comes with
> errors.
> This design will cause the bugs that we have regarding parallel executions
> of JUnit5 methods. It is fine in JUnit47+ because there is a trick which
> bypassed this problem.
> The next issue is the efficiency of the report writer because this class
> attempts to rewrite the content.
>
> So, this all is due to legacy code and therefore I did not spend the time
> with induvidual bugs because my time can be better utilized if I make one
> commit and fix 10 Jira issues at once.
> Now the cost makes sense for me only if we rewrite the class completely
> because we can close all bugs for all users, and not only the one bug for
> only few users.
>
> Maybe you want to paricipate in 3.0.0-M5 development.
>
>
> T
>
> On Mon, Nov 11, 2019 at 8:20 PM Ryan Thomas <[hidden email]> wrote:
>
>> Hi Tibor,
>>
>> Were you able to test this scenario with the code snippet I provided in
>> testSomething3()?
>> My company has a legitimate test that falls into this scenario but I
>> provided that snippet as a proof of concept.
>>
>> What I have found is 2 things:
>> 1. *There is a conflict in the reported type when there are
>> Errors/Failures before a Skip from an Assumption in
>> DefaultReporterFactory.getTestResultType()*
>> It is technically impossible for the code contained to return a skipped
>> status if there was previously an Error or Failure reported in the repeated
>> test Runs
>>
>> https://github.com/apache/maven-surefire/blob/33360045e005b38b16eb2c6d1169f43de927bb3d/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactory.java#L253
>> Here we see that if there was an Error or a Failure and the
>> rerunFailingTestsCount> 0, if there was a success after we report flake,
>> but because the other scenario is an ELSE, it will always in this flow
>> return seenError? error: failure.  It will not be able to move onto the
>> outer if/else if/else to return a skipped status.
>>
>> 2. *There is an issue in StatelessXmlReporter when we are then handling
>> this misreported Skipped state as a Failure*
>>
>> https://github.com/apache/maven-surefire/blob/master/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java#L250
>>
>> Here we see that when writing to the XML for all of the subsequent runs
>> after the first run, we are getting the singleRunEntry.
>> getReportEntryType().getRerunXmlTag(), which in the case for the
>> ReportType enum, SKIPPED has its flakyXmlTag and rerunXmlTag defined as "",
>> an empty string.
>>
>> https://github.com/apache/maven-surefire/blob/7149edc6f24fa8bff06372e24a177e86d4960d8c/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/ReportEntryType.java#L31
>>
>> This conflict arises because the individual entry IS reported as skipped,
>> but the getTestResultType check from the list or tries incorrectly reported
>> it as a Failure. So it goes through the incorrect reporting type for the
>> XML output, hence why it contains the system-out, and system-err sub
>> elements
>>
>> Thank you for taking the time to look into this!
>>
>> *Ryan Thomas*
>> *Sr. QA Engineer*
>> BrainPOP
>>
>>
>> On Mon, Nov 11, 2019 at 2:04 PM Tibor Digana <[hidden email]>
>> wrote:
>>
>>> Hi Ryan,
>>>
>>> I found this issue already reported in JIRA.
>>> https://issues.apache.org/jira/browse/SUREFIRE-1556
>>>
>>> There is no fix because we have not received any reproducible project.
>>>
>>> IMO this section should not exist in the XML:
>>>
>>> < message="This is a bug">
>>>
>>>       <system-out><![CDATA[...]]></system-out>
>>>
>>>       <system-err><![CDATA[...]]></system-err>
>>>
>>>     </>
>>>
>>> It looks like the message is XML attribute used as an element.
>>>
>>> I checked the StatelessXmlReporter  right now but i could not find any
>>> logical reason for this issue.
>>> I have really no idea what code and how injected this section into the
>>> XML file.
>>>
>>> Here is the method which is responsible for writing the system-out and
>>> system-err but I still do not see the root cause in the latest revision.
>>>
>>> https://github.com/apache/maven-surefire/blob/master/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java#L479
>>>
>>> Maybe you have older version where is this bug.
>>> So the only thing we can do is to ask you to run the test with
>>> 3.0.0-SNAPSHOT.
>>> And then somebody has to debug the code. ;-(
>>>
>>> Pls write the test result in JIRA.
>>> If the bug is still present with the snapshot version then I would ask
>>> you to help us, dig in to the code and debug it.
>>>
>>> Cheers
>>> Tibor
>>>
>>>
>>> On Mon, Nov 11, 2019 at 6:33 PM Ryan Thomas <[hidden email]> wrote:
>>>
>>>> Hi Tibor,
>>>>
>>>>
>>>>
>>>> My Company BrainpOP is using Maven 3.6.2 with Junit4.7+ (Junit
>>>> 4.13-beta-3, Junit 4.13-rc-1) and has recently come into an issue while
>>>> trying to optimize flaky test run times.
>>>>
>>>>
>>>>
>>>> We are implementing assumptions to have Tests that are in a bad state
>>>> to be skipped so that they do not add a ton of run time to the Test Suite
>>>> in a case where the next Selenium run command fails with a long
>>>> TimeOutException. However, we have come across some scenarios where
>>>> sometime the Test might fail do to a failed Assertion, and on its next Run
>>>> due to rerunFailingTestCount  criteria being met, an Assumption will fail
>>>> resulting in further runs of the test being skipped. When this occurs, The
>>>> surefire-report-plugin has been failing at parsing the generated XML.
>>>>
>>>>
>>>>
>>>> In analyzing the XML output for the scenario, I found the the XML
>>>> generated was invalid, and that the message tied to where the Assumption
>>>> failed in malformed. In this case, the Tag name (e.g. failure, skipped,
>>>> rerunFailure, etc…) is missing and only the message attribute  and type
>>>> remains. Looking at the surefire-test-report-3.0.xsd Schema, I can see that
>>>> this is invalid.
>>>>
>>>>
>>>>
>>>> Here is the sample Test run in Junit through Maven
>>>>
>>>>
>>>>
>>>> @SuppressWarnings("deprecation")
>>>>
>>>> @Test
>>>>
>>>> public void testSomething3() {
>>>>
>>>>                 System.out.println("the end");
>>>>
>>>>                 i++;
>>>>
>>>>                 if(i == 3)
>>>>
>>>>                                 Assume.assumeTrue("This is a bug",
>>>> false);
>>>>
>>>>                 else
>>>>
>>>>                                 Assert.assertTrue("Run: " + i,false);
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> Included here is a sample snippet of the malformed tag from the
>>>> generated XML
>>>>
>>>>
>>>>
>>>> <testcase name="testSomething3"
>>>> classname="com.brainpop.tests.LoggingTest" time="13.289">
>>>>
>>>>     <failure message="Run: 1"
>>>> type="java.lang.AssertionError">java.lang.AssertionError: Run: 1
>>>>
>>>>                 at
>>>> com.brainpop.tests.LoggingTest.testSomething3(LoggingTest.java:66)
>>>>
>>>> </failure>
>>>>
>>>>     <system-out><![CDATA[...]]></system-out>
>>>>
>>>>     <system-err><![CDATA[...]]></system-err>
>>>>
>>>>     <rerunFailure message="Run: 2" type="java.lang.AssertionError">
>>>>
>>>>       <stackTrace>java.lang.AssertionError: Run: 2
>>>>
>>>>                 at
>>>> com.brainpop.tests.LoggingTest.testSomething3(LoggingTest.java:66)
>>>>
>>>> </stackTrace>
>>>>
>>>>       <system-out><![CDATA[...]]></system-out>
>>>>
>>>>       <system-err><![CDATA[...]]></system-err>
>>>>
>>>>     </rerunFailure>
>>>>
>>>>     < message="This is a bug">
>>>>
>>>>       <system-out><![CDATA[...]]></system-out>
>>>>
>>>>       <system-err><![CDATA[...]]></system-err>
>>>>
>>>>     </>
>>>>
>>>>   </testcase>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> Sent from Mail <https://go.microsoft.com/fwlink/?LinkId=550986> for
>>>> Windows 10
>>>>
>>>>
>>>>
>>>
Reply | Threaded
Open this post in threaded view
|

Re: Surefire Report Bug with Assumption after at least 1 Rerun Test

Tibor Digana
We can write the concept together, here in ML.
Currently we are working on the release, so give us some days to finish it
first.

Meanwhile open every Provider implementation and notice which events are
fired from the provider via RunListener.
Try to realize the call sequences.
Then see the TestSetRunListener and the StatelessXmlReporter.
The classes in between are not mandatory to see.

You can draw a picture with the events if you want to see a global picture.
Then see the SimpleReportEntry and that's maybe all for the preparation
phase.

Of course see the unit or integration tests and you can write the missing
ones.
Based on the StatelessXmlReporter code you will see which tests are missing.
These are important tests because those will guarantee that we do not
produce more buggy implementation than it is now.
This would be a very big help!

Cheers
Tibor


On Mon, Nov 11, 2019 at 9:24 PM Ryan Thomas <[hidden email]> wrote:

> Hi Tibor,
>
> That would be great. How would I go about doing that?
>
> *Ryan Thomas*
> *Sr. QA Engineer*
> BrainPOP
>
>
> On Mon, Nov 11, 2019 at 2:57 PM Tibor Digana <[hidden email]>
> wrote:
>
>> yes, i saw the code. So it is reproducible.
>> but we will release a new version, so no time for me in this bug fix.
>>
>> IMHO this class should be written again.
>> We have this ambition in the version 3.0.0-M5.
>>
>> I can see the principal problems with the design, and you can see the
>> detailed code level issue.
>> I think you should try to find a workaround if your company cannot wait
>> and open a pull request on GitHub.
>> Another option is to participate on rewriting this class and write
>> missing tests. Then the class will reach higher quality.
>>
>> These issues will always exist because the class is stateful and it make
>> some guesses about the rerun feeture which is bad of course.
>> So i don't think that the logic is bullet proof.
>> IMO this class should handle all events and process them without making
>> any guess of what test was the first and second, and any guess of using the
>> loops.
>> We for instance it does not handle events in this class saying that this
>> is the normal run and this is the rerun, etc.
>> That's why this class makes some guess about the order and it comes with
>> errors.
>> This design will cause the bugs that we have regarding parallel
>> executions of JUnit5 methods. It is fine in JUnit47+ because there is a
>> trick which bypassed this problem.
>> The next issue is the efficiency of the report writer because this class
>> attempts to rewrite the content.
>>
>> So, this all is due to legacy code and therefore I did not spend the time
>> with induvidual bugs because my time can be better utilized if I make one
>> commit and fix 10 Jira issues at once.
>> Now the cost makes sense for me only if we rewrite the class completely
>> because we can close all bugs for all users, and not only the one bug for
>> only few users.
>>
>> Maybe you want to paricipate in 3.0.0-M5 development.
>>
>>
>> T
>>
>> On Mon, Nov 11, 2019 at 8:20 PM Ryan Thomas <[hidden email]> wrote:
>>
>>> Hi Tibor,
>>>
>>> Were you able to test this scenario with the code snippet I provided in
>>> testSomething3()?
>>> My company has a legitimate test that falls into this scenario but I
>>> provided that snippet as a proof of concept.
>>>
>>> What I have found is 2 things:
>>> 1. *There is a conflict in the reported type when there are
>>> Errors/Failures before a Skip from an Assumption in
>>> DefaultReporterFactory.getTestResultType()*
>>> It is technically impossible for the code contained to return a skipped
>>> status if there was previously an Error or Failure reported in the repeated
>>> test Runs
>>>
>>> https://github.com/apache/maven-surefire/blob/33360045e005b38b16eb2c6d1169f43de927bb3d/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/DefaultReporterFactory.java#L253
>>> Here we see that if there was an Error or a Failure and the
>>> rerunFailingTestsCount> 0, if there was a success after we report
>>> flake, but because the other scenario is an ELSE, it will always in this
>>> flow return seenError? error: failure.  It will not be able to move onto
>>> the outer if/else if/else to return a skipped status.
>>>
>>> 2. *There is an issue in StatelessXmlReporter when we are then handling
>>> this misreported Skipped state as a Failure*
>>>
>>> https://github.com/apache/maven-surefire/blob/master/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java#L250
>>>
>>> Here we see that when writing to the XML for all of the subsequent runs
>>> after the first run, we are getting the singleRunEntry.
>>> getReportEntryType().getRerunXmlTag(), which in the case for the
>>> ReportType enum, SKIPPED has its flakyXmlTag and rerunXmlTag defined as "",
>>> an empty string.
>>>
>>> https://github.com/apache/maven-surefire/blob/7149edc6f24fa8bff06372e24a177e86d4960d8c/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/ReportEntryType.java#L31
>>>
>>> This conflict arises because the individual entry IS reported as
>>> skipped, but the getTestResultType check from the list or tries incorrectly
>>> reported it as a Failure. So it goes through the incorrect reporting type
>>> for the XML output, hence why it contains the system-out, and system-err
>>> sub elements
>>>
>>> Thank you for taking the time to look into this!
>>>
>>> *Ryan Thomas*
>>> *Sr. QA Engineer*
>>> BrainPOP
>>>
>>>
>>> On Mon, Nov 11, 2019 at 2:04 PM Tibor Digana <[hidden email]>
>>> wrote:
>>>
>>>> Hi Ryan,
>>>>
>>>> I found this issue already reported in JIRA.
>>>> https://issues.apache.org/jira/browse/SUREFIRE-1556
>>>>
>>>> There is no fix because we have not received any reproducible project.
>>>>
>>>> IMO this section should not exist in the XML:
>>>>
>>>> < message="This is a bug">
>>>>
>>>>       <system-out><![CDATA[...]]></system-out>
>>>>
>>>>       <system-err><![CDATA[...]]></system-err>
>>>>
>>>>     </>
>>>>
>>>> It looks like the message is XML attribute used as an element.
>>>>
>>>> I checked the StatelessXmlReporter  right now but i could not find any
>>>> logical reason for this issue.
>>>> I have really no idea what code and how injected this section into the
>>>> XML file.
>>>>
>>>> Here is the method which is responsible for writing the system-out and
>>>> system-err but I still do not see the root cause in the latest revision.
>>>>
>>>> https://github.com/apache/maven-surefire/blob/master/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/StatelessXmlReporter.java#L479
>>>>
>>>> Maybe you have older version where is this bug.
>>>> So the only thing we can do is to ask you to run the test with
>>>> 3.0.0-SNAPSHOT.
>>>> And then somebody has to debug the code. ;-(
>>>>
>>>> Pls write the test result in JIRA.
>>>> If the bug is still present with the snapshot version then I would ask
>>>> you to help us, dig in to the code and debug it.
>>>>
>>>> Cheers
>>>> Tibor
>>>>
>>>>
>>>> On Mon, Nov 11, 2019 at 6:33 PM Ryan Thomas <[hidden email]> wrote:
>>>>
>>>>> Hi Tibor,
>>>>>
>>>>>
>>>>>
>>>>> My Company BrainpOP is using Maven 3.6.2 with Junit4.7+ (Junit
>>>>> 4.13-beta-3, Junit 4.13-rc-1) and has recently come into an issue while
>>>>> trying to optimize flaky test run times.
>>>>>
>>>>>
>>>>>
>>>>> We are implementing assumptions to have Tests that are in a bad state
>>>>> to be skipped so that they do not add a ton of run time to the Test Suite
>>>>> in a case where the next Selenium run command fails with a long
>>>>> TimeOutException. However, we have come across some scenarios where
>>>>> sometime the Test might fail do to a failed Assertion, and on its next Run
>>>>> due to rerunFailingTestCount  criteria being met, an Assumption will fail
>>>>> resulting in further runs of the test being skipped. When this occurs, The
>>>>> surefire-report-plugin has been failing at parsing the generated XML.
>>>>>
>>>>>
>>>>>
>>>>> In analyzing the XML output for the scenario, I found the the XML
>>>>> generated was invalid, and that the message tied to where the Assumption
>>>>> failed in malformed. In this case, the Tag name (e.g. failure, skipped,
>>>>> rerunFailure, etc…) is missing and only the message attribute  and type
>>>>> remains. Looking at the surefire-test-report-3.0.xsd Schema, I can see that
>>>>> this is invalid.
>>>>>
>>>>>
>>>>>
>>>>> Here is the sample Test run in Junit through Maven
>>>>>
>>>>>
>>>>>
>>>>> @SuppressWarnings("deprecation")
>>>>>
>>>>> @Test
>>>>>
>>>>> public void testSomething3() {
>>>>>
>>>>>                 System.out.println("the end");
>>>>>
>>>>>                 i++;
>>>>>
>>>>>                 if(i == 3)
>>>>>
>>>>>                                 Assume.assumeTrue("This is a bug",
>>>>> false);
>>>>>
>>>>>                 else
>>>>>
>>>>>                                 Assert.assertTrue("Run: " + i,false);
>>>>>
>>>>> }
>>>>>
>>>>>
>>>>>
>>>>> Included here is a sample snippet of the malformed tag from the
>>>>> generated XML
>>>>>
>>>>>
>>>>>
>>>>> <testcase name="testSomething3"
>>>>> classname="com.brainpop.tests.LoggingTest" time="13.289">
>>>>>
>>>>>     <failure message="Run: 1"
>>>>> type="java.lang.AssertionError">java.lang.AssertionError: Run: 1
>>>>>
>>>>>                 at
>>>>> com.brainpop.tests.LoggingTest.testSomething3(LoggingTest.java:66)
>>>>>
>>>>> </failure>
>>>>>
>>>>>     <system-out><![CDATA[...]]></system-out>
>>>>>
>>>>>     <system-err><![CDATA[...]]></system-err>
>>>>>
>>>>>     <rerunFailure message="Run: 2" type="java.lang.AssertionError">
>>>>>
>>>>>       <stackTrace>java.lang.AssertionError: Run: 2
>>>>>
>>>>>                 at
>>>>> com.brainpop.tests.LoggingTest.testSomething3(LoggingTest.java:66)
>>>>>
>>>>> </stackTrace>
>>>>>
>>>>>       <system-out><![CDATA[...]]></system-out>
>>>>>
>>>>>       <system-err><![CDATA[...]]></system-err>
>>>>>
>>>>>     </rerunFailure>
>>>>>
>>>>>     < message="This is a bug">
>>>>>
>>>>>       <system-out><![CDATA[...]]></system-out>
>>>>>
>>>>>       <system-err><![CDATA[...]]></system-err>
>>>>>
>>>>>     </>
>>>>>
>>>>>   </testcase>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Sent from Mail <https://go.microsoft.com/fwlink/?LinkId=550986> for
>>>>> Windows 10
>>>>>
>>>>>
>>>>>
>>>>