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

Ordering mode not practical now #96

Closed
lordofthejars opened this Issue Aug 5, 2017 · 7 comments

Comments

@lordofthejars
Member

lordofthejars commented Aug 5, 2017

After doing some tests with spark I noticed that ordering mode might not be useful right now. Let me explain two scenarios:

Project with single module

In this case surefire plugin is used. If one test fails, the build does not fails and continue the execution until the end. So ordering is executing the "affected" test first but it doesn't care if fails at first since you still need to wait the whole build. To fix this surefire comes with this property http://maven.apache.org/surefire/maven-surefire-plugin/examples/skip-after-failure.html but does not work in case of reuseForks equals to false. So maybe in configuration phase if we detect that reuseForks is set to true (default value) then we could configure automatically this flag.

Multimodule project

In this case it is the same as single module but I think it is even worst. In this case suppose that the modified test is on latest module of the project. Then since surefire is executing modules one after the other, until the module is not executed the build will not fail, so we are effectively ordering tests inside module but not the whole build.

With microservices which everything should be small probably everything is single module but in legacy code or libraries might be a problem.

So maybe we need to think about ordering modules as well.

@dipak-pawar

This comment has been minimized.

Show comment
Hide comment
@dipak-pawar

dipak-pawar Sep 1, 2017

Contributor

Current approach to solve this problem:

If smart testing is enabled:

  • calculateChanges
  • Project ordering:
    • We have all projects available in SmartTestingMavenConfigurer in afterProjectsRead, where we have entire list of project dependencies from where we can take required projects only.
  • Set list of required projectsto instance of MavenSession

How to select required projects?

Consider project structure like this:

 Parent
  - Module A
  - Module B

Assume Module A is dependent on Module B
So by default reactor order is

 Parent 
 - Module B
 - Module A

So we will look for any change in Module B, If yes then run tests against it, otherwise skip it & execute tests against Module A.

So In short if there is any change in the module then execute goal against it, otherwise skip it. While doing this we'll give priority for modules which has some change which we can calculate using scm.

With this we can solve problem of multi-module.

For single module, we can use approach suggested by @lordofthejars, we can set skipAfterFailureCount option from surefire on the fly.

Any suggestions welcome @bartoszmajsak, @hemanik, @MatousJobanek, @lordofthejars .

Contributor

dipak-pawar commented Sep 1, 2017

Current approach to solve this problem:

If smart testing is enabled:

  • calculateChanges
  • Project ordering:
    • We have all projects available in SmartTestingMavenConfigurer in afterProjectsRead, where we have entire list of project dependencies from where we can take required projects only.
  • Set list of required projectsto instance of MavenSession

How to select required projects?

Consider project structure like this:

 Parent
  - Module A
  - Module B

Assume Module A is dependent on Module B
So by default reactor order is

 Parent 
 - Module B
 - Module A

So we will look for any change in Module B, If yes then run tests against it, otherwise skip it & execute tests against Module A.

So In short if there is any change in the module then execute goal against it, otherwise skip it. While doing this we'll give priority for modules which has some change which we can calculate using scm.

With this we can solve problem of multi-module.

For single module, we can use approach suggested by @lordofthejars, we can set skipAfterFailureCount option from surefire on the fly.

Any suggestions welcome @bartoszmajsak, @hemanik, @MatousJobanek, @lordofthejars .

@bartoszmajsak

This comment has been minimized.

Show comment
Hide comment
@bartoszmajsak

bartoszmajsak Sep 1, 2017

Member

For single module, we can use approach suggested by @lordofthejars, we can set skipAfterFailureCount option from surefire on the fly.

Let's make this one as an instant win. For the more sophisticated stuff I have to chew it over, but overall seems reasonable if we use selecting mode. Let's discuss it on next weeks Cabal?

Member

bartoszmajsak commented Sep 1, 2017

For single module, we can use approach suggested by @lordofthejars, we can set skipAfterFailureCount option from surefire on the fly.

Let's make this one as an instant win. For the more sophisticated stuff I have to chew it over, but overall seems reasonable if we use selecting mode. Let's discuss it on next weeks Cabal?

@dipak-pawar

This comment has been minimized.

Show comment
Hide comment
@dipak-pawar

dipak-pawar Sep 1, 2017

Contributor

@bartoszmajsak I believe skipAfterFailureCount has limitations if we are running tests in parallel mode. So it won't work fully for parallel builds. So using this we are solving this problem fully for non-parallel builds

Sure we can discuss it on next weeks cabal.

Contributor

dipak-pawar commented Sep 1, 2017

@bartoszmajsak I believe skipAfterFailureCount has limitations if we are running tests in parallel mode. So it won't work fully for parallel builds. So using this we are solving this problem fully for non-parallel builds

Sure we can discuss it on next weeks cabal.

@bartoszmajsak

This comment has been minimized.

Show comment
Hide comment
@bartoszmajsak

bartoszmajsak Sep 1, 2017

Member

But this something we cannot overcome easily. I think it would still be better than nothing?

Member

bartoszmajsak commented Sep 1, 2017

But this something we cannot overcome easily. I think it would still be better than nothing?

@dipak-pawar

This comment has been minimized.

Show comment
Hide comment
@dipak-pawar

dipak-pawar Sep 1, 2017

Contributor

Yes I agreed with you.

Contributor

dipak-pawar commented Sep 1, 2017

Yes I agreed with you.

dipak-pawar added a commit to dipak-pawar/smart-testing that referenced this issue Sep 7, 2017

dipak-pawar added a commit to dipak-pawar/smart-testing that referenced this issue Sep 7, 2017

dipak-pawar added a commit to dipak-pawar/smart-testing that referenced this issue Sep 7, 2017

dipak-pawar added a commit to dipak-pawar/smart-testing that referenced this issue Sep 8, 2017

dipak-pawar added a commit to dipak-pawar/smart-testing that referenced this issue Sep 12, 2017

dipak-pawar added a commit to dipak-pawar/smart-testing that referenced this issue Sep 12, 2017

dipak-pawar added a commit to dipak-pawar/smart-testing that referenced this issue Sep 13, 2017

@bartoszmajsak

This comment has been minimized.

Show comment
Hide comment
@bartoszmajsak

bartoszmajsak Sep 13, 2017

Member

When releasing the very first version I decided not to put it as part of the release.

We somehow forgot in this discussion that Surefire also has its own ordering e.g.:

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-surefire-plugin</artifactId>
    <configuration>
        <runOrder>alphabetical</runOrder>
    </configuration>
</plugin>

and it does not set failure count to any number. So I think this is actually desired behaviour and we should not alternate and be consistent with Surefire. Maybe we can document / educate users about skipAfterFailureCount feature, but I am not convinced anymore if it should be built-in to our tool.

Member

bartoszmajsak commented Sep 13, 2017

When releasing the very first version I decided not to put it as part of the release.

We somehow forgot in this discussion that Surefire also has its own ordering e.g.:

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-surefire-plugin</artifactId>
    <configuration>
        <runOrder>alphabetical</runOrder>
    </configuration>
</plugin>

and it does not set failure count to any number. So I think this is actually desired behaviour and we should not alternate and be consistent with Surefire. Maybe we can document / educate users about skipAfterFailureCount feature, but I am not convinced anymore if it should be built-in to our tool.

@lordofthejars

This comment has been minimized.

Show comment
Hide comment
@lordofthejars

lordofthejars Oct 6, 2017

Member

I think we can close this issue for now.

Member

lordofthejars commented Oct 6, 2017

I think we can close this issue for now.

@MatousJobanek MatousJobanek added this to the 0.0.3 milestone Oct 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment