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 <tibordig...@apache.org> 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 <ry...@brainpop.com> 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 <tibordig...@apache.org> >> 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 <ry...@brainpop.com> 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 >>>> >>>> >>>> >>>
