SUREFIRE-1372 Filter tests to be rerun by description #150
Conversation
|
This is still WIP until cucumber-jvm 2.0.0 is done. |
mpkorstanje
changed the title
mpkorstanje
changed the title
|
All done. Tests are running against |
|
@mpkorstanje |
|
Yes! We're satisfied with the current state of the code and more people are now able to make releases. As such we're finalizing the release at the moment, I expect we'll do a couple of last minute bug fixes but that should be it. I can't make any promises about a time frame but I reckon it will be soon now. |
|
ok, take your time. No worries. Just rebase the PR after your team finished the release. Surefire need some week to make release as well. |
|
Cucumber 2.0.0 was released a few days ago. |
| </dependency> | ||
| </dependencies> | ||
|
|
||
| <repositories> |
| * Re-run execution in Cucumber JVM | ||
|
|
||
| Since of 2.21 the provider <<<surefire-junit47>>> can rerun scenarios created by <<cucumber-jvm>> | ||
| 2.0.0-SNAPSHOT and higher. |
There was a problem hiding this comment.
Pls change to 2.0.0 without SNAPSHOT.
| <name>Test for rerun failing cucumber tests in JUnit 47</name> | ||
|
|
||
| <properties> | ||
| <cucumber.version>2.0.0-SNAPSHOT</cucumber.version> |
| /** | ||
| * Only run test methods in the given input map, indexed by test class | ||
| */ | ||
| final class FailingMethodFilter |
There was a problem hiding this comment.
I don't think this can be removed. I have to check the algorithms of FailingMethodFilter and MatchMethodDescriptions.
There was a problem hiding this comment.
Before deleting a class, tests with high coverage should be added and new implementation should pass on them.
There was a problem hiding this comment.
This is not really possible as as the way we're dealing selecting the tests to rerun has changed fundamentally. I have however added JUnit4ProviderUtilTest#testGenerateFailingTestDescriptions to show equivalence to JUnit4ProviderUtilTest#testGenerateFailingTests
This shows that the set of failed tests is the same as before - though described in a different format.
| } | ||
| return description.toString(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Here a new line should be added.
| @Override | ||
| public boolean shouldRun( Description description ) | ||
| { | ||
| for ( Filter filter : filters ) |
There was a problem hiding this comment.
This loop is different from the deleted class but I need to have spare time to have deep looking.
There was a problem hiding this comment.
MatchMethodDescriptions is created with a list of failed test descriptions. FailingMethodFilter was created with a list of failed class-method tuples.
When shouldRun is called the FailingMethodFilter would recurse down the description tree to find a leave (a test case). For each leave it uses the class of the description to find the set of failed methods for that class. Then it tests if the set of failed methods contain the method of the description. If so it returns true and the test should be rerun.
When shouldRun is called the MatchMethodDescriptions will check for each failed-test-description if it can be found in the the tree of descriptions. This is done by delegating the recursive descent search to Filter#matchMethodDescription. matchMethodDescription recurses until it finds a leave, when found it compares descriptions. When they match, the test should be rerun.
This should explain where the recursion has moved to.
|
I'll try to address the minor changes soontm. |
|
@mpkorstanje |
|
@Tibor17 whats ETA for this ? |
|
@ckurban |
|
@Tibor17 have there been any updates for this? I'm pretty keen to start using the new version if it fixes support for Cucumber tests with |
|
@sworisbreathing |
|
@mpkorstanje |
|
Is it possible to ask the original developer (@mpkorstanje ) to look at this deleted file changes as it might help get it through quickly. I suppose quite a number of people are waiting for this fix to go in (including me) |
|
@Tibor17 would it help if I rebased this branch against master? |
|
@Tibor17 It appears this branch is still up to date with master. Is the removal of the class the problem or the underlying fundamentals? If it is just the removal I can restore |
|
As far as I remember the filter was used in rerun feature where broken
tests were executed several more times after.
Yet let me check your branch because that's better than if you spent your
time again.
I will inform you about my findings in few days.
…On Fri, Dec 8, 2017 at 5:07 PM, M.P. Korstanje ***@***.***> wrote:
@Tibor17 <https://github.com/tibor17> It appears this branch is still up
to date with master.
Is the removal of the class the problem or the underlying fundamentals? If
it is just the removal I can restore FailingMethodFilter.java. It went
unused after my changes. If it is the underlying fundamentals please let me
know how I can help you.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#150 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA_yR4f-4bf3vz9RkLIyf-k7OEB7VA4Kks5s-V6rgaJpZM4NoCQj>
.
--
Cheers
Tibor
|
| </parent> | ||
|
|
||
| <artifactId>junit47-rerun-failing-tests-with-cucumber</artifactId> | ||
| <packaging>jar</packaging> |
There was a problem hiding this comment.
jar packaging is default one. You don't need to specify it.
|
@mpkorstanje |
|
@mpkorstanje |
|
@mpkorstanje |
|
@Tibor17 that should be doable. Replacing the inner most loop in the for ( int i = 0; i < rerunFailingTestsCount && !failureListener.getAllFailures().isEmpty(); i++ )
{
Set<Description> failures = generateFailingTestDescriptions( failureListener.getAllFailures() );
failureListener.reset();
FilterFactory filterFactory = new FilterFactory( testClassLoader );
Filter failureDescriptionFilter = filterFactory.createMatchDescriptionsFilter( failures );
Request.aClass( clazz ).filterWith( failingMethodsFilter ).getRunner().run( rerunNotifier );
} |
Filtering tests by the tuple of (Class, Method) does not work when a test suite is used as a suite may not have (Class, Method) . Filtering by the description of the failed test should however work. Related Issues: - cucumber/cucumber-jvm#1134 - temyers/cucumber-jvm-parallel-plugin#31
Done.
I seem to be unable to get checkstyle to scan these tests and classes. I've visually checked these files. |
|
The fast way would be to give it a try in a copy of your project apart and
run the local build
"mvn install -P run-its -Dcheckstyle.skip=true".
Even if you copied FilterFactory into JUnit47 provider and used this loop
as you have mentioned above and build it, it would be safer for you. If the
build will be finally successful, we will just move the class FilterFactory
to the module common-junit4, refactor usages and then patch the JUnit4
provider in this PR.
…On Sat, Dec 23, 2017 at 9:25 PM, M.P. Korstanje ***@***.***> wrote:
@Tibor17 <https://github.com/tibor17> that should be doable. Replacing
the inner most loop in the JUnit4Provider.executeWithRerun with the code
below should do it. This would require the FilterFactory to be moved to
another common module. I can't quite tell what the the nock-on effects of
that would be.
for ( int i = 0; i < rerunFailingTestsCount && !failureListener.getAllFailures().isEmpty(); i++ )
{
Set<Description> failures = generateFailingTestDescriptions( failureListener.getAllFailures() );
failureListener.reset();
FilterFactory filterFactory = new FilterFactory( testClassLoader );
Filter failingMethodsFilter = filterFactory.createMatchMethodDescriptionsFilter( failures );
Request.aClass( clazz ).filterWith( failingMethodsFilter ).getRunner().run( rerunNotifier );
}
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#150 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA_yR-4dWYNw6T5vwzi-xzC6CWnSZL9yks5tDWG4gaJpZM4NoCQj>
.
--
Cheers
Tibor
|
|
I've added an extra commit onto this PR that shows just that. |
| @@ -55,7 +53,8 @@ | |||
| import static java.lang.reflect.Modifier.isAbstract; | |||
| import static java.lang.reflect.Modifier.isInterface; | |||
| import static org.apache.maven.surefire.booter.CommandReader.getReader; | |||
| import static org.apache.maven.surefire.common.junit4.JUnit4ProviderUtil.generateFailingTests; | |||
| import static org.apache.maven.surefire.common.junit4.FilterFactory.createMatchDescriptionsFilter; | |||
There was a problem hiding this comment.
It seems we can move this method to JUnit4ProviderUtil and remove org.apache.maven.surefire.common.junit4.FilterFactory.
|
@mpkorstanje I could not find a documentation for configuring annotation One more question I have is regarding artifacts of Cucumber. Which one is right to use: |
Cheers.
I've changed the layout of the project to match the default used by cucumber. When using the default layout All in all
|
|
Merged in 0a81c48. |
|
@mpkorstanje |
|
You're welcome! |
|
Hi Guys, Thanks for helping us by contributing to the open source community... I am using surefire 2.20.1 and getting the same exception "org.apache.maven.surefire.testset.TestSetFailedException: Unable to create test Class". Could you please let me know if I should be waiting for the next version of surefire or am I missing something else here? |
|
Bit hard to say without the full stack trace, components and version numbers involved. To fix this bug I also had to fix cucumber to ensure that the it provided the proper ids to Description.createSuiteDescription. The ids had to be unique in a single run but when a suite was recreated they had to be the same such as to satisfy the equals and hashcode method. |
|
Sorry for not giving enough information. the full track trace is here org.apache.maven.surefire.testset.TestSetFailedException: Unable to create testclass 'rerun_test' where 'rerun_test' is the feature file name. And the cucumber libraries used are 2.3.1 from group Id "io.cucumber" and cucumber-jvm-parallel-plugin version 4.2.0 |
|
In that case yes, you only need to wait for the release of surefire. cucumber/cucumber-jvm#1120 was fixed by cucumber/cucumber-jvm#1134 and is part of cucumber-jvm 2.x.x. |
|
Sure and let me wait!!!!! |
|
The project has new build script and testing JDK 7-10 in combination of
multiple platforms. That is the main purpose of Version 2.21.0. Sorry that
it takes so long. We are discovering issues and fixing them one by one. I
think we are close to the end.
…On Fri, Feb 2, 2018 at 4:02 PM, hemachandraraok ***@***.***> wrote:
Sure and let me wait!!!!!
Thank you so much for the updates!!!Cheers!!!!!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#150 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA_yR4VCbLtVu6ypblIkJm7ZSxUbmhBGks5tQyOcgaJpZM4NoCQj>
.
|
|
Now there is only one issue to fix related to Windows paths.
Need to investigate.
https://builds.apache.org/job/maven-surefire-pipeline/job/SUREFIRE-1463/121
[windows-jdk9-maven3.5.x]
shouldMatchSimpleClassNameAndMethodWithJavaExtWildcardPkg[0](org.apache.maven.surefire.its.TestMultipleMethodPatternsTestNGIT)
…On Sun, Feb 4, 2018 at 9:06 PM, Tibor Digana ***@***.***> wrote:
The project has new build script and testing JDK 7-10 in combination of
multiple platforms. That is the main purpose of Version 2.21.0. Sorry that
it takes so long. We are discovering issues and fixing them one by one. I
think we are close to the end.
On Fri, Feb 2, 2018 at 4:02 PM, hemachandraraok ***@***.***>
wrote:
> Sure and let me wait!!!!!
> Thank you so much for the updates!!!Cheers!!!!!
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#150 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AA_yR4VCbLtVu6ypblIkJm7ZSxUbmhBGks5tQyOcgaJpZM4NoCQj>
> .
>
|
|
The path: Contains 261 characters. MS allows for a maximum of 260 characters including the drive designation. https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx Assuming all the other parts of the path are not under your control you might want to shorten the method name a bit. |
|
Hi All.. with the new version of maven-surefire 2.21.0 and cucumber-jvm 2.4.0 (io.cucucmber)... we are able to perform rerun's successfully. However, I am facing a new issue with regards to reporting... in our project we have integrated our reports with VSTS i.e. after I perform parallel execution (using docker container) all the reports are getting generated with TEST-.(*).xml and VSTS then consolidates all the .xml files to give us a single report after parallel execution. But in the XML the testcase names are missing i.e. we are not able to view each step definitions (step-step names Given When Then) rather I am seeing only the scenario name as a result.. is anyone aware why we are having this ? and how we can resolve this issue? Below is my XML file if i am using info.cukes as group ID for execution (cucumber-jvm v 1.2.5) -- Rerun doesnt work here but test case name is available. Below is the my XML file if I am using io.cucumber as group id for execution (cucumber-jvm v 2.4.0) --- Rerun works perfectly fine but testcase name is missing
|
|
It would appear that your XML snippets have been truncated. Anyway. As of 2.x |
|
@mpkorstanje -- Thank you.. I did try by providing @CucumberOptions(junit = "--no-step-notifications") . but no luck.. I need each step to be show in my report. Please find below my runner file - Parallel01it.java (parallel execution) @RunWith(Cucumber.class) Below is the my XML file if I am using io.cucumber as group id for execution (cucumber-jvm v 2.4.0) --- Rerun works perfectly fine but testcase name is missing Below is what I am getting at present:
testcase name="Sign Up Verify That Error Message Is Shown If Any Of The Mandatory Field Is Left Blank" classname="Sign Up Verify That Error Message Is Shown If Any Of The Mandatory Field Is Left Blank" time="18.391"/> Below is my XML file if i am using info.cukes as group ID for execution (cucumber-jvm v 1.2.5) -- Rerun doesnt work here but test case name is available. testcase name="Given I am on the home page" classname="Scenario: Sign Up Verify That Error Message Is Shown If Any Of The Mandatory Field Is Left Blank" time="0"/> |
|
Try |
|
Thanks a lot @mpkorstanje ... My issue is fixed with this change 👍 |
|
@mpkorstanje -- Any idea how we can embed screenshot to XML report (surefire-XML report ?)... At present I am attaching/ embedding the failed screenshot to html report.. but I want to embed the screenshot to surefire-maven XML report TEST.(.*).XML |
|
@mpkorstanje <https://github.com/mpkorstanje> Should we change something
in Surefire documentation? It seems we should according to this discussion.
…On Sat, Mar 24, 2018 at 2:38 PM, adarshkv ***@***.***> wrote:
@mpkorstanje <https://github.com/mpkorstanje> -- Any idea how we can
embed screenshot to XML report (surefire-XML report ?)... At present I am
attaching/ embedding the failed screenshot to html report.. *but I want
to embed the screenshot to surefire-maven XML report TEST.(.*).XML*
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#150 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA_yR4Yd6Y6uonO-ScGf8ZOg8IQ5On0Qks5thkxWgaJpZM4NoCQj>
.
--
Cheers
Tibor
|
|
The current surefire documentation references Using Admittedly the documentation of |
Filtering tests by the tuple of (Class, Method) does not work when a test
suite is used as a suite may not have (Class, Method) . Filtering by the
description of the failed test should however work.
Related Issues: