Skip to content
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

Affected strategy should take into account also non-test-classes from the test directory src/test/java #202

Open
MatousJobanek opened this issue Oct 2, 2017 · 5 comments

Comments

@MatousJobanek
Copy link
Contributor

Issue Overview

Investigate the behavior of the method AffectedTestsDetector.getTests and the method TestVerifier.isCore.
The isCore method returns just a negation of the check if the class is a test class. However, in the provider here we create the TestVerifier saying that test classes are only those ones that are in the set of the tests to run: testsToRun.getClassByName(className) != null

In other words, consider test suite containing two test classes:

TestSomeObject.java
AnotherObjectTest.java

and inclusion: **/*Test.java
then the AffectedTestsDetector logic should take the first test class TestSomeObject.java as a business class. As a side effect, the affected method should work also on CustomAssertion classes. However when I tried it in my simple example and the tests affected by a change in a such a "test class" wasn't executed.

Expected Behaviour

When I change CustomVerifier and execute the build with mvn clean package -Dsmart.testing=affected then the FirstSimpleTest should be executed.

Current Behaviour

nothing is executed

@bartoszmajsak
Copy link
Member

Interesting thing here is that we don't really know what is test and what not until we analyze the source code and if for example we have methods with @test (coming from JUnit or Testng) or we extend from Specification class (Spock) etc. As much as it sounds unlikely you can pretty much use any pattern and surefire will run it. Is there any logic in surefire checking if a class matching defined pattern is an actual test?

@MatousJobanek
Copy link
Contributor Author

I think that we already know what is test class - classes contained in the list to be executed.
In other words, all classes scanned/filtered by surefire based on the content and the inclusions/exclusions parameters

@bartoszmajsak
Copy link
Member

On surefire side, sure, we are covered. The thing I'm wondering though is what about other integration points. How do we know there what is the test and what not?

@MatousJobanek
Copy link
Contributor Author

In Che, we are fine with this definition as well. When we call the ST tool, we already know which test classes have been chosen for the execution. In other cases of an integration with some IDE, it will be same I think.
In case of continuous testing approach, this will be a little bit harder. After all, we can still use the same logic for detecting test classes that is used normally (based on a content or based on some parameters).

@lordofthejars
Copy link
Member

Yeah this is what I was wondering all the time, how to detect a test. Just one thing to think about in the future, notice that what we are doing is everytime we want to know if a class is a test, we iterate though all lists of tests detected by surefire, and if it is there, then we know it is a test. But this comes with a computational cost. For example in case of assertJ is a list of 1k or so.

What I mean that for now it is ok, but we will need to find another way (call it inspecting annotations or whatever).

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

No branches or pull requests

3 participants