Convert TestingModule to SkylarkCallable

PiperOrigin-RevId: 203817931
Change-Id: I9dda8d5ba47f805a0a4892bb4a8ab3b0e808e47c

copybara/integration/cram.bzl

@@ -12,27 +12,30 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-def cram_test(name, srcs, deps=[]):
+def cram_test(name, srcs, deps = []):
     for s in srcs:
-        testname = name + '_' + s
-        script = testname + '_script.sh'
+        testname = name + "_" + s
+        script = testname + "_script.sh"
         gr = "_gen_" + script
+
         # This genrule is a kludge, and at some point we should move
         # to a real rule
         # (http://bazel.io/docs/skylark/rules.html). What this does is
         # it builds a "bash script" whose first line execs cram on the
         # script. Conveniently, that first line is a comment as far as
         # cram is concerned (it's not indented at all), so everything
         # just works.
-        native.genrule(name=gr,
-                       srcs=[s],
-                       outs=[script],
-                       cmd=("echo 'exec $${TEST_SRCDIR}/copybara/integration/cram " +
-                            "--xunit-file=$$XML_OUTPUT_FILE $$0' > \"$@\" ; " +
-                            "cat $(SRCS) >> \"$@\""),
+        native.genrule(
+            name = gr,
+            srcs = [s],
+            outs = [script],
+            cmd = ("echo 'exec $${TEST_SRCDIR}/copybara/integration/cram " +
+                   "--xunit-file=$$XML_OUTPUT_FILE $$0' > \"$@\" ; " +
+                   "cat $(SRCS) >> \"$@\""),
         )
 
-        native.sh_test(name=testname,
-                       srcs=[script],
-                       data=["//copybara/integration:cram"] + deps,
+        native.sh_test(
+            name = testname,
+            srcs = [script],
+            data = ["//copybara/integration:cram"] + deps,
         )

java/com/google/copybara/docs.bzl

@@ -15,35 +15,36 @@
 """Bazel rule to generate copybara reference docs."""
 
 def _doc_impl(ctx):
-  jars = []
-  for dep in ctx.attr.deps:
-    for jar in dep.java.transitive_source_jars:
-      jars.append(jar)
-  tmp = ctx.new_file("tmp.md")
-  ctx.actions.run(
-      inputs = [ctx.executable._doc_tool] + jars,
-      outputs = [tmp],
-      progress_message = "Generating reference documentation for %s" % ctx.label,
-      use_default_shell_env = True,
-      executable = ctx.executable._doc_tool.path,
-      arguments = [tmp.path] + [f.path for f in jars],
-  )
-  # If suffix file exists, concat, copy otherwise
-  if ctx.attr.suffix_file != None:
-    ctx.actions.run_shell(
-        inputs = [ctx.files.suffix_file[0], tmp],
-        outputs = [ctx.outputs.out],
-        progress_message = "Appending suffix from %s" % ctx.files.suffix_file,
-        command = "cat %s %s >> %s" %  (tmp.path, ctx.files.suffix_file[0].path, ctx.outputs.out.path),
-    )
-  else:
+    jars = []
+    for dep in ctx.attr.deps:
+        for jar in dep.java.transitive_source_jars:
+            jars.append(jar)
+    tmp = ctx.new_file("tmp.md")
     ctx.actions.run(
-        inputs = [tmp],
-        outputs = [ctx.outputs.out],
-        executable = "/bin/cp",
-        arguments = [tmp.path, ctx.outputs.out.path],
+        inputs = [ctx.executable._doc_tool] + jars,
+        outputs = [tmp],
+        progress_message = "Generating reference documentation for %s" % ctx.label,
+        use_default_shell_env = True,
+        executable = ctx.executable._doc_tool.path,
+        arguments = [tmp.path] + [f.path for f in jars],
     )
 
+    # If suffix file exists, concat, copy otherwise
+    if ctx.attr.suffix_file != None:
+        ctx.actions.run_shell(
+            inputs = [ctx.files.suffix_file[0], tmp],
+            outputs = [ctx.outputs.out],
+            progress_message = "Appending suffix from %s" % ctx.files.suffix_file,
+            command = "cat %s %s >> %s" % (tmp.path, ctx.files.suffix_file[0].path, ctx.outputs.out.path),
+        )
+    else:
+        ctx.actions.run(
+            inputs = [tmp],
+            outputs = [ctx.outputs.out],
+            executable = "/bin/cp",
+            arguments = [tmp.path, ctx.outputs.out.path],
+        )
+
 # Generates documentation by scanning the transitive set of dependencies of a Java binary.
 doc_generator = rule(
     attrs = {
@@ -57,7 +58,7 @@ doc_generator = rule(
             allow_files = True,
             default = Label("//java/com/google/copybara:doc_skylark.sh"),
         ),
-        "suffix_file": attr.label(mandatory=False, allow_files=True, single_file=True)
+        "suffix_file": attr.label(mandatory = False, allow_files = True, single_file = True),
     },
     outputs = {"out": "%{name}.md"},
     implementation = _doc_impl,

java/com/google/copybara/testing/SkylarkTestExecutor.java

@@ -51,7 +51,7 @@ public class SkylarkTestExecutor {
   private final Map<String, byte[]> extraConfigFiles = new HashMap<>();
   private SkylarkParser skylarkParser;
   private ModuleSupplier moduleSupplierForTest;
-  private ImmutableSet<Class<?>> testModules = ImmutableSet.of();
+  private ImmutableSet<Class<?>> staticTestModules = ImmutableSet.of();
 
   public SkylarkTestExecutor(OptionsBuilder options) {
     this(options, new ModuleSupplier(options.general.getEnvironment(),
@@ -64,8 +64,8 @@ protected SkylarkTestExecutor(OptionsBuilder options, ModuleSupplier moduleSuppl
     initParser();
   }
 
-  public SkylarkTestExecutor withStaticModules(ImmutableSet<Class<?>> testModules) {
-    this.testModules = Preconditions.checkNotNull(testModules);
+  public SkylarkTestExecutor withStaticModules(ImmutableSet<Class<?>> staticTestModules) {
+    this.staticTestModules = Preconditions.checkNotNull(staticTestModules);
     // TODO(malcon): Remove this once all the static modules are gone.
     initParser();
     return this;
@@ -236,15 +236,16 @@ private class ModuleSupplierForTest extends ModuleSupplier {
     protected ImmutableSet<Class<?>> getStaticModules() {
        return ImmutableSet.<Class<?>>builder()
            .addAll(moduleSupplier.create().getStaticModules())
-           // This is used too frequently to pass it in testModules.
-           .add(TestingModule.class)
-           .addAll(testModules)
+           .addAll(staticTestModules)
            .build();
     }
 
     @Override
     public ImmutableSet<Object> getModules(Options options) {
-      return moduleSupplier.getModules(options);
+      return ImmutableSet.builder()
+          .addAll(moduleSupplier.getModules(options))
+          .add(new TestingModule(options))
+          .build();
     }
 
     @Override

java/com/google/copybara/testing/TestingModule.java

@@ -16,105 +16,56 @@
 
 package com.google.copybara.testing;
 
-import com.google.copybara.GeneralOptions;
+import static com.google.common.base.Preconditions.checkNotNull;
+
 import com.google.copybara.Option;
 import com.google.copybara.Options;
-import com.google.copybara.config.OptionsAwareModule;
-import com.google.devtools.build.lib.skylarkinterface.Param;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
-import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature;
-import com.google.devtools.build.lib.syntax.BuiltinFunction;
-import com.google.devtools.build.lib.syntax.EvalException;
 
-/**
- * A Skylark module used by tests
- */
+/** A Skylark module used by tests */
 @SkylarkModule(
     name = "testing",
-    doc = "",
+    doc = "Module to use mock endpoints in tests.",
     category = SkylarkModuleCategory.BUILTIN)
-public class TestingModule implements OptionsAwareModule {
+public class TestingModule {
 
-  private TestingOptions testingOptions;
+  private final TestingOptions testingOptions;
 
-  @Override
-  public void setOptions(Options options) {
-    testingOptions = options.get(TestingOptions.class);
-    options.get(GeneralOptions.class);
+  public TestingModule(Options options) {
+    Options opts = checkNotNull(options, "Options cannot be null");
+    this.testingOptions = checkNotNull(opts.get(TestingOptions.class), "TestOptions not set");
   }
 
-  @SkylarkSignature(name = "origin", returnType = DummyOrigin.class,
-      doc = "A dummy origin",
-      parameters = {
-          @Param(name = "self", type = TestingModule.class, doc = "this object"),
-      },
-      objectType = TestingModule.class)
-  public static final BuiltinFunction ORIGIN = new BuiltinFunction("origin") {
-    public DummyOrigin invoke(TestingModule self) throws EvalException {
-      return self.testingOptions.origin;
-    }
-  };
+  @SkylarkCallable(name = "origin", doc = "A dummy origin")
+  public DummyOrigin origin() {
+    return testingOptions.origin;
+  }
 
-  @SkylarkSignature(name = "destination",
-      returnType = RecordsProcessCallDestination.class,
-      doc = "A dummy destination",
-      parameters = {
-          @Param(name = "self", type = TestingModule.class, doc = "this object"),
-      },
-      objectType = TestingModule.class)
-  public static final BuiltinFunction DESTINATION = new BuiltinFunction("destination") {
-    public RecordsProcessCallDestination invoke(TestingModule self) throws EvalException {
-      return self.testingOptions.destination;
-    }
-  };
+  @SkylarkCallable(name = "destination", doc = "A dummy destination")
+  public RecordsProcessCallDestination destination() {
+    return testingOptions.destination;
+  }
 
-  @SkylarkSignature(
-    name = "dummy_endpoint",
-    returnType = DummyEndpoint.class,
-    doc = "A dummy feedback endpoint",
-    parameters = {
-      @Param(name = "self", type = TestingModule.class, doc = "this object"),
-    },
-    objectType = TestingModule.class
-  )
-  public static final BuiltinFunction FEEDBACK_ENDPOINT =
-      new BuiltinFunction("dummy_endpoint") {
-        public DummyEndpoint invoke(TestingModule self) throws EvalException {
-          return self.testingOptions.feedbackTrigger;
-        }
-      };
+  @SkylarkCallable(name = "dummy_endpoint", doc = "A dummy feedback endpoint")
+  public DummyEndpoint dummyEndpoint() {
+    return testingOptions.feedbackTrigger;
+  }
 
-  @SkylarkSignature(
-      name = "dummy_trigger",
-      returnType = DummyTrigger.class,
-      doc = "A dummy feedback trigger",
-      parameters = {
-        @Param(name = "self", type = TestingModule.class, doc = "this object"),
-      },
-      objectType = TestingModule.class)
-  public static final BuiltinFunction FEEDBACK_TRIGGER =
-      new BuiltinFunction("dummy_trigger") {
-        public DummyTrigger invoke(TestingModule self) throws EvalException {
-          return self.testingOptions.feedbackTrigger;
-        }
-      };
+  @SkylarkCallable(name = "dummy_trigger", doc = "A dummy feedback trigger")
+  public DummyTrigger dummyTrigger() {
+    return testingOptions.feedbackTrigger;
+  }
 
-  @SkylarkSignature(
-      name = "dummy_checker",
-      returnType = DummyChecker.class,
-      doc = "A dummy checker",
-      parameters = {
-        @Param(name = "self", type = TestingModule.class, doc = "this object"),
-      },
-      objectType = TestingModule.class)
-  public static final BuiltinFunction CHECKER =
-      new BuiltinFunction("dummy_checker") {
-        public DummyChecker invoke(TestingModule self) throws EvalException {
-          return self.testingOptions.checker;
-        }
-      };
+  @SkylarkCallable(name = "dummy_checker", doc = "A dummy checker")
+  public DummyChecker dummyChecker() {
+    return testingOptions.checker;
+  }
 
+  /**
+   * Holder for options to adjust this module's behavior to the needs of a test.
+   */
   public final static class TestingOptions implements Option {
 
     public DummyOrigin origin;

javatests/com/google/copybara/test.bzl

@@ -12,29 +12,31 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-def all_tests(tests, deps, tags=[], shard_count=1, data = []):
-  for file in tests:
-    # TODO(malcon): Skip files not ending as *Test.java
-    relative_target = file[:-5]
-    suffix = relative_target.replace("/", ".")
-    pos = native.package_name().rfind("javatests/") + len("javatests/")
-    test_class = native.package_name()[pos:].replace('/', '.',) + '.' + suffix
-    native.java_test(
-        name = file[:-5],
-        srcs = [file],
-        javacopts = [
-            "-Xlint:unchecked", "-source", "1.8",
-            "-Xep:FutureReturnValueIgnored:OFF",
-        ],
-        test_class = test_class,
-        data = data,
-        deps = deps + [
-            # These deps are automatically included with Bazel, but not with the
-            # internal BUILD system. So add them explicitly here.
-            "//third_party:guava",
-            "//third_party:jsr305",
-            "//third_party:junit",
-        ],
-        tags = tags,
-      shard_count = shard_count,
-    )
+def all_tests(tests, deps, tags = [], shard_count = 1, data = []):
+    for file in tests:
+        # TODO(malcon): Skip files not ending as *Test.java
+        relative_target = file[:-5]
+        suffix = relative_target.replace("/", ".")
+        pos = native.package_name().rfind("javatests/") + len("javatests/")
+        test_class = native.package_name()[pos:].replace("/", ".") + "." + suffix
+        native.java_test(
+            name = file[:-5],
+            srcs = [file],
+            javacopts = [
+                "-Xlint:unchecked",
+                "-source",
+                "1.8",
+                "-Xep:FutureReturnValueIgnored:OFF",
+            ],
+            test_class = test_class,
+            data = data,
+            deps = deps + [
+                # These deps are automatically included with Bazel, but not with the
+                # internal BUILD system. So add them explicitly here.
+                "//third_party:guava",
+                "//third_party:jsr305",
+                "//third_party:junit",
+            ],
+            tags = tags,
+            shard_count = shard_count,
+        )