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

Transitivity in Affected strategy #98

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

Comments

Projects
None yet
3 participants
@lordofthejars
Member

lordofthejars commented Aug 7, 2017

Currently Affected strategy just read the imports from the test class and uses this information to decide if given business class should be executed or not.

This means that if you have Test A that depends on A class and this class at the same time depends on B class ATest -> A -> B and if the modified class is B then ATest is not executed.

Ideally affected strategy should also resolve transitive dependencies of business classes.

@lordofthejars

This comment has been minimized.

Show comment
Hide comment
@lordofthejars

lordofthejars Aug 8, 2017

Member

After thinking a bit about how to limit the growth of the graph I though next rules:

  • By default smart.testing.depth.level is equal to 5, I think that with this default we might cover a lot of cases, and this even more true in microservics world.
  • By default smart.testing.package.level is 2. This means that package filtering is done by getting the two first elements of the test package and classes are filtered by these numbers.
  • By default smart.testing.package.exclusions is empty.

Then last two options are exclusive so if smart.testing.package.exclusions is set, then smart.testing.package.level is ignored. And smart.testing.package.exclusions can receive a URL to a file containing all the exclusions in package form or a string in CSV form of exclusions package.

Member

lordofthejars commented Aug 8, 2017

After thinking a bit about how to limit the growth of the graph I though next rules:

  • By default smart.testing.depth.level is equal to 5, I think that with this default we might cover a lot of cases, and this even more true in microservics world.
  • By default smart.testing.package.level is 2. This means that package filtering is done by getting the two first elements of the test package and classes are filtered by these numbers.
  • By default smart.testing.package.exclusions is empty.

Then last two options are exclusive so if smart.testing.package.exclusions is set, then smart.testing.package.level is ignored. And smart.testing.package.exclusions can receive a URL to a file containing all the exclusions in package form or a string in CSV form of exclusions package.

@MatousJobanek

This comment has been minimized.

Show comment
Hide comment
@MatousJobanek

MatousJobanek Aug 8, 2017

Contributor

smart.testing.package.exclusions how about regex?
as for the defaults. well, why not... At first, I'd like to measure the impact of the levels on some big project. If there is a big difference between "5" vs "without limit" and similar...

Contributor

MatousJobanek commented Aug 8, 2017

smart.testing.package.exclusions how about regex?
as for the defaults. well, why not... At first, I'd like to measure the impact of the levels on some big project. If there is a big difference between "5" vs "without limit" and similar...

@lordofthejars

This comment has been minimized.

Show comment
Hide comment
@lordofthejars

lordofthejars Aug 8, 2017

Member

I thought about regexp, so you mean or file (with regexp) or directly regexp, I am concerned about performance

Member

lordofthejars commented Aug 8, 2017

I thought about regexp, so you mean or file (with regexp) or directly regexp, I am concerned about performance

@bartoszmajsak

This comment has been minimized.

Show comment
Hide comment
@bartoszmajsak

bartoszmajsak Aug 8, 2017

Member

Do we have any measures to be already concerned about the performance and have a real need to introduce things such as smart.testing.depth.level limitation?

Maybe it's better to have the code exercised against the full project/module, then measure and see where we could improve rather than starting with some heuristics which can seriously limit the usefulness of our tool.

Package filtering will obviously help, but I am not so sure about the others, to be honest.

Member

bartoszmajsak commented Aug 8, 2017

Do we have any measures to be already concerned about the performance and have a real need to introduce things such as smart.testing.depth.level limitation?

Maybe it's better to have the code exercised against the full project/module, then measure and see where we could improve rather than starting with some heuristics which can seriously limit the usefulness of our tool.

Package filtering will obviously help, but I am not so sure about the others, to be honest.

@lordofthejars

This comment has been minimized.

Show comment
Hide comment
@lordofthejars

lordofthejars Aug 8, 2017

Member

Well I am thinking for example in a Spring application, in these cases that Spring has several thousands of classes interconnected, I think it might be some problems.

Member

lordofthejars commented Aug 8, 2017

Well I am thinking for example in a Spring application, in these cases that Spring has several thousands of classes interconnected, I think it might be some problems.

@bartoszmajsak

This comment has been minimized.

Show comment
Hide comment
@bartoszmajsak

bartoszmajsak Aug 8, 2017

Member

I agree there will be "problems" - what I'm not sure about if what's proposed above is the solution. At least not yet.

Member

bartoszmajsak commented Aug 8, 2017

I agree there will be "problems" - what I'm not sure about if what's proposed above is the solution. At least not yet.

@lordofthejars

This comment has been minimized.

Show comment
Hide comment
@lordofthejars

lordofthejars Aug 8, 2017

Member

Well limiting the search tree for sure minimize the problem

Member

lordofthejars commented Aug 8, 2017

Well limiting the search tree for sure minimize the problem

@bartoszmajsak

This comment has been minimized.

Show comment
Hide comment
@bartoszmajsak

bartoszmajsak Aug 8, 2017

Member

Well limiting the search tree for sure minimize the problem

and the potential usefulness or in other words completeness of the related tests search.

Member

bartoszmajsak commented Aug 8, 2017

Well limiting the search tree for sure minimize the problem

and the potential usefulness or in other words completeness of the related tests search.

@lordofthejars

This comment has been minimized.

Show comment
Hide comment
@lordofthejars

lordofthejars Aug 9, 2017

Member

Current implementation uses regexp-like expressions setting as inclusions or exclusions, having priority exclusions.

The properties to set are: smart.testing.affected.exclusions and smart.testing.affected.inclusions in CSV format i.e org.mypackage.*, org.myotherpackage.MyClass

Also you can set them as a file by using smart.testing.affected.exclusions.file and smart.testing.affected.inclusions.file

Member

lordofthejars commented Aug 9, 2017

Current implementation uses regexp-like expressions setting as inclusions or exclusions, having priority exclusions.

The properties to set are: smart.testing.affected.exclusions and smart.testing.affected.inclusions in CSV format i.e org.mypackage.*, org.myotherpackage.MyClass

Also you can set them as a file by using smart.testing.affected.exclusions.file and smart.testing.affected.inclusions.file

@lordofthejars

This comment has been minimized.

Show comment
Hide comment
@lordofthejars

lordofthejars Aug 9, 2017

Member

Finally instead of having two properties to set inclusions and exclusions file, we opted for having for having one property called smart.testing.affected.config where you can set the file location of a properties file containing exclusions and inclusions in csv.

Notice that smart.testing.affected.exclusions and smart.testing.affected.inclusions are appended to config values

Member

lordofthejars commented Aug 9, 2017

Finally instead of having two properties to set inclusions and exclusions file, we opted for having for having one property called smart.testing.affected.config where you can set the file location of a properties file containing exclusions and inclusions in csv.

Notice that smart.testing.affected.exclusions and smart.testing.affected.inclusions are appended to config values

bartoszmajsak added a commit that referenced this issue Aug 10, 2017

fix(#98): adds transitivity in affectes strategy (#99)
* chore(refactor): Abstracts graph elements form JavaAssist
* fix(transitive): Avoid resolving some transitive imports
* fix: Don't create UnparsableClass in case of class not in ClassPool
* fix(build): introduces category for non-thread-safe tests and isolated surefire group
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment