Skip to content

Commit

Permalink
Merge pull request #85 from menny/allow-test-only-deps
Browse files Browse the repository at this point in the history
Fixing test-only marking for inner-deps
  • Loading branch information
menny authored Jun 2, 2021
2 parents f05a194 + 2a7553e commit 8187322
Show file tree
Hide file tree
Showing 4 changed files with 253 additions and 14 deletions.
15 changes: 15 additions & 0 deletions resolver/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,21 @@ java_test(
],
)

java_test(
name = "test_only_merger_test",
size = "small",
srcs = [
"src/main/javatest/net/evendanan/bazel/mvn/merger/TestOnlyMarkerTest.java",
],
test_class = "net.evendanan.bazel.mvn.merger.TestOnlyMarkerTest",
deps = [
":api_lib",
":merger",
"//resolver/main_deps/junit/junit",
"//resolver/main_deps/org/mockito/mockito-core",
],
)

java_test(
name = "downloader_tests",
size = "small",
Expand Down
38 changes: 24 additions & 14 deletions resolver/src/main/java/net/evendanan/bazel/mvn/Merger.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import net.evendanan.bazel.mvn.merger.PublicTargetsCategory;
import net.evendanan.bazel.mvn.merger.SourcesJarLocator;
import net.evendanan.bazel.mvn.merger.TargetCommenter;
import net.evendanan.bazel.mvn.merger.TestOnlyMarker;
import net.evendanan.timing.ProgressTimer;

import java.io.File;
Expand All @@ -42,6 +43,7 @@
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
Expand Down Expand Up @@ -154,9 +156,20 @@ public static void main(String[] args) throws Exception {
}
System.out.println("✓");

System.out.print("Marking dependencies as test-only...");
final Predicate<MavenCoordinate> testOnlyDeps = TestOnlyMarker.mark(
resolutions,
resolutionOutputs
.stream()
.filter(ResolutionOutput::testOnly)
.map(r -> r.resolution().rootDependency())
.collect(Collectors.toSet()));
System.out.println("✓");

driver.writeResults(
resolutionOutputs,
dependencies,
testOnlyDeps,
downloader,
options,
dependencyTools);
Expand Down Expand Up @@ -250,16 +263,15 @@ private Collection<ResolutionOutput> readResolutions(CommandLineOptions options)
final Serialization serialization = new Serialization();
final List<ResolutionOutput> resolutions =
options.artifacts.stream()
.map(
inputFile -> {
System.out.print('.');
try (final FileReader reader =
new FileReader(inputFile, Charsets.UTF_8)) {
return serialization.deserialize(reader);
} catch (Exception e) {
throw new RuntimeException(e);
}
})
.map(inputFile -> {
System.out.print('.');
try (final FileReader reader =
new FileReader(inputFile, Charsets.UTF_8)) {
return serialization.deserialize(reader);
} catch (Exception e) {
throw new RuntimeException(e);
}
})
.collect(Collectors.toList());
System.out.println();

Expand All @@ -276,6 +288,7 @@ private Collection<Dependency> mergeResolutions(Collection<Resolution> resolutio
private void writeResults(
Collection<ResolutionOutput> resolutions,
Collection<Dependency> resolvedDependencies,
final Predicate<MavenCoordinate> testOnlyDeps,
final Function<Dependency, URI> downloader,
final CommandLineOptions options,
DependencyTools dependencyTools)
Expand Down Expand Up @@ -325,9 +338,6 @@ private void writeResults(
final Map<MavenCoordinate, TargetType> targetTypeMap = resolutions
.stream()
.collect(Collectors.toMap(r -> r.resolution().rootDependency(), ResolutionOutput::targetType));
final Map<MavenCoordinate, Boolean> testOnlyMap = resolutions
.stream()
.collect(Collectors.toMap(r -> r.resolution().rootDependency(), ResolutionOutput::testOnly));
final Function<Dependency, TargetsBuilder> ruleMapper = buildRuleMapper(
downloader,
dep -> targetTypeMap.getOrDefault(dep, TargetType.naive),
Expand All @@ -344,7 +354,7 @@ private void writeResults(
resolvedDependencies.stream()
.peek(d -> timer.taskDone(dependencyTools.mavenCoordinates(d)))
.map(dependency -> Dependency.builder(dependency)
.testOnly(testOnlyMap.getOrDefault(dependency.mavenCoordinate(), false))
.testOnly(testOnlyDeps.test(dependency.mavenCoordinate()))
.build())
.map(dependency -> new TargetsToWrite(
fileImporter.buildTargets(dependency, dependencyTools),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package net.evendanan.bazel.mvn.merger;

import net.evendanan.bazel.mvn.api.model.Dependency;
import net.evendanan.bazel.mvn.api.model.MavenCoordinate;
import net.evendanan.bazel.mvn.api.model.Resolution;

import java.util.Collection;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Predicate;
import java.util.stream.Collectors;

public class TestOnlyMarker {
public static Predicate<MavenCoordinate> mark(Collection<Resolution> resolutions, Set<MavenCoordinate> initialTestOnlyMap) {
if (initialTestOnlyMap.isEmpty())
return c -> initialTestOnlyMap.contains(strippedDownCoord(c));

final Set<MavenCoordinate> marked = initialTestOnlyMap
.stream()
.map(TestOnlyMarker::strippedDownCoord)
.collect(Collectors.toSet());

resolutions.forEach(resolution -> {
if (isTestOnlySubTree(marked, resolution.allResolvedDependencies(), resolution.rootDependency()) && !initialTestOnlyMap.contains(resolution.rootDependency())) {
throw new GraphVerifications.InvalidGraphException("TestOnlyMarker", resolution.rootDependency(), "Dependency has test-only dependencies but is not a test-only artifact!");
}
});
return c -> marked.contains(strippedDownCoord(c));
}

private static boolean isTestOnlySubTree(Set<MavenCoordinate> marked, Collection<Dependency> allResolvedDependencies, MavenCoordinate current) {
final Dependency dependency = allResolvedDependencies
.stream()
.filter(d -> d.mavenCoordinate().equals(current))
.findFirst()
.get();
/*NOTE: we have to go over the entire list, so the subtrees will be marked as wall*/
final AtomicBoolean foundTestOnly = new AtomicBoolean(false);
dependency.dependencies().forEach(child -> {
if (isTestOnlySubTree(marked, allResolvedDependencies, child))
foundTestOnly.set(true);
});

if (foundTestOnly.get()) {
marked.add(strippedDownCoord(current));
return true;
} else {
return marked.contains(strippedDownCoord(current));
}
}

private static MavenCoordinate strippedDownCoord(MavenCoordinate coordinate) {
return MavenCoordinate.create(coordinate.groupId(), coordinate.artifactId(), "", "");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
package net.evendanan.bazel.mvn.merger;

import net.evendanan.bazel.mvn.api.model.Dependency;
import net.evendanan.bazel.mvn.api.model.MavenCoordinate;
import net.evendanan.bazel.mvn.api.model.Resolution;

import org.junit.Assert;
import org.junit.Test;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.function.Predicate;

public class TestOnlyMarkerTest {

@Test
public void testMarkingHappyPath() {
final Dependency junitDep = Dependency.builder()
.mavenCoordinate(MavenCoordinate.create("junit", "junit", "2", "jar"))
.build();
final Resolution junit = Resolution.create(
junitDep.mavenCoordinate(),
Collections.singleton(junitDep));

final Resolution appTestUtil = Resolution.create(
MavenCoordinate.create("util", "junit-helper", "1", "jar"),
Arrays.asList(
junitDep,
Dependency.builder()
.mavenCoordinate(MavenCoordinate.create("a", "b", "1", "jar"))
.dependencies(Collections.singleton(junitDep.mavenCoordinate()))
.build(),
Dependency.builder()
.mavenCoordinate(MavenCoordinate.create("util", "junit-helper", "1", "jar"))
.dependencies(Collections.singleton(MavenCoordinate.create("a", "b", "1", "jar")))
.build())
);

final HashSet<MavenCoordinate> initialTestOnlyMap = new HashSet<>(Arrays.asList(junitDep.mavenCoordinate(), appTestUtil.rootDependency()));
final Predicate<MavenCoordinate> marked = TestOnlyMarker.mark(
Arrays.asList(junit, appTestUtil),
initialTestOnlyMap);

Assert.assertTrue(marked.test(junit.rootDependency()));
Assert.assertTrue(marked.test(appTestUtil.rootDependency()));
Assert.assertTrue(marked.test(MavenCoordinate.create("a", "b", "1", "jar")));
//testing strip-down
Assert.assertTrue(marked.test(MavenCoordinate.create("a", "b", "2", "jar")));
Assert.assertTrue(marked.test(MavenCoordinate.create("a", "b", "1", "aar")));
//random
Assert.assertFalse(marked.test(MavenCoordinate.create("b", "b", "1", "jar")));
}

@Test
public void testMarkingHappyPathWithNonTestOnly() {
final Dependency junitDep = Dependency.builder()
.mavenCoordinate(MavenCoordinate.create("junit", "junit", "2", "jar"))
.build();
final Resolution junit = Resolution.create(
junitDep.mavenCoordinate(),
Collections.singleton(junitDep));

final Resolution appUtil = Resolution.create(
MavenCoordinate.create("util", "helper", "1", "jar"),
Arrays.asList(
junitDep,
Dependency.builder()
.mavenCoordinate(MavenCoordinate.create("a", "b", "1", "jar"))
.dependencies(Collections.emptyList())
.build(),
Dependency.builder()
.mavenCoordinate(MavenCoordinate.create("util", "helper", "1", "jar"))
.dependencies(Collections.singleton(MavenCoordinate.create("a", "b", "1", "jar")))
.build())
);

final Set<MavenCoordinate> initialTestOnlyMap = Collections.singleton(junitDep.mavenCoordinate());
final Predicate<MavenCoordinate> marked = TestOnlyMarker.mark(
Arrays.asList(junit, appUtil),
initialTestOnlyMap);

Assert.assertTrue(marked.test(junit.rootDependency()));
Assert.assertFalse(marked.test(appUtil.rootDependency()));
Assert.assertFalse(marked.test(MavenCoordinate.create("a", "b", "1", "jar")));
}

@Test
public void testMarkingWhenTestOnlyDepHasDifferentVersion() {
final Dependency junitDep = Dependency.builder()
.mavenCoordinate(MavenCoordinate.create("junit", "junit", "2", "jar"))
.build();
final Resolution junit = Resolution.create(
junitDep.mavenCoordinate(),
Collections.singleton(junitDep));

final Dependency junitOld = Dependency.builder(junitDep)
.mavenCoordinate(MavenCoordinate.create("junit", "junit", "1", "jar"))
.build();
final Resolution appTestUtil = Resolution.create(
MavenCoordinate.create("util", "junit-helper", "1", "jar"),
Arrays.asList(
junitOld,
Dependency.builder()
.mavenCoordinate(MavenCoordinate.create("a", "b", "1", "jar"))
.dependencies(Collections.singleton(junitOld.mavenCoordinate()))
.build(),
Dependency.builder()
.mavenCoordinate(MavenCoordinate.create("util", "junit-helper", "1", "jar"))
.dependencies(Collections.singleton(MavenCoordinate.create("a", "b", "1", "jar")))
.build())
);

final Predicate<MavenCoordinate> marked = TestOnlyMarker.mark(
Arrays.asList(junit, appTestUtil),
new HashSet<>(Arrays.asList(junitDep.mavenCoordinate(), appTestUtil.rootDependency())));

Assert.assertTrue(marked.test(junit.rootDependency()));
Assert.assertTrue(marked.test(MavenCoordinate.create("junit", "junit", "1", "jar")));
Assert.assertTrue(marked.test(appTestUtil.rootDependency()));
Assert.assertTrue(marked.test(MavenCoordinate.create("a", "b", "1", "jar")));
Assert.assertTrue(marked.test(MavenCoordinate.create("a", "b", "2", "jar")));
}

@Test(expected = GraphVerifications.InvalidGraphException.class)
public void testThrowsExceptionIfRootIsMarkedAsTestOnlyByMarker() {
final Dependency junitDep = Dependency.builder()
.mavenCoordinate(MavenCoordinate.create("junit", "junit", "1", "jar"))
.build();
final Resolution junit = Resolution.create(
junitDep.mavenCoordinate(),
Collections.singleton(junitDep));

final Resolution appTestUtil = Resolution.create(
MavenCoordinate.create("util", "junit-helper", "1", "jar"),
Arrays.asList(
junitDep,
Dependency.builder()
.mavenCoordinate(MavenCoordinate.create("util", "junit-helper", "1", "jar"))
.dependencies(Collections.singleton(junitDep.mavenCoordinate()))
.build())
);

TestOnlyMarker.mark(Arrays.asList(junit, appTestUtil), Collections.singleton(junitDep.mavenCoordinate()));
}

@Test
public void testReturnsSameIfInitialMapIsEmpty() {
final Set<MavenCoordinate> emptySet = new HashSet<>();
final Predicate<MavenCoordinate> marked = TestOnlyMarker.mark(
Collections.singleton(Resolution.create(
MavenCoordinate.create("a", "b", "1", "jar"),
Collections.emptyList())),
emptySet);

Assert.assertFalse(marked.test(MavenCoordinate.create("a", "b", "1", "jar")));
}
}

0 comments on commit 8187322

Please sign in to comment.