refactor the testbench mocks so they are more composeable (bazelbuild…

…#148) * refactor the testbench mocks so they are more composeable. * Add sources and output jar normalization tests
djfinlay · Sep 18, 2018 · 92a3cfc · 92a3cfc
1 parent 9c60df0
commit 92a3cfc
Show file tree

Hide file tree

Showing 13 changed files with 279 additions and 212 deletions.
diff --git a/kotlin/internal/repositories/BUILD.com_github_jetbrains_kotlin b/kotlin/internal/repositories/BUILD.com_github_jetbrains_kotlin
@@ -12,7 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 package(default_visibility = ["//visibility:public"])
-load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_import", "kt_js_import")
+
+load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_js_import", "kt_jvm_import")
 
 # Kotlin home filegroup containing everything that is needed.
 filegroup(

diff --git a/src/test/kotlin/io/bazel/kotlin/builder/BUILD b/src/test/kotlin/io/bazel/kotlin/builder/BUILD
@@ -21,16 +21,18 @@ _COMMON_DEPS = [
     "//third_party/jvm/com/google/truth",
     "//third_party/jvm/junit",
     "@io_bazel_rules_kotlin_com_google_guava_guava//jar",
+    "//third_party/jvm/com/google/code/findbugs:jsr305",
 ]
 
 java_library(
     name = "test_lib",
     testonly = 1,
     srcs = [
         "Deps.java",
-        "KotlinBuilderJsTestTask.java",
-        "KotlinBuilderJvmTestTask.java",
-        "KotlinBuilderResource.java",
+        "DirectoryType.java",
+        "KotlinAbstractTestBuilder.java",
+        "KotlinJsTestBuilder.java",
+        "KotlinJvmTestBuilder.java",
     ],
     data = [
         "//src/main/kotlin:compiler_lib",

diff --git a/src/test/kotlin/io/bazel/kotlin/builder/Deps.java b/src/test/kotlin/io/bazel/kotlin/builder/Deps.java
@@ -16,9 +16,12 @@
 package io.bazel.kotlin.builder;
 
 import com.google.auto.value.AutoValue;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
 import io.bazel.kotlin.builder.utils.BazelRunFiles;
 
+import javax.annotation.Nullable;
+import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Stream;
@@ -31,22 +34,33 @@ public abstract static class Dep {
 
     public abstract String moduleName();
 
-    public abstract Set<String> runtimeDeps();
+    public abstract List<String> runtimeDeps();
 
-    public abstract Set<String> compileJars();
+    public abstract List<String> compileJars();
+
+    @Nullable
+    public abstract String sourceJar();
+
+    @Nullable
+    public abstract String jdeps();
 
     public static Builder builder() {
-      return new AutoValue_Deps_Dep.Builder().runtimeDeps(ImmutableSet.of());
+      return new AutoValue_Deps_Dep.Builder().runtimeDeps(ImmutableList.of());
+    }
+
+    public final String singleCompileJar() {
+      Preconditions.checkState(compileJars().size() == 1);
+      return compileJars().get(0);
     }
 
     @SuppressWarnings("UnusedReturnValue")
     @AutoValue.Builder
     public abstract static class Builder {
-      public abstract Builder compileJars(Set<String> compileJars);
+      public abstract Builder compileJars(List<String> compileJars);
 
       public abstract Builder label(String label);
 
-      public abstract Builder runtimeDeps(Set<String> runtimeDeps);
+      public abstract Builder runtimeDeps(List<String> runtimeDeps);
 
       public abstract Builder moduleName(String moduleName);
 
@@ -56,6 +70,10 @@ public abstract static class Builder {
 
       abstract Dep autoBuild();
 
+      public abstract Builder sourceJar(String sourceJar);
+
+      public abstract Builder jdeps(String jdeps);
+
       public Dep build() {
         if (!moduleName().isPresent()) {
           moduleName(label());
@@ -73,7 +91,8 @@ public static Stream<String> classpathOf(Dep... dependencies) {
     public static Dep importJar(String label, String compileJar) {
       return Dep.builder()
           .label(label)
-          .compileJars(ImmutableSet.of(BazelRunFiles.resolveVerified(compileJar).getAbsolutePath()))
+          .compileJars(
+              ImmutableList.of(BazelRunFiles.resolveVerified(compileJar).getAbsolutePath()))
           .build();
     }
   }

diff --git a/src/test/kotlin/io/bazel/kotlin/builder/DirectoryType.java b/src/test/kotlin/io/bazel/kotlin/builder/DirectoryType.java
@@ -0,0 +1,38 @@
+package io.bazel.kotlin.builder;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.EnumSet;
+
+public enum DirectoryType {
+  SOURCES("sources", Paths.get("sources")),
+  CLASSES("compiled classes", Paths.get("classes")),
+  GENERATED_CLASSES("generated classes", Paths.get("generated_classes")),
+  TEMP("temp directory", Paths.get("temp")),
+  SOURCE_GEN("generated sources directory", Paths.get("generated_sources"));
+
+  final String name;
+  final Path relativePath;
+
+  DirectoryType(String name, Path relativePath) {
+    this.name = name;
+    this.relativePath = relativePath;
+  }
+
+  Path resolve(Path root) {
+    return root.resolve(relativePath);
+  }
+
+  static void createAll(Path root, EnumSet<DirectoryType> types) {
+    for (DirectoryType instanceType : types) {
+      try {
+        Files.createDirectory(instanceType.resolve(root));
+      } catch (IOException e) {
+        throw new UncheckedIOException(instanceType.name, e);
+      }
+    }
+  }
+}
diff --git a/...kotlin/builder/KotlinBuilderResource.java → ...in/builder/KotlinAbstractTestBuilder.java b/...kotlin/builder/KotlinBuilderResource.java → ...in/builder/KotlinAbstractTestBuilder.java
@@ -17,17 +17,16 @@
 
 import io.bazel.kotlin.builder.toolchain.CompilationStatusException;
 import io.bazel.kotlin.builder.utils.CompilationTaskContext;
-import io.bazel.kotlin.model.CompilationTaskInfo;
-import io.bazel.kotlin.model.KotlinToolchainInfo;
-import io.bazel.kotlin.model.Platform;
-import io.bazel.kotlin.model.RuleKind;
-import org.junit.rules.ExternalResource;
+import io.bazel.kotlin.model.*;
 
 import java.io.*;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.*;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
@@ -37,42 +36,17 @@
 import static com.google.common.truth.Truth.assertWithMessage;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
-public abstract class KotlinBuilderResource<T> extends ExternalResource {
-  public enum DirectoryType {
-    INSTANCE_ROOT("test root", null),
-    EXTERNAL("bazel external directory", null),
-    /** The rest of the paths are instance relative. */
-    SOURCES("sources", Paths.get("sources")),
-    CLASSES("compiled classes", Paths.get("classes")),
-    GENERATED_CLASSES("generated classes", Paths.get("generated_classes")),
-    TEMP("temp directory", Paths.get("temp")),
-    SOURCE_GEN("generated sources directory", Paths.get("generated_sources"));
-
-    private static final EnumSet<DirectoryType> INSTANCE_TYPES =
-        EnumSet.of(SOURCES, CLASSES, SOURCE_GEN, GENERATED_CLASSES, TEMP);
-
-    final String name;
-    private final Path relativePath;
-
-    DirectoryType(String name, Path relativePath) {
-      this.name = name;
-      this.relativePath = relativePath;
-    }
-  }
-
-  private static final Path
-      BAZEL_TEST_DIR = Paths.get(Objects.requireNonNull(System.getenv("TEST_TMPDIR"))),
-      EXTERNAL_PATH = Paths.get("external");
+abstract class KotlinAbstractTestBuilder<T> {
+  private static final Path BAZEL_TEST_DIR =
+      Paths.get(Objects.requireNonNull(System.getenv("TEST_TMPDIR")));
 
   private static final AtomicInteger counter = new AtomicInteger(0);
-
+  private final CompilationTaskInfo.Builder infoBuilder = CompilationTaskInfo.newBuilder();
   private Path instanceRoot = null;
   private String label = null;
   private List<String> outLines = null;
 
-  KotlinBuilderResource() {}
-
-  abstract CompilationTaskInfo.Builder infoBuilder();
+  abstract void setupForNext(CompilationTaskInfo.Builder infoBuilder);
 
   abstract T buildTask();
 
@@ -89,11 +63,10 @@ public final List<String> outLines() {
     return outLines;
   }
 
-  @Override
-  protected void before() throws Throwable {
+  final void resetForNext() {
     outLines = null;
     label = "a_test_" + counter.incrementAndGet();
-    infoBuilder()
+    infoBuilder
         .setLabel("//some/bogus:" + label())
         .setModuleName("some_bogus_module")
         .setPlatform(Platform.JVM)
@@ -111,36 +84,16 @@ protected void before() throws Throwable {
     } catch (IOException e) {
       throw new UncheckedIOException(e);
     }
-
-    for (DirectoryType instanceType : DirectoryType.INSTANCE_TYPES) {
-      try {
-        Files.createDirectory(instanceRoot.resolve(instanceType.relativePath));
-      } catch (IOException e) {
-        throw new RuntimeException("could not create instance directory: " + instanceType.name, e);
-      }
-    }
+    setupForNext(infoBuilder);
   }
 
   final Path directory(DirectoryType type) {
-    switch (type) {
-      case INSTANCE_ROOT:
-        return instanceRoot;
-      case EXTERNAL:
-        return KotlinBuilderResource.EXTERNAL_PATH;
-      case SOURCES:
-      case CLASSES:
-      case GENERATED_CLASSES:
-      case TEMP:
-      case SOURCE_GEN:
-        return instanceRoot.resolve(type.relativePath);
-      default:
-        throw new IllegalStateException(type.toString());
-    }
+    return type.resolve(instanceRoot);
   }
 
   @SuppressWarnings("unused")
   public final void setDebugTags(String... tags) {
-    infoBuilder().addAllDebug(Arrays.asList(tags));
+    infoBuilder.addAllDebug(Arrays.asList(tags));
   }
 
   final Path writeSourceFile(String filename, String[] lines) {
@@ -174,9 +127,8 @@ private <R> R runCompileTask(
   }
 
   final <R> R runCompileTask(BiFunction<CompilationTaskContext, T, R> operation) {
-    CompilationTaskInfo info = infoBuilder().build();
     T task = buildTask();
-    return runCompileTask(info, task, (ctx, t) -> operation.apply(ctx, task));
+    return runCompileTask(infoBuilder.build(), task, (ctx, t) -> operation.apply(ctx, task));
   }
 
   /**
@@ -238,7 +190,7 @@ private static Path toPlatformPath(String path) {
   }
 
   public final String toPlatform(String path) {
-    return KotlinBuilderResource.toPlatformPath(path).toString();
+    return KotlinAbstractTestBuilder.toPlatformPath(path).toString();
   }
 
   @SuppressWarnings("unused")

diff --git a/...tlin/builder/KotlinBuilderJsTestTask.java → ...l/kotlin/builder/KotlinJsTestBuilder.java b/...tlin/builder/KotlinBuilderJsTestTask.java → ...l/kotlin/builder/KotlinJsTestBuilder.java
@@ -19,32 +19,29 @@
 import io.bazel.kotlin.model.CompilationTaskInfo;
 import io.bazel.kotlin.model.JsCompilationTask;
 
-import java.nio.file.Path;
 import java.util.Arrays;
+import java.util.EnumSet;
 import java.util.List;
+import java.util.function.Consumer;
 
-public final class KotlinBuilderJsTestTask extends KotlinBuilderResource<JsCompilationTask> {
+public final class KotlinJsTestBuilder extends KotlinAbstractTestBuilder<JsCompilationTask> {
   private static final List<String> PASSTHROUGH_FLAGS =
       Arrays.asList("-source-map", "-meta-info", "-module-kind", "commonjs", "-target", "v5");
   private static final JsCompilationTask.Builder taskBuilder = JsCompilationTask.newBuilder();
   private static final KotlinBuilderComponent component =
       DaggerKotlinBuilderComponent.builder().toolchain(KotlinToolchain.createToolchain()).build();
-
-  @Override
-  CompilationTaskInfo.Builder infoBuilder() {
-    return taskBuilder.getInfoBuilder();
-  }
+  private static final EnumSet<DirectoryType> ALL_DIRECTORY_TYPES =
+      EnumSet.of(DirectoryType.SOURCES);
 
   @Override
   JsCompilationTask buildTask() {
     return taskBuilder.build();
   }
 
   @Override
-  protected final void before() throws Throwable {
-    taskBuilder.clear();
-    super.before();
-
+  void setupForNext(CompilationTaskInfo.Builder infoBuilder) {
+    taskBuilder.clear().setInfo(infoBuilder);
+    DirectoryType.createAll(instanceRoot(), ALL_DIRECTORY_TYPES);
     taskBuilder.addAllPassThroughFlags(PASSTHROUGH_FLAGS);
     taskBuilder
         .getOutputsBuilder()
@@ -53,12 +50,17 @@ protected final void before() throws Throwable {
         .setJs(instanceRoot().resolve(label() + ".js").toAbsolutePath().toString());
   }
 
-  public void addSource(String filename, String... lines) {
-    Path sourcePath = super.writeSourceFile(filename, lines);
-    taskBuilder.getInputsBuilder().addKotlinSources(sourcePath.toString());
+  public final class TaskBuilder {
+    public void addSource(String filename, String... lines) {
+      taskBuilder.getInputsBuilder().addKotlinSources(writeSourceFile(filename, lines).toString());
+    }
   }
 
-  public void runCompilationTask() {
+  private final TaskBuilder taskBuilderInstance = new TaskBuilder();
+
+  public void runCompilationTask(Consumer<TaskBuilder> setup) {
+    resetForNext();
+    setup.accept(taskBuilderInstance);
     runCompileTask(
         (taskContext, task) -> {
           component.jsTaskExecutor().execute(taskContext, task);