From e8766453cf3d01ba64e84821d79749fcb3152aa8 Mon Sep 17 00:00:00 2001 From: Takuto Ikuta Date: Tue, 7 Jun 2022 02:10:13 +0000 Subject: [PATCH] Revert "Android: Rename target incremental_javac_junit_tests -> build_junit_tests" This reverts commit ad6aed8c5a28404e4305f03ab33b211cc0677175. Reason for revert: https://crbug.com/1334000 Original change's description: > Android: Rename target incremental_javac_junit_tests -> build_junit_tests > > so that we can add more build-related tests to it. > > * Uses proper java package for the test > * Moves test target to build/android/BUILD.gn so that it doesn't > reference files above it in the directory tree > * Removes dep on //base's test runner (layering violation) > > Also: > Bug: 1322737 > Change-Id: Ic8033db3743019a26a15d27403f33f59fcf6d484 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688332 > Commit-Queue: Andrew Grieve > Reviewed-by: Peter Wen > Reviewed-by: Ben Pastene > Cr-Commit-Position: refs/heads/main@{#1011221} Bug: 1322737, 1334000 Change-Id: Ic7e6a327c57815b4f9458be34e3806c3e9f7b68b No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688023 Auto-Submit: Takuto Ikuta Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/heads/main@{#1011231} --- build/PRESUBMIT.py | 6 +- build/android/BUILD.gn | 8 - .../test}/IncrementalJavacTest.java | 8 +- build/android/test/BUILD.gn | 150 ++++++++++-------- testing/buildbot/chromium.android.fyi.json | 22 +-- testing/buildbot/chromium.android.json | 54 +++---- testing/buildbot/chromium.clang.json | 20 +-- testing/buildbot/chromium.fyi.json | 22 +-- testing/buildbot/gn_isolate_map.pyl | 8 +- testing/buildbot/test_suites.pyl | 10 +- 10 files changed, 154 insertions(+), 154 deletions(-) rename build/android/{junit/src/org/chromium/build => java/test}/IncrementalJavacTest.java (81%) diff --git a/build/PRESUBMIT.py b/build/PRESUBMIT.py index 5be696d133a279..5bdd1ddd4933e7 100644 --- a/build/PRESUBMIT.py +++ b/build/PRESUBMIT.py @@ -17,9 +17,7 @@ def CheckNoBadDeps(input_api, output_api): r'(.+/)?BUILD\.gn', r'.+\.gni', ] - blocklist_pattern = input_api.re.compile( - r'^[^#]*//(?:base|third_party|components)') - allowlist_pattern = input_api.re.compile(r'^[^#]*//third_party/junit') + bad_pattern = input_api.re.compile(r'^[^#]*//(base|third_party|components)') warning_message = textwrap.dedent(""" The //build directory is meant to be as hermetic as possible so that @@ -38,7 +36,7 @@ def FilterFile(affected_file): for f in input_api.AffectedSourceFiles(FilterFile): local_path = f.LocalPath() for line_number, line in f.ChangedContents(): - if blocklist_pattern.search(line) and not allowlist_pattern.search(line): + if (bad_pattern.search(line)): problems.append('%s:%d\n %s' % (local_path, line_number, line.strip())) if problems: diff --git a/build/android/BUILD.gn b/build/android/BUILD.gn index 2475f7d40a7041..b30bd5062a669c 100644 --- a/build/android/BUILD.gn +++ b/build/android/BUILD.gn @@ -53,14 +53,6 @@ if (enable_java_templates) { # be created when creating an apk. jar_excluded_patterns += [ "*/NativeLibraries.class" ] } - - junit_binary("build_junit_tests") { - sources = [ "junit/src/org/chromium/build/IncrementalJavacTest.java" ] - deps = [ - "//build/android/test/incremental_javac_gn:no_signature_change_prebuilt_java", - "//third_party/junit", - ] - } } python_library("devil_chromium_py") { diff --git a/build/android/junit/src/org/chromium/build/IncrementalJavacTest.java b/build/android/java/test/IncrementalJavacTest.java similarity index 81% rename from build/android/junit/src/org/chromium/build/IncrementalJavacTest.java rename to build/android/java/test/IncrementalJavacTest.java index b1daa5533ff3ae..c83178aa963739 100644 --- a/build/android/junit/src/org/chromium/build/IncrementalJavacTest.java +++ b/build/android/java/test/IncrementalJavacTest.java @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -package org.chromium.build; +package test; import static org.junit.Assert.assertEquals; @@ -10,15 +10,13 @@ import org.junit.runner.RunWith; import org.robolectric.annotation.Config; -import org.chromium.testing.local.LocalRobolectricTestRunner; - -import test.NoSignatureChangeIncrementalJavacTestHelper; +import org.chromium.base.test.BaseRobolectricTestRunner; /** * Checks that build picked up changes to * {@link NoSignatureChangeIncrementalJavacTestHelper#foo()}. */ -@RunWith(LocalRobolectricTestRunner.class) +@RunWith(BaseRobolectricTestRunner.class) @Config(manifest = Config.NONE) public final class IncrementalJavacTest { @Test diff --git a/build/android/test/BUILD.gn b/build/android/test/BUILD.gn index 8e33dd37854007..9edb6bfa49578c 100644 --- a/build/android/test/BUILD.gn +++ b/build/android/test/BUILD.gn @@ -6,78 +6,90 @@ import("//build/config/android/android_nocompile.gni") import("missing_symbol_test.gni") import("nocompile_gn/nocompile_sources.gni") -group("android_nocompile_tests") { - testonly = true +if (enable_java_templates) { + group("android_nocompile_tests") { + testonly = true - # No-compile tests use an output directory dedicated to no-compile tests. - # All test suites use targets in nocompile_gn/BUILD.gn in order to share the - # same target output directory and avoid running 'gn gen' for each - # android_nocompile_test_suite(). - deps = [ - ":android_lint_tests", - ":android_lookup_dep_tests", - ] -} + # No-compile tests use an output directory dedicated to no-compile tests. + # All test suites use targets in nocompile_gn/BUILD.gn in order to share the + # same target output directory and avoid running 'gn gen' for each + # android_nocompile_test_suite(). + deps = [ + ":android_lint_tests", + ":android_lookup_dep_tests", + ] + } -android_nocompile_test_suite("android_lint_tests") { - # Depend on lint script so that the action is re-run whenever the script is modified. - pydeps = [ "//build/android/gyp/lint.pydeps" ] + android_nocompile_test_suite("android_lint_tests") { + # Depend on lint script so that the action is re-run whenever the script is modified. + pydeps = [ "//build/android/gyp/lint.pydeps" ] - tests = [ - { - target = "nocompile_gn:default_locale_lint_test" - nocompile_sources = - rebase_path(default_locale_lint_test_nocompile_sources, - "", - "nocompile_gn") - expected_compile_output_regex = "Warning:.*DefaultLocale" - }, - { - target = "nocompile_gn:new_api_lint_test" - nocompile_sources = - rebase_path(new_api_lint_test_nocompile_sources, "", "nocompile_gn") - expected_compile_output_regex = "Error:.*NewApi" - }, - ] -} + tests = [ + { + target = "nocompile_gn:default_locale_lint_test" + nocompile_sources = + rebase_path(default_locale_lint_test_nocompile_sources, + "", + "nocompile_gn") + expected_compile_output_regex = "Warning:.*DefaultLocale" + }, + { + target = "nocompile_gn:new_api_lint_test" + nocompile_sources = + rebase_path(new_api_lint_test_nocompile_sources, "", "nocompile_gn") + expected_compile_output_regex = "Error:.*NewApi" + }, + ] + } + + android_nocompile_test_suite("android_lookup_dep_tests") { + sources = [ rebase_path( + missing_symbol_generated_importer_template_nocompile_source, + "", + "nocompile_gn") ] -android_nocompile_test_suite("android_lookup_dep_tests") { - sources = - [ rebase_path(missing_symbol_generated_importer_template_nocompile_source, - "", - "nocompile_gn") ] + tests = [ + { + target = "nocompile_gn:import_child_missing_symbol_test_java" + nocompile_sources = + rebase_path(import_child_missing_symbol_test_nocompile_sources, + "", + "nocompile_gn") + expected_compile_output_regex = "error: package test\.missing_symbol\.sub does not exist\nHint: Add \"//build/android/test/nocompile_gn:sub_b_java\" to deps of //build/android/test/nocompile_gn:import_child_missing_symbol_test_java" + }, + { + target = "nocompile_gn:import_parent_missing_symbol_test_java" + nocompile_sources = [] + expected_compile_output_regex = "error: cannot find symbol test\.missing_symbol\.B\nHint: Add \"//build/android/test/nocompile_gn:b_java\" to deps of //build/android/test/nocompile_gn:import_parent_missing_symbol_test_java" + }, + { + target = "nocompile_gn:import_turbine_missing_symbol_test_java" + nocompile_sources = + rebase_path(import_turbine_missing_symbol_test_nocompile_sources, + "", + "nocompile_gn") + expected_compile_output_regex = "error: symbol not found test\.missing_symbol\.B\nHint: Add \"//build/android/test/nocompile_gn:b_java\" to deps of //build/android/test/nocompile_gn:import_turbine_missing_symbol_test_java" + }, + { + target = "nocompile_gn:prebuilt_missing_symbol_test_java" + nocompile_sources = [] + expected_compile_output_regex = "error: cannot find symbol test\.missing_symbol\.C\nHint: Add \"//build/android/test/nocompile_gn:c_prebuilt_java\" to deps of //build/android/test/nocompile_gn:prebuilt_missing_symbol_test_java" + }, + { + target = "nocompile_gn:cpp_template_missing_symbol_test_java" + nocompile_sources = [] + expected_compile_output_regex = "error: cannot find symbol test\.missing_symbol\.D\nHint: Add \"//build/android/test/nocompile_gn:d_java\" to deps of //build/android/test/nocompile_gn:cpp_template_missing_symbol_test_java" + }, + ] + } - tests = [ - { - target = "nocompile_gn:import_child_missing_symbol_test_java" - nocompile_sources = - rebase_path(import_child_missing_symbol_test_nocompile_sources, - "", - "nocompile_gn") - expected_compile_output_regex = "error: package test\.missing_symbol\.sub does not exist\nHint: Add \"//build/android/test/nocompile_gn:sub_b_java\" to deps of //build/android/test/nocompile_gn:import_child_missing_symbol_test_java" - }, - { - target = "nocompile_gn:import_parent_missing_symbol_test_java" - nocompile_sources = [] - expected_compile_output_regex = "error: cannot find symbol test\.missing_symbol\.B\nHint: Add \"//build/android/test/nocompile_gn:b_java\" to deps of //build/android/test/nocompile_gn:import_parent_missing_symbol_test_java" - }, - { - target = "nocompile_gn:import_turbine_missing_symbol_test_java" - nocompile_sources = - rebase_path(import_turbine_missing_symbol_test_nocompile_sources, - "", - "nocompile_gn") - expected_compile_output_regex = "error: symbol not found test\.missing_symbol\.B\nHint: Add \"//build/android/test/nocompile_gn:b_java\" to deps of //build/android/test/nocompile_gn:import_turbine_missing_symbol_test_java" - }, - { - target = "nocompile_gn:prebuilt_missing_symbol_test_java" - nocompile_sources = [] - expected_compile_output_regex = "error: cannot find symbol test\.missing_symbol\.C\nHint: Add \"//build/android/test/nocompile_gn:c_prebuilt_java\" to deps of //build/android/test/nocompile_gn:prebuilt_missing_symbol_test_java" - }, - { - target = "nocompile_gn:cpp_template_missing_symbol_test_java" - nocompile_sources = [] - expected_compile_output_regex = "error: cannot find symbol test\.missing_symbol\.D\nHint: Add \"//build/android/test/nocompile_gn:d_java\" to deps of //build/android/test/nocompile_gn:cpp_template_missing_symbol_test_java" - }, - ] + # Tests that builds which use incremental javac are valid. + junit_binary("incremental_javac_junit_tests") { + sources = [ "../java/test/IncrementalJavacTest.java" ] + deps = [ + "incremental_javac_gn:no_signature_change_prebuilt_java", + "//base:base_junit_test_support", + "//third_party/junit", + ] + } } diff --git a/testing/buildbot/chromium.android.fyi.json b/testing/buildbot/chromium.android.fyi.json index 97b327e8dfc494..f03e31e210ea96 100644 --- a/testing/buildbot/chromium.android.fyi.json +++ b/testing/buildbot/chromium.android.fyi.json @@ -7704,17 +7704,6 @@ "test": "base_junit_tests", "test_id_prefix": "ninja://base:base_junit_tests/" }, - { - "isolate_profile_data": true, - "name": "build_junit_tests", - "resultdb": { - "enable": true, - "has_native_resultdb_integration": true - }, - "swarming": {}, - "test": "build_junit_tests", - "test_id_prefix": "ninja://build/android/test:build_junit_tests/" - }, { "isolate_profile_data": true, "name": "chrome_java_test_pagecontroller_junit_tests", @@ -7770,6 +7759,17 @@ "test": "device_junit_tests", "test_id_prefix": "ninja://device:device_junit_tests/" }, + { + "isolate_profile_data": true, + "name": "incremental_javac_junit_tests", + "resultdb": { + "enable": true, + "has_native_resultdb_integration": true + }, + "swarming": {}, + "test": "incremental_javac_junit_tests", + "test_id_prefix": "ninja://build/android/test:incremental_javac_junit_tests/" + }, { "isolate_profile_data": true, "name": "junit_unit_tests", diff --git a/testing/buildbot/chromium.android.json b/testing/buildbot/chromium.android.json index 044775c3bbff6f..11f849d84a41ad 100644 --- a/testing/buildbot/chromium.android.json +++ b/testing/buildbot/chromium.android.json @@ -1612,12 +1612,6 @@ "test": "base_junit_tests", "test_id_prefix": "ninja://base:base_junit_tests/" }, - { - "name": "build_junit_tests", - "swarming": {}, - "test": "build_junit_tests", - "test_id_prefix": "ninja://build/android/test:build_junit_tests/" - }, { "name": "chrome_java_test_pagecontroller_junit_tests", "swarming": {}, @@ -1648,6 +1642,12 @@ "test": "device_junit_tests", "test_id_prefix": "ninja://device:device_junit_tests/" }, + { + "name": "incremental_javac_junit_tests", + "swarming": {}, + "test": "incremental_javac_junit_tests", + "test_id_prefix": "ninja://build/android/test:incremental_javac_junit_tests/" + }, { "name": "junit_unit_tests", "swarming": {}, @@ -23490,16 +23490,6 @@ "test": "base_junit_tests", "test_id_prefix": "ninja://base:base_junit_tests/" }, - { - "name": "build_junit_tests", - "resultdb": { - "enable": true, - "has_native_resultdb_integration": true - }, - "swarming": {}, - "test": "build_junit_tests", - "test_id_prefix": "ninja://build/android/test:build_junit_tests/" - }, { "name": "chrome_java_test_pagecontroller_junit_tests", "resultdb": { @@ -23550,6 +23540,16 @@ "test": "device_junit_tests", "test_id_prefix": "ninja://device:device_junit_tests/" }, + { + "name": "incremental_javac_junit_tests", + "resultdb": { + "enable": true, + "has_native_resultdb_integration": true + }, + "swarming": {}, + "test": "incremental_javac_junit_tests", + "test_id_prefix": "ninja://build/android/test:incremental_javac_junit_tests/" + }, { "name": "junit_unit_tests", "resultdb": { @@ -31547,17 +31547,6 @@ "test": "base_junit_tests", "test_id_prefix": "ninja://base:base_junit_tests/" }, - { - "isolate_profile_data": true, - "name": "build_junit_tests", - "resultdb": { - "enable": true, - "has_native_resultdb_integration": true - }, - "swarming": {}, - "test": "build_junit_tests", - "test_id_prefix": "ninja://build/android/test:build_junit_tests/" - }, { "isolate_profile_data": true, "name": "chrome_java_test_pagecontroller_junit_tests", @@ -31616,6 +31605,17 @@ "test": "device_junit_tests", "test_id_prefix": "ninja://device:device_junit_tests/" }, + { + "isolate_profile_data": true, + "name": "incremental_javac_junit_tests", + "resultdb": { + "enable": true, + "has_native_resultdb_integration": true + }, + "swarming": {}, + "test": "incremental_javac_junit_tests", + "test_id_prefix": "ninja://build/android/test:incremental_javac_junit_tests/" + }, { "isolate_profile_data": true, "name": "junit_unit_tests", diff --git a/testing/buildbot/chromium.clang.json b/testing/buildbot/chromium.clang.json index 23ac4d2e2e36bc..2d9358bc19b77a 100644 --- a/testing/buildbot/chromium.clang.json +++ b/testing/buildbot/chromium.clang.json @@ -8328,16 +8328,6 @@ "test": "base_junit_tests", "test_id_prefix": "ninja://base:base_junit_tests/" }, - { - "name": "build_junit_tests", - "resultdb": { - "enable": true, - "has_native_resultdb_integration": true - }, - "swarming": {}, - "test": "build_junit_tests", - "test_id_prefix": "ninja://build/android/test:build_junit_tests/" - }, { "name": "chrome_java_test_pagecontroller_junit_tests", "resultdb": { @@ -8388,6 +8378,16 @@ "test": "device_junit_tests", "test_id_prefix": "ninja://device:device_junit_tests/" }, + { + "name": "incremental_javac_junit_tests", + "resultdb": { + "enable": true, + "has_native_resultdb_integration": true + }, + "swarming": {}, + "test": "incremental_javac_junit_tests", + "test_id_prefix": "ninja://build/android/test:incremental_javac_junit_tests/" + }, { "name": "junit_unit_tests", "resultdb": { diff --git a/testing/buildbot/chromium.fyi.json b/testing/buildbot/chromium.fyi.json index d5d8ef49cd9d17..040146a9b8ecb6 100644 --- a/testing/buildbot/chromium.fyi.json +++ b/testing/buildbot/chromium.fyi.json @@ -13966,17 +13966,6 @@ "test": "base_junit_tests", "test_id_prefix": "ninja://base:base_junit_tests/" }, - { - "isolate_profile_data": true, - "name": "build_junit_tests", - "resultdb": { - "enable": true, - "has_native_resultdb_integration": true - }, - "swarming": {}, - "test": "build_junit_tests", - "test_id_prefix": "ninja://build/android/test:build_junit_tests/" - }, { "isolate_profile_data": true, "name": "chrome_java_test_pagecontroller_junit_tests", @@ -14032,6 +14021,17 @@ "test": "device_junit_tests", "test_id_prefix": "ninja://device:device_junit_tests/" }, + { + "isolate_profile_data": true, + "name": "incremental_javac_junit_tests", + "resultdb": { + "enable": true, + "has_native_resultdb_integration": true + }, + "swarming": {}, + "test": "incremental_javac_junit_tests", + "test_id_prefix": "ninja://build/android/test:incremental_javac_junit_tests/" + }, { "isolate_profile_data": true, "name": "junit_unit_tests", diff --git a/testing/buildbot/gn_isolate_map.pyl b/testing/buildbot/gn_isolate_map.pyl index 5a06c50a57f252..6c33b91ec4ddae 100644 --- a/testing/buildbot/gn_isolate_map.pyl +++ b/testing/buildbot/gn_isolate_map.pyl @@ -307,10 +307,6 @@ "type": "windowed_test_launcher", "executable": "browser_tests", }, - "build_junit_tests": { - "label": "//build/android/test:build_junit_tests", - "type": "junit_test", - }, "captured_sites_interactive_tests": { "label": "//chrome/test:captured_sites_interactive_tests", "type": "windowed_test_launcher", @@ -954,6 +950,10 @@ "label": "//headless:headless_unittests", "type": "console_test_launcher", }, + "incremental_javac_junit_tests": { + "label": "//build/android/test:incremental_javac_junit_tests", + "type": "junit_test", + }, "install_static_unittests": { "label": "//chrome/install_static:install_static_unittests", "type": "console_test_launcher", diff --git a/testing/buildbot/test_suites.pyl b/testing/buildbot/test_suites.pyl index 68a2b71a5614a6..a0ebc1ebefc9c0 100644 --- a/testing/buildbot/test_suites.pyl +++ b/testing/buildbot/test_suites.pyl @@ -982,11 +982,6 @@ 'nougat-x86-emulator', ], }, - 'build_junit_tests':{ - 'remove_mixins': [ - 'nougat-x86-emulator', - ], - }, 'chrome_java_test_pagecontroller_junit_tests': { 'remove_mixins': [ 'nougat-x86-emulator', @@ -1012,6 +1007,11 @@ 'nougat-x86-emulator', ], }, + 'incremental_javac_junit_tests':{ + 'remove_mixins': [ + 'nougat-x86-emulator', + ], + }, 'junit_unit_tests': { 'remove_mixins': [ 'nougat-x86-emulator',