Commit 61d72b3c authored by Peter Kotwicz's avatar Peter Kotwicz Committed by Chromium LUCI CQ

[Build] Make Android Lint Tests Better Integration Tests

This CL makes android_lint_test use the android_nocompile_test_suite()
template introduced in
https://chromium-review.googlesource.com/c/chromium/src/+/2538313

This CL also modifies the android_nocompile_test_suite() template to:
- Support pydeps
- To rerun the tests when a "expected_compile_output" value is updated.

BUG=1132014

Change-Id: Iee0480feb082a0afed4dbafa2c35d2b5d05851c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2620838Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Reviewed-by: default avatarDirk Pranke <dpranke@google.com>
Reviewed-by: default avatarPeter Wen <wnwen@chromium.org>
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844579}
parent ab901a3b
...@@ -335,9 +335,9 @@ group("gn_all") { ...@@ -335,9 +335,9 @@ group("gn_all") {
"//base:base_junit_tests", "//base:base_junit_tests",
"//base/android/jni_generator:jni_generator_tests", "//base/android/jni_generator:jni_generator_tests",
"//base/android/linker:chromium_android_linker", "//base/android/linker:chromium_android_linker",
"//build/android:android_lint_test",
"//build/android/gyp/test:hello_world", "//build/android/gyp/test:hello_world",
"//build/android/stacktrace:java_deobfuscate", "//build/android/stacktrace:java_deobfuscate",
"//build/android/test:android_lint_test",
"//build/config/android/test/proto:test_build_protos", "//build/config/android/test/proto:test_build_protos",
"//chrome/android/monochrome:monochrome_public_apk_checker", "//chrome/android/monochrome:monochrome_public_apk_checker",
"//chrome/android/webapk/shell_apk:maps_go_webapk", "//chrome/android/webapk/shell_apk:maps_go_webapk",
......
...@@ -16,27 +16,6 @@ if (enable_java_templates) { ...@@ -16,27 +16,6 @@ if (enable_java_templates) {
create_cache = true create_cache = true
} }
android_library("lint_test_java") {
sources = [ "java/test/LintTest.java" ]
}
android_apk("lint_test_apk") {
# This cannot be marked testonly since lint has special ignores for testonly
# targets. We need to test linting a normal apk target.
apk_name = "LintTest"
deps = [ ":lint_test_java" ]
android_manifest = "//build/android/AndroidManifest.xml"
}
android_lint("android_lint_test") {
lint_expected_warnings = "DefaultLocale,NewApi"
_test_apk_target = ":lint_test_apk"
deps = [ "${_test_apk_target}__java" ]
build_config_dep = "$_test_apk_target$build_config_target_suffix"
build_config = get_label_info(_test_apk_target, "target_gen_dir") + "/" +
get_label_info(_test_apk_target, "name") + ".build_config"
}
if (enable_jdk_library_desugaring) { if (enable_jdk_library_desugaring) {
dex_jdk_libs("all_jdk_libs") { dex_jdk_libs("all_jdk_libs") {
output = "$target_out_dir/$target_name.l8.dex" output = "$target_out_dir/$target_name.l8.dex"
......
...@@ -190,17 +190,6 @@ def _WriteXmlFile(root, path): ...@@ -190,17 +190,6 @@ def _WriteXmlFile(root, path):
root, encoding='utf-8')).toprettyxml(indent=' ')) root, encoding='utf-8')).toprettyxml(indent=' '))
def _CheckLintWarning(expected_warnings, lint_output):
for expected_warning in expected_warnings.split(','):
expected_str = '[{}]'.format(expected_warning)
if expected_str not in lint_output:
raise Exception('Expected {!r} warning in lint output:\n{}.'.format(
expected_str, lint_output))
# Do not print warning
return ''
def _RunLint(lint_binary_path, def _RunLint(lint_binary_path,
backported_methods_path, backported_methods_path,
config_path, config_path,
...@@ -218,7 +207,6 @@ def _RunLint(lint_binary_path, ...@@ -218,7 +207,6 @@ def _RunLint(lint_binary_path,
android_sdk_root, android_sdk_root,
lint_gen_dir, lint_gen_dir,
baseline, baseline,
expected_warnings,
testonly_target=False, testonly_target=False,
warnings_as_errors=False): warnings_as_errors=False):
logging.info('Lint starting') logging.info('Lint starting')
...@@ -330,8 +318,6 @@ def _RunLint(lint_binary_path, ...@@ -330,8 +318,6 @@ def _RunLint(lint_binary_path,
# This filter is necessary for JDK11. # This filter is necessary for JDK11.
stderr_filter = build_utils.FilterReflectiveAccessJavaWarnings stderr_filter = build_utils.FilterReflectiveAccessJavaWarnings
stdout_filter = lambda x: build_utils.FilterLines(x, 'No issues found') stdout_filter = lambda x: build_utils.FilterLines(x, 'No issues found')
if expected_warnings:
stdout_filter = functools.partial(_CheckLintWarning, expected_warnings)
start = time.time() start = time.time()
logging.debug('Lint command %s', ' '.join(cmd)) logging.debug('Lint command %s', ' '.join(cmd))
...@@ -426,9 +412,6 @@ def _ParseArgs(argv): ...@@ -426,9 +412,6 @@ def _ParseArgs(argv):
parser.add_argument('--baseline', parser.add_argument('--baseline',
help='Baseline file to ignore existing errors and fail ' help='Baseline file to ignore existing errors and fail '
'on new errors.') 'on new errors.')
parser.add_argument('--expected-warnings',
help='Comma separated list of warnings to test for in '
'the output, failing if not found.')
args = parser.parse_args(build_utils.ExpandFileArgs(argv)) args = parser.parse_args(build_utils.ExpandFileArgs(argv))
args.java_sources = build_utils.ParseGnList(args.java_sources) args.java_sources = build_utils.ParseGnList(args.java_sources)
...@@ -476,7 +459,6 @@ def main(): ...@@ -476,7 +459,6 @@ def main():
args.android_sdk_root, args.android_sdk_root,
args.lint_gen_dir, args.lint_gen_dir,
args.baseline, args.baseline,
args.expected_warnings,
testonly_target=args.testonly, testonly_target=args.testonly,
warnings_as_errors=args.warnings_as_errors) warnings_as_errors=args.warnings_as_errors)
logging.info('Creating stamp file') logging.info('Creating stamp file')
......
...@@ -7,6 +7,7 @@ import argparse ...@@ -7,6 +7,7 @@ import argparse
import json import json
import os import os
import subprocess import subprocess
import re
import sys import sys
from util import build_utils from util import build_utils
...@@ -85,7 +86,7 @@ def _find_lines_after_prefix(text, prefix, num_lines): ...@@ -85,7 +86,7 @@ def _find_lines_after_prefix(text, prefix, num_lines):
lines = text.split('\n') lines = text.split('\n')
for i, line in enumerate(lines): for i, line in enumerate(lines):
if line.startswith(prefix): if line.startswith(prefix):
return '\n'.join(lines[i:i + num_lines]) return lines[i:i + num_lines]
return None return None
...@@ -128,18 +129,25 @@ def main(): ...@@ -128,18 +129,25 @@ def main():
for config in test_configs: for config in test_configs:
# Strip leading '//' # Strip leading '//'
gn_path = config['target'][2:] gn_path = config['target'][2:]
expectation = config['expect'] expect_regex = config['expect_regex']
ninja_args = [_NINJA_PATH, '-C', options.out_dir, gn_path] ninja_args = [_NINJA_PATH, '-C', options.out_dir, gn_path]
# Purpose of quotes at beginning of message is to make it clear that # Purpose of quotes at beginning of message is to make it clear that
# "Compile successful." is not a compiler log message. # "Compile successful." is not a compiler log message.
test_output = _run_command_get_output(ninja_args, '""\nCompile successful.') test_output = _run_command_get_output(ninja_args, '""\nCompile successful.')
failure_message = _find_lines_after_prefix(test_output, 'FAILED:', 5) failure_message_lines = _find_lines_after_prefix(test_output, 'FAILED:', 5)
if not failure_message or expectation not in failure_message:
error_message = '//{} failed.\nExpected compile output:\n'\ found_expect_regex = False
if failure_message_lines:
for line in failure_message_lines:
if re.search(expect_regex, line):
found_expect_regex = True
break
if not found_expect_regex:
error_message = '//{} failed.\nExpected compile output pattern:\n'\
'{}\nActual compile output:\n{}'.format( '{}\nActual compile output:\n{}'.format(
gn_path, expectation, test_output) gn_path, expect_regex, test_output)
error_messages.append(error_message) error_messages.append(error_message)
if error_messages: if error_messages:
......
// Copyright 2020 The Chromium Authors. All rights reserved. // Copyright 2021 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
...@@ -7,18 +7,11 @@ package test; ...@@ -7,18 +7,11 @@ package test;
import android.app.Application; import android.app.Application;
/** /**
* A class with methods that are meant to trigger lint warnings. If it does not trigger these * Class which fails 'DefaultLocale' lint check.
* expected warnings, then the build will fail. This prevents lint regressions where lint warnings
* are accidentally disabled.
*/ */
public class LintTest extends Application { public class LintTest extends Application {
public String testTriggerDefaultLocaleCheck(int any) { public String testTriggerDefaultLocaleCheck(int any) {
// String format with an integer requires a Locale since it may be formatted differently. // String format with an integer requires a Locale since it may be formatted differently.
return String.format("Test %d", any); return String.format("Test %d", any);
} }
public String testTriggerNewApiCheck() {
// This was added in API level 30.
return getApplicationContext().getAttributionTag();
}
} }
// Copyright 2021 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package test;
import android.app.Application;
/**
* Class which fails 'NewAPI' lint check.
*/
public class NewApiTest extends Application {
public String testTriggerNewApiCheck() {
// This was added in API level 30.
return getApplicationContext().getAttributionTag();
}
}
# Copyright 2021 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import("//build/config/android/android_nocompile.gni")
import("nocompile_gn/nocompile_sources.gni")
if (enable_java_templates) {
android_nocompile_test_suite("android_lint_test") {
# Depend on lint Python script so that the action is re-run whenever the lint 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"
},
]
}
}
# Copyright 2021 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import("//build/config/android/android_nocompile.gni")
import("//build/config/android/rules.gni")
import("nocompile_sources.gni")
template("lint_test") {
_library_target_name = "${target_name}_test_java"
_apk_target_name = "${target_name}_apk"
android_library(_library_target_name) {
sources = [ "//tools/android/errorprone_plugin/test/src/org/chromium/tools/errorprone/plugin/Empty.java" ]
not_needed(invoker, [ "sources" ])
if (enable_android_nocompile_tests) {
sources += invoker.sources
}
}
android_apk(_apk_target_name) {
# This cannot be marked testonly since lint has special ignores for testonly
# targets. We need to test linting a normal apk target.
apk_name = _apk_target_name
deps = [ ":$_library_target_name" ]
android_manifest = "//build/android/AndroidManifest.xml"
}
android_lint(target_name) {
_apk_target = ":${_apk_target_name}"
deps = [ "${_apk_target}__java" ]
build_config_dep = "$_apk_target$build_config_target_suffix"
build_config = get_label_info(_apk_target, "target_gen_dir") + "/" +
get_label_info(_apk_target, "name") + ".build_config"
}
}
lint_test("default_locale_lint_test") {
sources = default_locale_lint_test_nocompile_sources
}
lint_test("new_api_lint_test") {
sources = new_api_lint_test_nocompile_sources
}
# Copyright 2021 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
default_locale_lint_test_nocompile_sources =
[ "../../java/test/DefaultLocaleLintTest.java" ]
new_api_lint_test_nocompile_sources = [ "../../java/test/NewApiLintTest.java" ]
...@@ -24,7 +24,7 @@ declare_args() { ...@@ -24,7 +24,7 @@ declare_args() {
# 'target': The GN target which should not compile when # 'target': The GN target which should not compile when
# enable_android_nocompile_tests=true The target should compile when # enable_android_nocompile_tests=true The target should compile when
# enable_android_nocompile_tests=false. # enable_android_nocompile_tests=false.
# 'expected_compile_output': Error message to test for when compile fails. # 'expected_compile_output_regex': Error message regex to search for when compile fails.
# 'nocompile_sources': Source files which do not compile. This ensures that # 'nocompile_sources': Source files which do not compile. This ensures that
# the test suite is re-run when one of these files change (as the test # the test suite is re-run when one of these files change (as the test
# targets might not depend of the files when # targets might not depend of the files when
...@@ -48,7 +48,17 @@ template("android_nocompile_test_suite") { ...@@ -48,7 +48,17 @@ template("android_nocompile_test_suite") {
deps = [] deps = []
if (defined(invoker.deps)) { if (defined(invoker.deps)) {
deps = invoker.deps deps += invoker.deps
}
inputs = []
if (defined(invoker.pydeps)) {
foreach(_pydeps_file, invoker.pydeps) {
_pydeps_file_lines = read_file(_pydeps_file, "list lines")
_pydeps_entries = filter_exclude(_pydeps_file_lines, [ "#*" ])
_pydeps_file_dir = get_path_info(_pydeps_file, "dir")
inputs += rebase_path(_pydeps_entries, ".", _pydeps_file_dir)
}
} }
sources = [] sources = []
...@@ -62,7 +72,7 @@ template("android_nocompile_test_suite") { ...@@ -62,7 +72,7 @@ template("android_nocompile_test_suite") {
_json_test_configs += [ _json_test_configs += [
{ {
target = "${_dep_dir}:${_dep_name}" target = "${_dep_dir}:${_dep_name}"
expect = _test_config["expected_compile_output"] expect_regex = _test_config["expected_compile_output_regex"]
}, },
] ]
} }
...@@ -82,6 +92,7 @@ template("android_nocompile_test_suite") { ...@@ -82,6 +92,7 @@ template("android_nocompile_test_suite") {
"--stamp", "--stamp",
rebase_path(_stamp_path, root_build_dir), rebase_path(_stamp_path, root_build_dir),
] ]
inputs += [ _config_path ]
outputs = [ _stamp_path ] outputs = [ _stamp_path ]
} }
} }
...@@ -1115,13 +1115,6 @@ if (enable_java_templates) { ...@@ -1115,13 +1115,6 @@ if (enable_java_templates) {
# The full classpath is required for annotation checks like @IntDef. # The full classpath is required for annotation checks like @IntDef.
"--classpath=@FileArg($_rebased_build_config:deps_info:javac_full_interface_classpath)", "--classpath=@FileArg($_rebased_build_config:deps_info:javac_full_interface_classpath)",
] ]
if (defined(invoker.lint_expected_warnings)) {
args += [
"--expected-warnings",
invoker.lint_expected_warnings,
]
}
} }
outputs = [ _stamp_path ] outputs = [ _stamp_path ]
......
...@@ -93,7 +93,7 @@ ...@@ -93,7 +93,7 @@
"type": "windowed_test_launcher", "type": "windowed_test_launcher",
}, },
"android_lint_test": { "android_lint_test": {
"label": "//build/android:android_lint_test", "label": "//build/android/test:android_lint_test",
"type": "additional_compile_target", "type": "additional_compile_target",
}, },
"android_sync_integration_tests": { "android_sync_integration_tests": {
......
...@@ -16,7 +16,7 @@ android_nocompile_test_suite("errorprone_plugin_tests") { ...@@ -16,7 +16,7 @@ android_nocompile_test_suite("errorprone_plugin_tests") {
rebase_path(no_redundant_field_init_check_int_test_nocompile_sources, rebase_path(no_redundant_field_init_check_int_test_nocompile_sources,
"", "",
"nocompile_gn") "nocompile_gn")
expected_compile_output = "warning: [NoRedundantFieldInit]" expected_compile_output_regex = "warning: .NoRedundantFieldInit"
}, },
{ {
target = "nocompile_gn:test_class_name_check_test_java" target = "nocompile_gn:test_class_name_check_test_java"
...@@ -24,7 +24,7 @@ android_nocompile_test_suite("errorprone_plugin_tests") { ...@@ -24,7 +24,7 @@ android_nocompile_test_suite("errorprone_plugin_tests") {
rebase_path(test_class_name_check_test_nocompile_sources, rebase_path(test_class_name_check_test_nocompile_sources,
"", "",
"nocompile_gn") "nocompile_gn")
expected_compile_output = "warning: [TestClassNameCheck]" expected_compile_output_regex = "warning: .TestClassNameCheck"
}, },
] ]
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment