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

make @AnalyzeClasses import package of test class by default #828

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 4 additions & 0 deletions archunit-junit/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ dependencies {
exclude module: 'guava'
}
testImplementation project(path: ':archunit', configuration: 'tests')

// This is a hack for running tests with IntelliJ instead of delegating to Gradle,
// because for some reason this dependency cannot be resolved otherwise :-(
testRuntimeOnly dependency.asm
}

archUnitTest {
Expand Down
2 changes: 1 addition & 1 deletion archunit-junit/junit4/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ dependencies {
testImplementation project(path: ':archunit-junit', configuration: 'tests')

// This is a hack for running tests with IntelliJ instead of delegating to Gradle,
// because for some reason this dependency cannot be resolved otherwise only for archunit-junit4 :-(
// because for some reason this dependency cannot be resolved otherwise :-(
testRuntimeOnly dependency.asm
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@
*/
Class<? extends LocationProvider>[] locations() default {};

/**
* @return Whether to import all classes on the classpath.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe scan or analyze instead of import, to better distinguish the wording from the ImportOptions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What about Whether to look for classes on the whole classpath? I just checked the other Javadocs and that seems consistent to packages(), etc...

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good

* Warning: Without any further filtering this can require a lot of resources.
*/
boolean wholeClasspath() default false;

/**
* Allows to filter the class import. The supplied types will be instantiated and used to create the
* {@link ImportOptions} passed to the {@link ClassFileImporter}. Considering caching, compare the notes on
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,5 +217,10 @@ public Class<? extends ImportOption>[] getImportOptions() {
public CacheMode getCacheMode() {
return analyzeClasses.cacheMode();
}

@Override
public boolean scanWholeClasspath() {
return analyzeClasses.wholeClasspath();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public void runner_creates_correct_analysis_request() {
assertThat(analysisRequest.getPackageNames()).isEqualTo(analyzeClasses.packages());
assertThat(analysisRequest.getPackageRoots()).isEqualTo(analyzeClasses.packagesOf());
assertThat(analysisRequest.getLocationProviders()).isEqualTo(analyzeClasses.locations());
assertThat(analysisRequest.scanWholeClasspath()).as("scan whole classpath").isTrue();
assertThat(analysisRequest.getImportOptions()).isEqualTo(analyzeClasses.importOptions());
}

Expand Down Expand Up @@ -149,11 +150,12 @@ public boolean includes(Location location) {
packages = {"com.foo", "com.bar"},
packagesOf = {ArchUnitRunner.class, ArchUnitRunnerTest.class},
locations = {DummyLocation.class, OtherDummyLocation.class},
wholeClasspath = true,
importOptions = {DummyImportOption.class, OtherDummyImportOption.class}
)
public static class MaxAnnotatedTest {
@ArchTest
public static void someTest(JavaClasses classes) {
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@
*/
Class<? extends LocationProvider>[] locations() default {};

/**
* @return Whether to look for classes on the whole classpath.
* Warning: Without any further {@link #importOptions() filtering} this can require a lot of resources.
*/
boolean wholeClasspath() default false;

/**
* Allows to filter the class import. The supplied types will be instantiated and used to create the
* {@link ImportOptions} passed to the {@link ClassFileImporter}. Considering caching, compare the notes on
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,5 +289,10 @@ public Class<? extends ImportOption>[] getImportOptions() {
public CacheMode getCacheMode() {
return analyzeClasses.cacheMode();
}

@Override
public boolean scanWholeClasspath() {
return analyzeClasses.wholeClasspath();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,7 @@ void passes_AnalyzeClasses_to_cache() {
assertThat(request.getPackageNames()).isEqualTo(expected.packages());
assertThat(request.getPackageRoots()).isEqualTo(expected.packagesOf());
assertThat(request.getLocationProviders()).isEqualTo(expected.locations());
assertThat(request.scanWholeClasspath()).as("scan whole classpath").isTrue();
assertThat(request.getImportOptions()).isEqualTo(expected.importOptions());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
packages = {"first.pkg", "second.pkg"},
packagesOf = {Object.class, File.class},
locations = {FirstLocationProvider.class, SecondLocationProvider.class},
wholeClasspath = true,
importOptions = {DoNotIncludeTests.class, DoNotIncludeJars.class})
public class FullAnalyzeClassesSpec {
@ArchTest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ interface ClassAnalysisRequest {
Class<? extends ImportOption>[] getImportOptions();

CacheMode getCacheMode();

boolean scanWholeClasspath();
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.tngtech.archunit.junit;

import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -154,40 +155,43 @@ private abstract static class RequestedLocations {

abstract LocationsKey asKey();

private static class All extends RequestedLocations {
private Class<? extends ImportOption>[] importOptions;
private static class AllInTestPackage extends RequestedLocations {
private final Class<? extends ImportOption>[] importOptions;
private final Set<Location> locationsOfTestPackage;

private All(Class<? extends ImportOption>[] importOptions) {
private AllInTestPackage(Class<? extends ImportOption>[] importOptions, Class<?> testClass) {
this.importOptions = importOptions;
locationsOfTestPackage = Locations.ofPackage(testClass.getPackage().getName());
}

@Override
LocationsKey asKey() {
return new LocationsKey(importOptions, Locations.inClassPath());
return new LocationsKey(importOptions, locationsOfTestPackage);
}
}

private static class Specific extends RequestedLocations {
private final ClassAnalysisRequest classAnalysisRequest;
private final Class<? extends ImportOption>[] importOptions;
private final Set<Location> declaredLocations;

private Specific(ClassAnalysisRequest classAnalysisRequest, Class<?> testClass) {
this.classAnalysisRequest = classAnalysisRequest;
importOptions = classAnalysisRequest.getImportOptions();
declaredLocations = ImmutableSet.<Location>builder()
.addAll(getLocationsOfPackages())
.addAll(getLocationsOfProviders(testClass))
.addAll(getLocationsOfPackages(classAnalysisRequest))
.addAll(getLocationsOfProviders(classAnalysisRequest, testClass))
.addAll(classAnalysisRequest.scanWholeClasspath() ? Locations.inClassPath() : Collections.<Location>emptySet())
.build();
}

private Set<Location> getLocationsOfPackages() {
private Set<Location> getLocationsOfPackages(ClassAnalysisRequest classAnalysisRequest) {
Set<String> packages = ImmutableSet.<String>builder()
.add(classAnalysisRequest.getPackageNames())
.addAll(toPackageStrings(classAnalysisRequest.getPackageRoots()))
.build();
return locationsOf(packages);
}

private Set<Location> getLocationsOfProviders(Class<?> testClass) {
private Set<Location> getLocationsOfProviders(ClassAnalysisRequest classAnalysisRequest, Class<?> testClass) {
Set<Location> result = new HashSet<>();
for (Class<? extends LocationProvider> providerClass : classAnalysisRequest.getLocationProviders()) {
result.addAll(tryCreate(providerClass).get(testClass));
Expand Down Expand Up @@ -224,20 +228,21 @@ private Set<Location> locationsOf(Set<String> packages) {

@Override
LocationsKey asKey() {
return new LocationsKey(classAnalysisRequest.getImportOptions(), declaredLocations);
return new LocationsKey(importOptions, declaredLocations);
}
}

public static RequestedLocations by(ClassAnalysisRequest classAnalysisRequest, Class<?> testClass) {
return noSpecificLocationRequested(classAnalysisRequest) ?
new All(classAnalysisRequest.getImportOptions()) :
new AllInTestPackage(classAnalysisRequest.getImportOptions(), testClass) :
new Specific(classAnalysisRequest, testClass);
}

private static boolean noSpecificLocationRequested(ClassAnalysisRequest classAnalysisRequest) {
return classAnalysisRequest.getPackageNames().length == 0
&& classAnalysisRequest.getPackageRoots().length == 0
&& classAnalysisRequest.getLocationProviders().length == 0;
&& classAnalysisRequest.getLocationProviders().length == 0
&& !classAnalysisRequest.scanWholeClasspath();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import com.tngtech.archunit.core.domain.JavaClass;
import com.tngtech.archunit.core.domain.JavaClasses;
import com.tngtech.archunit.core.importer.ClassFileImporter;
import com.tngtech.archunit.core.importer.ImportOption;
import com.tngtech.archunit.core.importer.ImportOptions;
import com.tngtech.archunit.core.importer.Location;
Expand All @@ -15,6 +16,7 @@
import com.tngtech.java.junit.dataprovider.DataProvider;
import com.tngtech.java.junit.dataprovider.DataProviderRunner;
import com.tngtech.java.junit.dataprovider.UseDataProvider;
import org.assertj.core.api.Condition;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
Expand All @@ -32,6 +34,7 @@
import static com.tngtech.java.junit.dataprovider.DataProviders.testForEach;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
Expand Down Expand Up @@ -125,7 +128,6 @@ public void get_all_classes_by_LocationProvider() {

@Test
public void rejects_LocationProviders_without_default_constructor() {

thrown.expect(ArchTestExecutionException.class);
thrown.expectMessage("public default constructor");
thrown.expectMessage(LocationProvider.class.getSimpleName());
Expand All @@ -134,10 +136,38 @@ public void rejects_LocationProviders_without_default_constructor() {
analyzeLocation(WrongLocationProviderWithConstructorParam.class));
}

@Test
public void if_no_import_locations_are_specified_and_whole_classpath_is_set_false_then_the_default_is_the_package_of_the_test_class() {
TestAnalysisRequest defaultOptions = new TestAnalysisRequest().withWholeClasspath(false);

JavaClasses classes = cache.getClassesToAnalyzeFor(TestClass.class, defaultOptions);

assertThatTypes(classes).contain(getClass(), TestAnalysisRequest.class);
assertThatTypes(classes).doNotContain(ClassFileImporter.class);
}

@Test
public void if_whole_classpath_is_set_true_then_the_whole_classpath_is_imported() {
TestAnalysisRequest defaultOptions = new TestAnalysisRequest().withWholeClasspath(true);
Class<?>[] expectedImportResult = new Class[]{getClass()};
doReturn(new ClassFileImporter().importClasses(expectedImportResult))
.when(cacheClassFileImporter).importClasses(any(ImportOptions.class), ArgumentMatchers.<Location>anyCollection());

JavaClasses classes = cache.getClassesToAnalyzeFor(TestClass.class, defaultOptions);

assertThatTypes(classes).matchExactly(expectedImportResult);
verify(cacheClassFileImporter).importClasses(any(ImportOptions.class), locationCaptor.capture());
assertThat(locationCaptor.getValue())
.has(locationContaining("archunit"))
.has(locationContaining("asm"))
.has(locationContaining("google"))
.has(locationContaining("mockito"));
}

@Test
public void filters_urls() {
JavaClasses classes = cache.getClassesToAnalyzeFor(TestClass.class,
new TestAnalysisRequest().withImportOptions(TestFilterForJUnitJars.class));
new TestAnalysisRequest().withImportOptions(TestFilterForJUnitJars.class).withWholeClasspath(true));

assertThat(classes).isNotEmpty();
for (JavaClass clazz : classes) {
Expand Down Expand Up @@ -218,6 +248,20 @@ private void verifyNumberOfImports(int number) {
verifyNoMoreInteractions(cacheClassFileImporter);
}

private static Condition<Iterable<? extends Location>> locationContaining(final String part) {
return new Condition<Iterable<? extends Location>>() {
@Override
public boolean matches(Iterable<? extends Location> locations) {
for (Location location : locations) {
if (location.contains(part)) {
return true;
}
}
return false;
}
};
}

public static class TestClass {
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ class TestAnalysisRequest implements ClassAnalysisRequest {
private String[] packages = new String[0];
private Class<?>[] packageRoots = new Class<?>[0];
private Class<? extends LocationProvider>[] locationProviders = new Class[0];
private boolean wholeClasspath = false;
private Class<? extends ImportOption>[] importOptions = new Class[0];
private CacheMode cacheMode = CacheMode.FOREVER;

Expand All @@ -25,6 +26,11 @@ public Class<? extends LocationProvider>[] getLocationProviders() {
return locationProviders;
}

@Override
public boolean scanWholeClasspath() {
return wholeClasspath;
}

@Override
public Class<? extends ImportOption>[] getImportOptions() {
return importOptions;
Expand All @@ -51,6 +57,11 @@ final TestAnalysisRequest withLocationProviders(Class<? extends LocationProvider
return this;
}

final TestAnalysisRequest withWholeClasspath(boolean wholeClasspath) {
this.wholeClasspath = wholeClasspath;
return this;
}

@SafeVarargs
final TestAnalysisRequest withImportOptions(Class<? extends ImportOption>... importOptions) {
this.importOptions = importOptions;
Expand Down