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

Adds package annotation for affected tests #220

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions api/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<parent>
<artifactId>smart-testing-parent</artifactId>
<groupId>org.arquillian.smart.testing</groupId>
<version>0.0.4-SNAPSHOT</version>
</parent>
<modelVersion>4.0.0</modelVersion>

<artifactId>api</artifactId>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created a new artifact so users of this annotation don't need to import all smart testing affected strategy jar but just an special artifact. Since I think that in the future we might use more annotations, @MatousJobanek and me decided this would be as start a good place to put it.


</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package org.arquillian.smart.testing.strategies.affected;

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Repeatable;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* Annotation used to set which production classes are tested in current test.
* By default it appends all classes defined in all attributes.
*
* If none of the attributes are set, then all production classes with same package as test and its subpackages are added.
*/
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@Repeatable(TestsList.class)
@Documented
public @interface Tests {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By looking at this annotation my first thought was that it will execute list of test classes defined in it, so the name is a bit ambiguous. Maybe @Covers or @ComponentUnderTest makes it more self-explanatory?


/**
* Packages of classes that needs to be added as tested classes in current test. You can set the package name "org.superbiz" which means only classes defined in this package,
* or ending with start (*) operator "org.superbiz.*" which means all classes of current package and its subpackages.
* @return Packages containing Java classes.
*/
String[] packages() default {};

/**
* Packages of classes that needs to be added as tested classes in current test. It is used Class to get the package.
* Notice that in this case subpackages are not scanned.
* @return Packages containing Java classes.
*/
Class[] packagesOf() default {};

/**
* Classes to be added as tested classes in current test.
* @return Classes
*/
Class[] classes() default {};

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.arquillian.smart.testing.strategies.affected;

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@Documented
public @interface TestsList {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then this can become @ComponentsUnderTest

Tests[] value();
}
30 changes: 30 additions & 0 deletions docs/configuration.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,36 @@ IMPORTANT: This strategy is currently only applicable for _white box_ testing ap

WARNING: At this moment, this strategy does not work with Java 9.

===== Explicitly Set

By default affected strategy uses _imports_ of tests to build the graph of dependencies.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe graph of collaborators - dependency is rather broad term and could be understood that we build a graph of libraries we use based on imports

This approach is fine for unit tests (white box tests) but might not work in all cases of high level tests (black box test).

There are some test technologies that allows you to deploy an application locally and then run tests against it.
For example Wildfly Swarm has `@DefaultDeployment` annotation or for example Spring (Boot) deploys all application automatically.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove second for example in this statement. Also deploys entire application maybe?


This means that production classes are not directly imported into the test, so there is no way to get a relationship between test and production classes.
For this reason `affected` provides an annotation to set package(s) of production classes that are deployed by the test.

The first thing you need to do is register next artifact into your build script: `org.arquillian.smart.testing:api:<version>`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

next artifact -> following artifact?


Then you can annotate your test with `org.arquillian.smart.testing.strategies.affected.Tests` annotation.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be awesome to extract org.arquillian.smart.testing.strategies.affected.Tests automatically from the code - sounds like small doable extension?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if small but yes, let me open a new issue, since we are using the same approach in several places and several files I think it is better to implement as a separate PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, by all means, do so. I didn't mean to have it as part of this PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


For example:

[source, java]
----
include::../strategies/affected/src/test/java/org/arquillian/smart/testing/strategies/affected/fakeproject/test/ZTest.java[tag=docs]
----

In previous example all classes belonging to packages and subpackages specified at `packages` attribute are considered as classes used by the test.

You can also use `packageOf` attribute to set a reference class.
With this attribute all classes that are placed in the same package as the reference class are considered as classes used by the test.
With this approach your tests are resilient to package name changes.

If none of the attributes are set, then all production classes with same package as test and its subpackages are added automatically as classes used by the test.

==== Failed

`Failed` strategy just gets all tests that failed from previous executions and mark them as *important* tests to run first (_ordering_) or not filtered (_selecting_).
Expand Down
7 changes: 7 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
<version.assertj-core>3.8.0</version.assertj-core>
<version.system-rules>1.16.1</version.system-rules>
<version.javassist>3.21.0-GA</version.javassist>
<version.fast-classpath-scanner>2.7.4</version.fast-classpath-scanner>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really light project for scanning classpath. I think we can use more and more features it offers.

<version.jgrapht>1.0.1</version.jgrapht>
<version.shrinkwrap.resolver>3.0.0-beta-2</version.shrinkwrap.resolver>
<version.snakeyaml>1.19</version.snakeyaml>
Expand All @@ -70,6 +71,7 @@

<modules>
<module>core</module>
<module>api</module>
<module>surefire-provider</module>
<module>junit-test-result-parser</module>
<module>strategies/affected</module>
Expand Down Expand Up @@ -127,6 +129,11 @@
<artifactId>snakeyaml</artifactId>
<version>${version.snakeyaml}</version>
</dependency>
<dependency>
<groupId>io.github.lukehutch</groupId>
<artifactId>fast-classpath-scanner</artifactId>
<version>${version.fast-classpath-scanner}</version>
</dependency>
</dependencies>
</dependencyManagement>

Expand Down
9 changes: 9 additions & 0 deletions strategies/affected/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@
<artifactId>core</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.arquillian.smart.testing</groupId>
<artifactId>api</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.arquillian.smart.testing</groupId>
<artifactId>strategy-changed</artifactId>
Expand All @@ -29,6 +34,10 @@
<groupId>org.jgrapht</groupId>
<artifactId>jgrapht-core</artifactId>
</dependency>
<dependency>
<groupId>io.github.lukehutch</groupId>
<artifactId>fast-classpath-scanner</artifactId>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,18 @@
*/
package org.arquillian.smart.testing.strategies.affected;

import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner;
import java.io.File;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import org.arquillian.smart.testing.api.TestVerifier;
import org.arquillian.smart.testing.logger.Log;
import org.arquillian.smart.testing.logger.Logger;
import org.arquillian.smart.testing.strategies.affected.ast.JavaClass;
import org.arquillian.smart.testing.strategies.affected.ast.JavaClassBuilder;
import org.jgrapht.DirectedGraph;
Expand All @@ -44,6 +49,7 @@

public class ClassDependenciesGraph {

private static final Logger logger = Log.getLogger();
private static final Filter coreJava = new Filter("", "java.*");

private final JavaClassBuilder builder;
Expand Down Expand Up @@ -73,14 +79,86 @@ void buildTestDependencyGraph(Collection<File> testJavaFiles) {

// Then find dependencies
for (String changedTestClassNames : testClassesNames) {
JavaClass javaClass = builder.getClassDescription(changedTestClassNames);
if (javaClass != null) {
addToIndex(new JavaElement(javaClass), javaClass.getImports());
JavaClass testJavaClass = builder.getClassDescription(changedTestClassNames);
if (testJavaClass != null) {
final String[] imports = testJavaClass.getImports();
final List<String> manualProductionClasses = calculateManualAddedDependencies(testJavaClass);
manualProductionClasses.addAll(Arrays.asList(imports));
addToIndex(new JavaElement(testJavaClass), manualProductionClasses);
}
}
}

private void addToIndex(JavaElement javaElement, String[] imports) {
private List<String> calculateManualAddedDependencies(JavaClass testJavaClass) {
final List<String> manualDependencyClasses = new ArrayList<>();
final Tests[] allTestsAnnotation = getAllAnnotations(testJavaClass);

for (Tests tests : allTestsAnnotation) {
List<String> packages = getPackages(testJavaClass.packageName(), tests);
for (String packag : packages) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo packag :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well it is not a type, package is a reserved word, so I decided to put in this way. But any other suggestion is welcome :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was late hahahaha. pkg maybe? :)

facepalm-real

final String trimmedPackage = packag.trim();
manualDependencyClasses.addAll(scanClassesFromPackage(trimmedPackage));
}
}

return manualDependencyClasses;

}

private Tests[] getAllAnnotations(JavaClass testJavaClass) {

final Optional<TestsList> testsListOptional = testJavaClass.getAnnotationByType(TestsList.class);

Tests[] tests = testsListOptional
.map(TestsList::value)
.orElseGet(() -> testJavaClass.getAnnotationByType(Tests.class)
.map(annotation -> new Tests[] {annotation})
.orElse(new Tests[0]));


return tests;
}

private List<String> scanClassesFromPackage(String trimmedPackage) {
final List<String> manualDependencyClasses = new ArrayList<>();
if (trimmedPackage.endsWith(".*")) {
String realPackage = trimmedPackage.substring(0, trimmedPackage.indexOf(".*"));
final List<String> classesOfPackage =
new FastClasspathScanner(realPackage).scan()
.getNamesOfAllClasses();

manualDependencyClasses.addAll(
classesOfPackage);
} else {
final List<String> classesOfPackage =
new FastClasspathScanner(trimmedPackage).disableRecursiveScanning().scan()
.getNamesOfAllClasses();
manualDependencyClasses.addAll(
classesOfPackage);
}

if (manualDependencyClasses.isEmpty()) {
logger.warn("You set %s package as reference classes to run tests, but no classes found. Maybe a package refactor?", trimmedPackage);
}

return manualDependencyClasses;
}

private List<String> getPackages(String testPackage, Tests tests) {
List<String> packages = new ArrayList<>();
if (tests.classes().length == 0 && tests.packages().length == 0 && tests.packagesOf().length == 0) {
packages.add(testPackage + ".*");
} else {
packages.addAll(Arrays.asList(tests.packages()));

packages.addAll(Arrays.stream(tests.packagesOf())
.map(clazz -> clazz.getPackage().getName())
.collect(Collectors.toList()));
}
return packages;
}

private void addToIndex(JavaElement javaElement, List<String> imports) {
addToGraph(javaElement);
updateJavaElementWithImportReferences(javaElement, imports);
}
Expand All @@ -101,14 +179,14 @@ private void replaceVertex(JavaElement newClass) {
}
}

private void updateJavaElementWithImportReferences(JavaElement javaElementParentClass, String[] imports) {
private void updateJavaElementWithImportReferences(JavaElement javaElementParentClass, List<String> imports) {

for (String importz : imports) {

if (addImport(javaElementParentClass, importz) && filter.shouldBeIncluded(importz) && this.enableTransitivity) {
JavaClass javaClass = builder.getClassDescription(importz);
if (javaClass != null) {
updateJavaElementWithImportReferences(javaElementParentClass, javaClass.getImports());
updateJavaElementWithImportReferences(javaElementParentClass, Arrays.asList(javaClass.getImports()));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import javassist.CtClass;
import javassist.CtField;
Expand All @@ -53,10 +54,12 @@ public class JavaAssistClass extends AbstractJavaClass {
private final String[] imports;
private final String className;
private File classFile;
private final CtClass classReference;

JavaAssistClass(CtClass classReference) {
imports = findImports(classReference);
className = classReference.getName();
this.classReference = classReference;
}

@Override
Expand Down Expand Up @@ -163,6 +166,11 @@ public String getName() {
return className;
}

@Override
public String packageName() {
return classReference.getPackageName();
}

@Override
public String toString() {
return getName();
Expand All @@ -176,4 +184,13 @@ public void setClassFile(File classFile) {
public File getClassFile() {
return classFile;
}

@Override
public <T> Optional<T> getAnnotationByType(Class<T> type) {
try {
return Optional.ofNullable((T) this.classReference.getAnnotation(type));
} catch (ClassNotFoundException e) {
throw new IllegalStateException(e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,20 @@
package org.arquillian.smart.testing.strategies.affected.ast;

import java.io.File;
import java.util.Optional;

public interface JavaClass {
String getName();

String packageName();
/**
* Gets the collection on classes that this class depends on. i.e. the list
* of this classes children.
*/
String[] getImports();

File getClassFile();

<T> Optional<T> getAnnotationByType(Class<T> type);

}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
package org.arquillian.smart.testing.strategies.affected.ast;

import java.io.File;
import java.util.Optional;

public class UnparsableClass implements JavaClass {
private static final String[] NO_IMPORT = new String[0];
Expand All @@ -43,6 +44,11 @@ public File getClassFile() {
return null;
}

@Override
public <T> Optional<T> getAnnotationByType(Class<T> type) {
return Optional.empty();
}

@Override
public String[] getImports() {
return NO_IMPORT;
Expand All @@ -53,6 +59,11 @@ public String getName() {
return classname;
}

@Override
public String packageName() {
return "";
}

@Override
public String toString() {
return "UnparsableClass{" + "classname='" + classname + '\'' + '}';
Expand Down
Loading