[ggj][bazel][goldens] fix: refactor and fix bazel golden_update rules (#363)

* fix: use imperative bazel rule names

* fix: refactor and fix bazel golden_update rules

Author: miraleung

BUILD.bazel

@@ -46,50 +46,6 @@ java_binary(
     ],
 )
 
-# JUnit runner binary, this is used to generate test output for updating goldens files.
-# Run `bazel run testTarget_update` will trigger this runner.
-java_binary(
-    name = "golden_update_junit_runner",
-    srcs = [
-        "//src/test/java/com/google/api/generator/gapic/dummy:dummy_files",
-        "//src/test/java/com/google/api/generator/engine:engine_files",
-        "//src/test/java/com/google/api/generator/gapic/composer:composer_files",
-        "//src/test/java/com/google/api/generator/test/framework:framework_files",
-    ],
-    data = [
-        "//src/test/java/com/google/api/generator/gapic/dummy/goldens:goldens_files",
-        "//src/test/java/com/google/api/generator/engine/goldens:goldens_files",
-        "//src/test/java/com/google/api/generator/gapic/composer/goldens:goldens_files",
-        "//src/test/java/com/google/api/generator/gapic/testdata:gapic_config_files",
-        "//src/test/java/com/google/api/generator/gapic/testdata:service_config_files",
-    ],
-    jvm_flags = ["-Xmx512m"],
-    main_class = "com.google.api.generator.test.framework.SingleJUnitTestRunner",
-    visibility = ["//visibility:public"],
-    deps = [
-        "//src/main/java/com/google/api/generator/engine/ast",
-        "//src/main/java/com/google/api/generator/engine/writer",
-        "//src/test/java/com/google/api/generator/test/framework",
-        "//src/main/java/com/google/api/generator/engine/lexicon",
-        "//src/main/java/com/google/api/generator/gapic/composer",
-        "//src/main/java/com/google/api/generator/gapic/model",
-        "//src/main/java/com/google/api/generator/gapic/protoparser",
-        "//src/test/java/com/google/api/generator/gapic/testdata:showcase_java_proto",
-        "//src/test/java/com/google/api/generator/gapic/testdata:testgapic_java_proto",
-        "//src/test/java/com/google/api/generator/gapic/composer:common_resources_java_proto",
-        "@com_google_guava_guava//:com_google_guava_guava",
-        "@com_google_api_gax_java//gax",
-        "@com_google_googleapis//google/logging/v2:logging_java_proto",
-        "@com_google_googleapis//google/pubsub/v1:pubsub_java_proto",
-        "@com_google_googleapis//google/rpc:rpc_java_proto",
-        "@com_google_protobuf//:protobuf_java",
-        "//:service_config_java_proto",
-        "@com_google_truth_truth//jar",
-        "@io_github_java_diff_utils//jar",
-        "@junit_junit//jar",
-        "@org_hamcrest_hamcrest_core//jar",
-    ],
-)
 
 # google-java-format
 java_binary(

rules_bazel/java/java_diff_test.bzl

@@ -1,87 +1,84 @@
-def _junit_output_impl(ctx):
-    test_class_name = ctx.attr.test_class_name
+def _overwrite_golden_impl(ctx):
+    test_class = ctx.attr.test_class
     inputs = ctx.files.srcs
-    output = ctx.outputs.output
+    goldens_output_zip = ctx.outputs.goldens_output_zip
     test_runner = ctx.executable.test_runner
 
-    command = """
-    mkdir local_tmp  
-    TEST_OUTPUT_HOME="$(pwd)/local_tmp" \
-    {test_runner_path} $@
-    cd local_tmp 
+    # Generate the goldens from tests.
+    generate_goldens_script = """
+    mkdir local_tmp
+    TEST_OUTPUT_HOME="$(pwd)/local_tmp" {test_runner_path} $@
+    cd local_tmp
     # Zip all files under local_tmp with all nested parent folders except for local_tmp itself.
     # Zip files because there are cases that one Junit test can produce multiple goldens.
-    zip -r ../{output} .
+    zip -r ../{goldens_output_zip} .
     """.format(
         test_runner_path = test_runner.path,
-        output=output.path,
+        goldens_output_zip = goldens_output_zip.path,
     )
 
     ctx.actions.run_shell(
         inputs = inputs,
-        outputs = [output],
-        arguments = [test_class_name],
+        outputs = [goldens_output_zip],
+        arguments = [test_class],
         tools = [test_runner],
-        command = command,
+        command = generate_goldens_script,
     )
 
-junit_output_zip = rule(
-    attrs = {
-        "test_class_name": attr.string(mandatory=True),
-        "srcs": attr.label_list(
-            allow_files = True,
-            mandatory = True,
-        ),
-        "test_runner": attr.label(
-            mandatory = True,
-            executable = True,
-            cfg = "host",
-        ),
-    },
-    outputs = {
-        "output": "%{name}.zip",
-    },
-    implementation = _junit_output_impl,  
-)
-    
-def _overwrite_golden_impl(ctx):
-    script_content = """
+    # Overwrite the goldens.
+    golden_update_script_content = """
     #!/bin/bash
     cd ${{BUILD_WORKSPACE_DIRECTORY}}
-    unzip -ao {unit_test_results} -d src/test/java
+    unzip -ao {goldens_output_zip} -d src/test/java
     """.format(
-        unit_test_results = ctx.file.unit_test_results.path,
+        goldens_output_zip = goldens_output_zip.path,
     )
     ctx.actions.write(
-        output = ctx.outputs.bin,
-        content = script_content,
+        output = ctx.outputs.golden_update_script,
+        content = golden_update_script_content,
         is_executable = True,
     )
-    return [DefaultInfo(executable = ctx.outputs.bin)]
-
+    return [DefaultInfo(executable = ctx.outputs.golden_update_script)]
 
 overwrite_golden = rule(
     attrs = {
-        "unit_test_results": attr.label(
+        "test_class": attr.string(mandatory = True),
+        "srcs": attr.label_list(
+            allow_files = True,
             mandatory = True,
-            allow_single_file = True),
+        ),
+        "test_runner": attr.label(
+            mandatory = True,
+            executable = True,
+            cfg = "host",
+        ),
     },
     outputs = {
-        "bin": "%{name}.sh",
+        "goldens_output_zip": "%{name}.zip",
+        "golden_update_script": "%{name}.sh",
     },
     executable = True,
     implementation = _overwrite_golden_impl,
 )
 
-def golden_update(name, test_class_name, srcs):
-    junit_output_name = "%s_output" % name
-    junit_output_zip(
-        name = junit_output_name,
-        test_class_name = test_class_name,
-        test_runner = "//:golden_update_junit_runner",
+def golden_update(
+        name,
+        srcs,
+        test_class,
+        data = [],
+        deps = []):
+    golden_junit_runner_name = "%s_junit_runner" % name
+    native.java_binary(
+        name = golden_junit_runner_name,
         srcs = srcs,
+        data = data,
+        main_class = "com.google.api.generator.test.framework.SingleJUnitTestRunner",
+        deps = ["//src/test/java/com/google/api/generator/test/framework:junit_runner"] + deps,
     )
+
     overwrite_golden(
         name = name,
-        unit_test_results = ":%s" % junit_output_name
+        test_class = test_class,
+        test_runner = ":%s" % golden_junit_runner_name,
+        srcs = srcs + data,
     )

src/test/java/com/google/api/generator/engine/BUILD.bazel

@@ -15,27 +15,30 @@ TESTS = [
     "JavaCodeGeneratorTest",
 ]
 
+TEST_DEPS = [
+    "//src/main/java/com/google/api/generator/engine/ast",
+    "//src/main/java/com/google/api/generator/engine/writer",
+    "//src/test/java/com/google/api/generator/test/framework:asserts",
+    "//src/test/java/com/google/api/generator/test/framework:utils",
+    "@junit_junit//jar",
+]
+
 [java_test(
     name = test_name,
     srcs = ["{0}.java".format(test_name)],
     data = ["//src/test/java/com/google/api/generator/engine/goldens:goldens_files"],
     test_class = "com.google.api.generator.engine.{0}".format(test_name),
-    deps = [
-        "//src/main/java/com/google/api/generator/engine/ast",
-        "//src/main/java/com/google/api/generator/engine/writer",
-        "//src/test/java/com/google/api/generator/test/framework",
-        "@junit_junit//jar",
-    ],
+    deps = TEST_DEPS,
 ) for test_name in TESTS]
 
 TEST_CLASS_NAME = "com.google.api.generator.engine.JavaCodeGeneratorTest"
 
 # Run `bazel run src/test/java/com/google/api/generator/engine:JavaCodeGeneratorTest_update`
 # to update goldens as expected generated code.
-golden_update(
-    name = "JavaCodeGeneratorTest_update",
-    srcs = [
-        ":engine_files",
-    ],
-    test_class_name = TEST_CLASS_NAME,
-)
+[golden_update(
+    name = "{0}_update".format(test_name),
+    srcs = ["{0}.java".format(test_name)],
+    data = ["//src/test/java/com/google/api/generator/engine/goldens:goldens_files"],
+    test_class = "com.google.api.generator.engine.{0}".format(test_name),
+    deps = TEST_DEPS,
+) for test_name in TESTS]

src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel

@@ -2,35 +2,47 @@ load("//:rules_bazel/java/java_diff_test.bzl", "golden_update")
 
 package(default_visibility = ["//visibility:public"])
 
-TESTS = [
+UPDATE_GOLDENS_TESTS = [
     "BatchingDescriptorComposerTest",
     "ComposerTest",
-    "DefaultValueComposerTest",
     "GrpcServiceCallableFactoryClassComposerTest",
     "GrpcServiceStubClassComposerTest",
     "MockServiceClassComposerTest",
     "MockServiceImplClassComposerTest",
     "ResourceNameHelperClassComposerTest",
+    "ServiceSettingsClassComposerTest",
+    "ServiceStubSettingsClassComposerTest",
+    "ServiceStubClassComposerTest",
+]
+
+TESTS = UPDATE_GOLDENS_TESTS + [
+    "DefaultValueComposerTest",
     "ResourceNameTokenizerTest",
     "RetrySettingsComposerTest",
     "ServiceClientClassComposerTest",
     "ServiceClientTestClassComposerTest",
-    "ServiceSettingsClassComposerTest",
-    "ServiceStubSettingsClassComposerTest",
-    "ServiceStubClassComposerTest",
 ]
 
-UPDATE_GOLDENS_TESTS = [
-    "BatchingDescriptorComposerTest",
-    "ComposerTest",
-    "GrpcServiceCallableFactoryClassComposerTest",
-    "GrpcServiceStubClassComposerTest",
-    "MockServiceClassComposerTest",
-    "MockServiceImplClassComposerTest",
-    "ResourceNameHelperClassComposerTest",
-    "ServiceSettingsClassComposerTest",
-    "ServiceStubSettingsClassComposerTest",
-    "ServiceStubClassComposerTest",
+TEST_DEPS = [
+    ":common_resources_java_proto",
+    "//:service_config_java_proto",
+    "//src/main/java/com/google/api/generator/engine/ast",
+    "//src/main/java/com/google/api/generator/engine/writer",
+    "//src/main/java/com/google/api/generator/gapic/composer",
+    "//src/test/java/com/google/api/generator/test/framework:asserts",
+    "//src/test/java/com/google/api/generator/test/framework:utils",
+    "//src/main/java/com/google/api/generator/gapic/model",
+    "//src/main/java/com/google/api/generator/gapic/protoparser",
+    "//src/test/java/com/google/api/generator/gapic/testdata:showcase_java_proto",
+    "//src/test/java/com/google/api/generator/gapic/testdata:testgapic_java_proto",
+    "@com_google_api_gax_java//gax",
+    "@com_google_googleapis//google/logging/v2:logging_java_proto",
+    "@com_google_googleapis//google/pubsub/v1:pubsub_java_proto",
+    "@com_google_googleapis//google/rpc:rpc_java_proto",
+    "@com_google_guava_guava",
+    "@com_google_protobuf//:protobuf_java",
+    "@com_google_truth_truth//jar",
+    "@junit_junit//jar",
 ]
 
 filegroup(
@@ -55,7 +67,7 @@ java_proto_library(
 [java_test(
     name = test_name,
     srcs = [
-        "{0}.java".format(test_name), 
+        "{0}.java".format(test_name),
         "ComposerConstants.java",
     ],
     data = [
@@ -64,40 +76,25 @@ java_proto_library(
         "//src/test/java/com/google/api/generator/gapic/testdata:service_config_files",
     ],
     test_class = "com.google.api.generator.gapic.composer.{0}".format(test_name),
-    deps = [
-        ":common_resources_java_proto",
-        "//:service_config_java_proto",
-        "//src/main/java/com/google/api/generator/engine/ast",
-        "//src/main/java/com/google/api/generator/engine/writer",
-        "//src/main/java/com/google/api/generator/gapic/composer",
-        "//src/test/java/com/google/api/generator/test/framework",
-        "//src/main/java/com/google/api/generator/gapic/model",
-        "//src/main/java/com/google/api/generator/gapic/protoparser",
-        "//src/test/java/com/google/api/generator/gapic/testdata:showcase_java_proto",
-        "//src/test/java/com/google/api/generator/gapic/testdata:testgapic_java_proto",
-        "@com_google_api_gax_java//gax",
-        "@com_google_googleapis//google/logging/v2:logging_java_proto",
-        "@com_google_googleapis//google/pubsub/v1:pubsub_java_proto",
-        "@com_google_googleapis//google/rpc:rpc_java_proto",
-        "@com_google_protobuf//:protobuf_java",
-        "@com_google_truth_truth//jar",
-        "@junit_junit//jar",
-    ],
+    deps = TEST_DEPS,
 ) for test_name in TESTS]
 
-
-
 TEST_CLASS_DIR = "com.google.api.generator.gapic.composer."
 # Run `bazel run src/test/java/com/google/api/generator/gapic/composer:testTargetName_update`
 # to update goldens as expected generated code.
 # `ServiceClient*` tests are not supported now since they are still in active development.
 
 [golden_update(
-    name = test_name + "_update",
+    name = "{0}_update".format(test_name),
     srcs = [
-        ":composer_files",
+        "{0}.java".format(test_name),
+        "ComposerConstants.java",
+    ],
+    data = [
+        "//src/test/java/com/google/api/generator/gapic/composer/goldens:goldens_files",
         "//src/test/java/com/google/api/generator/gapic/testdata:gapic_config_files",
         "//src/test/java/com/google/api/generator/gapic/testdata:service_config_files",
     ],
-    test_class_name = TEST_CLASS_DIR + test_name,
-) for test_name in UPDATE_GOLDENS_TESTS] 
+    test_class = "com.google.api.generator.gapic.composer.{0}".format(test_name),
+    deps = TEST_DEPS,
+) for test_name in UPDATE_GOLDENS_TESTS]