From 6e9b4a51f9237a672c4c19a0aebf0851c98bdf06 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Mon, 5 Oct 2020 18:04:52 -0700 Subject: [PATCH] [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 --- BUILD.bazel | 44 --------- rules_bazel/java/java_diff_test.bzl | 97 +++++++++---------- .../google/api/generator/engine/BUILD.bazel | 29 +++--- .../api/generator/gapic/composer/BUILD.bazel | 81 ++++++++-------- .../api/generator/gapic/dummy/BUILD.bazel | 33 ++++--- .../api/generator/test/framework/BUILD.bazel | 22 ++++- 6 files changed, 138 insertions(+), 168 deletions(-) diff --git a/BUILD.bazel b/BUILD.bazel index 245547c69f..a767045a39 100644 --- a/BUILD.bazel +++ b/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( diff --git a/rules_bazel/java/java_diff_test.bzl b/rules_bazel/java/java_diff_test.bzl index d6290590ce..626c33b251 100644 --- a/rules_bazel/java/java_diff_test.bzl +++ b/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, ) diff --git a/src/test/java/com/google/api/generator/engine/BUILD.bazel b/src/test/java/com/google/api/generator/engine/BUILD.bazel index 1bcbd20f5f..f1b9c96ae0 100644 --- a/src/test/java/com/google/api/generator/engine/BUILD.bazel +++ b/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] diff --git a/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel index 6e02d520d4..9211d8b5a1 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/BUILD.bazel +++ b/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] diff --git a/src/test/java/com/google/api/generator/gapic/dummy/BUILD.bazel b/src/test/java/com/google/api/generator/gapic/dummy/BUILD.bazel index 3caa66fb99..0a4ae41759 100644 --- a/src/test/java/com/google/api/generator/gapic/dummy/BUILD.bazel +++ b/src/test/java/com/google/api/generator/gapic/dummy/BUILD.bazel @@ -6,6 +6,14 @@ TESTS = [ "FileDiffInfraDummyTest", ] +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", +] + filegroup( name = "dummy_files", srcs = ["{0}.java".format(f) for f in TESTS], @@ -14,26 +22,19 @@ filegroup( [java_test( name = test_name, srcs = ["{0}.java".format(test_name)], - data = glob(["goldens/*.golden"]), + data = ["//src/test/java/com/google/api/generator/gapic/dummy/goldens:goldens_files"], test_class = "com.google.api.generator.gapic.dummy.{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.gapic.dummy.FileDiffInfraDummyTest" # Run `bazel run src/test/java/com/google/api/generator/gapic/dummy:FileDiffInfraDummyTest_update` # to update goldens as expected generated code. -golden_update( - name = "FileDiffInfraDummyTest_update", - srcs = [ - ":dummy_files", - "//src/test/java/com/google/api/generator/gapic/dummy/goldens:goldens_files", - "//src/test/java/com/google/api/generator/test/framework:framework_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/gapic/dummy/goldens:goldens_files"], + test_class = "com.google.api.generator.gapic.dummy.{0}".format(test_name), + deps = TEST_DEPS, +) for test_name in TESTS] diff --git a/src/test/java/com/google/api/generator/test/framework/BUILD.bazel b/src/test/java/com/google/api/generator/test/framework/BUILD.bazel index 31b6d7d6f7..5d167df4bb 100644 --- a/src/test/java/com/google/api/generator/test/framework/BUILD.bazel +++ b/src/test/java/com/google/api/generator/test/framework/BUILD.bazel @@ -6,14 +6,30 @@ filegroup( ) java_library( - name = "framework", + name = "asserts", srcs = [ - ":framework_files", + "Assert.java", + "Differ.java", ], - data = ["//src/test/java/com/google/api/generator/gapic/dummy/goldens:goldens_files"], deps = [ "@io_github_java_diff_utils//jar", "@junit_junit//jar", "@org_hamcrest_hamcrest_core//jar", ], ) + +java_library( + name = "utils", + srcs = ["Utils.java"], +) + +java_library( + name = "junit_runner", + srcs = [ + "SingleJUnitTestRunner.java", + ], + deps = [ + ":utils", + "@junit_junit//jar", + ], +)