From 603d401fc2e70e5a9d447b25601c0d2f974fd39b Mon Sep 17 00:00:00 2001 From: Michael Klimushyn Date: Tue, 23 Jul 2019 09:06:19 -0700 Subject: [PATCH] Add working Robolectric tests (flutter/engine#9954) `gclient sync` now grabs Robolectric, JUnit, and their transitive runtime dependencies. They're being stored in a new CIPD package, `flutter/android/robolectric_bundle`. `shell/platform/android/BUILD.gn` has a new target for building the tests, `robolectric_tests`. `testing/run_tests.py` has been extended to build and run the new target. Runs the android tests under "build_and_test_android" on CI. This also adds some very simple sample tests to start with and a README to the java tests directory. --- DEPS | 11 +++ engine/src/flutter/.cirrus.yml | 3 +- .../flutter/shell/platform/android/BUILD.gn | 54 ++++++++++- .../shell/platform/android/test/README.md | 95 +++++++++++++++++++ .../test/io/flutter/FlutterTestSuite.java | 20 ++++ .../android/test/io/flutter/SmokeTest.java | 23 +++++ .../io/flutter/util/PreconditionsTest.java | 34 +++++++ engine/src/flutter/testing/run_tests.py | 71 +++++++++++++- 8 files changed, 305 insertions(+), 6 deletions(-) create mode 100644 engine/src/flutter/shell/platform/android/test/README.md create mode 100644 engine/src/flutter/shell/platform/android/test/io/flutter/FlutterTestSuite.java create mode 100644 engine/src/flutter/shell/platform/android/test/io/flutter/SmokeTest.java create mode 100644 engine/src/flutter/shell/platform/android/test/io/flutter/util/PreconditionsTest.java diff --git a/DEPS b/DEPS index bee2294fbd6..9a59ab93fbb 100644 --- a/DEPS +++ b/DEPS @@ -458,6 +458,17 @@ deps = { 'dep_type': 'cipd', }, + 'src/third_party/robolectric': { + 'packages': [ + { + 'package': 'flutter/android/robolectric_bundle', + 'version': 'last_updated:2019-07-22@11:16:04-07:00' + } + ], + 'condition': 'download_android_deps', + 'dep_type': 'cipd', + }, + 'src/third_party/dart/tools/sdks': { 'packages': [ { diff --git a/engine/src/flutter/.cirrus.yml b/engine/src/flutter/.cirrus.yml index 28d982b882a..30bc6bc7f32 100644 --- a/engine/src/flutter/.cirrus.yml +++ b/engine/src/flutter/.cirrus.yml @@ -60,7 +60,7 @@ task: test_host_script: | cd $ENGINE_PATH/src ./flutter/testing/run_tests.sh host_release - - name: build_android_unopt_debug + - name: build_and_test_android_unopt_debug get_android_sdk_script: | echo 'solutions = [{"managed": False,"name": "src/flutter","url": "git@github.com:flutter/engine.git","deps_file": "DEPS", "custom_vars": {"download_windows_deps" : False,},},]' > $ENGINE_PATH/.gclient cd $ENGINE_PATH/src @@ -75,6 +75,7 @@ task: ninja -C out/android_debug_unopt mkdir javadoc_tmp ./flutter/tools/gen_javadoc.py --out-dir javadoc_tmp + test_android_script: cd $ENGINE_PATH/src && python ./flutter/testing/run_tests.py --type=java - name: format_and_dart_test format_script: | cd $ENGINE_PATH/src/flutter diff --git a/engine/src/flutter/shell/platform/android/BUILD.gn b/engine/src/flutter/shell/platform/android/BUILD.gn index 8cf69d41601..869b70a3e96 100644 --- a/engine/src/flutter/shell/platform/android/BUILD.gn +++ b/engine/src/flutter/shell/platform/android/BUILD.gn @@ -107,11 +107,13 @@ action("gen_android_build_config_java") { ] } +flutter_jar_path = "$root_out_dir/flutter_java.jar" + action("flutter_shell_java") { script = "//build/android/gyp/javac.py" depfile = "$target_gen_dir/$target_name.d" - jar_path = "$root_out_dir/flutter_java.jar" + jar_path = flutter_jar_path sources = [ "io/flutter/Log.java", @@ -311,3 +313,53 @@ action("android") { ":flutter_shell_native", ] } + +# To build and run: +# testing/run_tests.py [--type=java] [--filter=io.flutter.TestClassName] +action("robolectric_tests") { + script = "//build/android/gyp/javac.py" + depfile = "$target_gen_dir/$target_name.d" + + jar_path = "$root_out_dir/robolectric_tests.jar" + + sources = [ + "test/io/flutter/FlutterTestSuite.java", + "test/io/flutter/SmokeTest.java", + "test/io/flutter/util/PreconditionsTest.java", + ] + + outputs = [ + depfile, + jar_path, + jar_path + ".md5.stamp", + ] + + _jar_dependencies = [ + android_sdk_jar, + flutter_jar_path, + "//third_party/robolectric/lib/junit-3.8.jar", + "//third_party/robolectric/lib/junit-4.13-beta-3.jar", + "//third_party/robolectric/lib/robolectric-3.8.jar", + "//third_party/robolectric/lib/annotations-3.8.jar", + ] + + inputs = _jar_dependencies + + _rebased_jar_path = rebase_path(jar_path, root_build_dir) + _rebased_depfile = rebase_path(depfile, root_build_dir) + _rebased_classpath = rebase_path(_jar_dependencies, root_build_dir) + _rebased_srcjars = rebase_path(_jar_dependencies, root_build_dir) + + args = [ + "--depfile=$_rebased_depfile", + "--jar-path=$_rebased_jar_path", + "--classpath=$_rebased_classpath", + "--java-srcjars=$_rebased_srcjars", + ] + + args += rebase_path(sources, root_build_dir) + + deps = [ + ":flutter_shell_java", + ] +} diff --git a/engine/src/flutter/shell/platform/android/test/README.md b/engine/src/flutter/shell/platform/android/test/README.md new file mode 100644 index 00000000000..081be70d4a8 --- /dev/null +++ b/engine/src/flutter/shell/platform/android/test/README.md @@ -0,0 +1,95 @@ +# Unit testing Java code + +All Java code in the engine should now be able to be tested with Robolectric 3.8 +and JUnit 4. The test suite has been added after the bulk of the Java code was +first written, so most of these classes do not have existing tests. Ideally code +after this point should be tested, either with unit tests here or with +integration tests in other repos. + +## Adding a new test + +1. Create a file under `test/` matching the path and name of the class under + test. For example, + `shell/platform/android/io/flutter/util/Preconditions.java` -> + `shell/platform/android/**test**/io/flutter/util/Preconditions**Test**.java`. +2. Add your file to the `sources` of the `robolectric_tests` build target in + `/shell/platform/android/BUILD.gn`. This compiles the test class into the + test jar. +3. Add your class to the `@SuiteClasses` annotation in `FlutterTestSuite.java`. + This makes sure the test is actually executed at run time. +4. Write your test. +5. Build and run with `testing/run_tests.py [--type=java] [filter=]`. + +## Q&A + +### Why are we using Robolectric 3.8 when Robolectric 4+ is current? + +Robolectric 4+ uses the AndroidX libraries, and the engine sources use the +deprecated android.support ones. See +[flutter/flutter#23586](https://github.com/flutter/flutter/issues/23586). If +this is an issue we could use Jetifier on `flutter.jar` first and _then_ run +the tests, but it would add an extra point of failure. + +### My new test won't run. There's a "ClassNotFoundException". + +Your test is probably using a dependency that we haven't needed yet. You +probably need to find the dependency you need, add it to the +`flutter/android/robolectric_bundle` CIPD package, and then re-run `gclient +sync`. See ["Updating a CIPD dependency"](#Updating-a-CIPD-dependency) below. + +### My new test won't compile. It can't find one of my imports. + +You could be using a brand new dependency. If so, you'll need to add it to the +CIPD package for the robolectric tests. See ["Updating a CIPD +dependency"](#Updating-a-CIPD-dependency) below. + +Then you'll also need to add the jar to the `robolectric_tests` build target. +Add `//third_party/robolectric/lib/` to +`robolectric_tests._jar_dependencies` in `/shell/platform/android/BUILD.gn`. + +There's also a chance that you're using a dependency that we're relying on at +runtime, but not compile time. If so you'll just need to update +`_jar_dependencies` in `BUILD.gn`. + +### Updating a CIPD dependency + +See the Chromium instructions on ["Updating a CIPD +dependency"](https://chromium.googlesource.com/chromium/src/+/master/docs/cipd.md#Updating-a-CIPD-dependency) +for how to upload a package update to CIPD. + +Once you've done that, also make sure to tag the new package version with the +updated timestamp and robolectric version (most likely still 3.8, unless you've +migrated all the packges to 4+). + + $ cipd set-tag --version= -tag "last_updated:" + $ cipd set-tag --version= -tag "robolectric_version:" + +You can run `cipd describe flutter/android/robolectric_bundle +--version=` to verify. You should see: + +``` +Package: flutter/android/robolectric_bundle +Instance ID: +... +Tags: + last_updated: + robolectric_version: +``` + +Then update the `DEPS` file to use the new version by pointing to your new +`last_updated_at` tag. + +``` + 'src/third_party/robolectric': { + 'packages': [ + { + 'package': 'flutter/android/robolectric_bundle', + 'version': 'last_updated:' + } + ], + 'condition': 'download_android_deps', + 'dep_type': 'cipd', + }, +``` + +You can now re-run `gclient sync` to fetch the latest package version. diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/FlutterTestSuite.java b/engine/src/flutter/shell/platform/android/test/io/flutter/FlutterTestSuite.java new file mode 100644 index 00000000000..9b705f80fad --- /dev/null +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/FlutterTestSuite.java @@ -0,0 +1,20 @@ +// Copyright 2013 The Flutter 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 io.flutter; + +import io.flutter.SmokeTest; +import io.flutter.util.PreconditionsTest; + +import org.junit.runner.RunWith; +import org.junit.runners.Suite; +import org.junit.runners.Suite.SuiteClasses; + +@RunWith(Suite.class) +@SuiteClasses({ + PreconditionsTest.class, + SmokeTest.class +}) +/** Runs all of the unit tests listed in the {@code @SuiteClasses} annotation. */ +public class FlutterTestSuite {} diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/SmokeTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/SmokeTest.java new file mode 100644 index 00000000000..5d1e41aefc4 --- /dev/null +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/SmokeTest.java @@ -0,0 +1,23 @@ +// Copyright 2013 The Flutter 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 io.flutter; + +import static org.junit.Assert.assertTrue; + +import android.text.TextUtils; +import org.junit.runner.RunWith; +import org.junit.Test; +import org.robolectric.annotation.Config; +import org.robolectric.RobolectricTestRunner; + +/** Basic smoke test verifying that Robolectric is loaded and mocking out Android APIs. */ +@Config(manifest=Config.NONE) +@RunWith(RobolectricTestRunner.class) +public class SmokeTest { + @Test + public void androidLibraryLoaded() { + assertTrue(TextUtils.equals("xyzzy", "xyzzy")); + } +} diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/util/PreconditionsTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/util/PreconditionsTest.java new file mode 100644 index 00000000000..2ac56b901d8 --- /dev/null +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/util/PreconditionsTest.java @@ -0,0 +1,34 @@ +// Copyright 2013 The Flutter 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 io.flutter.util; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertThrows; + +import android.text.TextUtils; +import io.flutter.util.Preconditions; +import org.junit.runner.RunWith; +import org.junit.Test; +import org.robolectric.annotation.Config; +import org.robolectric.RobolectricTestRunner; + +@Config(manifest=Config.NONE) +@RunWith(RobolectricTestRunner.class) +public class PreconditionsTest { + @Test + public void checkNotNull_notNull() { + // Should always return its input. + assertEquals("non-null", Preconditions.checkNotNull("non-null")); + assertEquals(42, (int) Preconditions.checkNotNull(42)); + Object classParam = new Object(); + assertEquals(classParam, Preconditions.checkNotNull(classParam)); + } + + @Test + public void checkNotNull_Null() { + assertThrows(NullPointerException.class, () -> {Preconditions.checkNotNull(null);}); + } +} diff --git a/engine/src/flutter/testing/run_tests.py b/engine/src/flutter/testing/run_tests.py index 7a63bd8a5e2..e8fd50a2cc9 100755 --- a/engine/src/flutter/testing/run_tests.py +++ b/engine/src/flutter/testing/run_tests.py @@ -191,6 +191,58 @@ def EnsureDebugUnoptSkyPackagesAreBuilt(): subprocess.check_call(gn_command, cwd=buildroot_dir) subprocess.check_call(ninja_command, cwd=buildroot_dir) +def EnsureJavaTestsAreBuilt(android_out_dir): + ninja_command = [ + 'ninja', + '-C', + android_out_dir, + 'flutter/shell/platform/android:robolectric_tests' + ] + + # Attempt running Ninja if the out directory exists. + # We don't want to blow away any custom GN args the caller may have already set. + if os.path.exists(android_out_dir): + subprocess.check_call(ninja_command, cwd=buildroot_dir) + return + + # Otherwise prepare the directory first, then build the test. + gn_command = [ + os.path.join(buildroot_dir, 'flutter', 'tools', 'gn'), + '--android', + '--unoptimized', + '--runtime-mode=debug', + '--no-lto', + ] + subprocess.check_call(gn_command, cwd=buildroot_dir) + subprocess.check_call(ninja_command, cwd=buildroot_dir) + +def RunJavaTests(filter): + # There's no real reason why other Android build types couldn't be supported + # here. Could default to this but use any other variant of android_ if it + # exists, but it seems like overkill to add that logic in right now. + android_out_dir = os.path.join(out_dir, 'android_debug_unopt') + EnsureJavaTestsAreBuilt(android_out_dir) + + robolectric_dir = os.path.join(buildroot_dir, 'third_party', 'robolectric', 'lib') + classpath = map(str, [ + os.path.join(buildroot_dir, 'third_party', 'android_tools', 'sdk', 'platforms', 'android-28', 'android.jar'), + os.path.join(robolectric_dir, '*'), # Wildcard for all jars in the directory + os.path.join(android_out_dir, 'flutter_java.jar'), + os.path.join(android_out_dir, 'robolectric_tests.jar') + ]) + + test_class = filter if filter else 'io.flutter.FlutterTestSuite' + command = [ + 'java', + '-Drobolectric.offline=true', + '-Drobolectric.dependency.dir=' + robolectric_dir, + '-classpath', ':'.join(classpath), + 'org.junit.runner.JUnitCore', + test_class + ] + + return subprocess.call(command) + def RunDartTests(build_dir, filter): # This one is a bit messy. The pubspec.yaml at flutter/testing/dart/pubspec.yaml # has dependencies that are hardcoded to point to the sky packages at host_debug_unopt/ @@ -219,26 +271,37 @@ def main(): help='A list of engine test executables to run.') parser.add_argument('--dart-filter', type=str, default='', help='A list of Dart test scripts to run.') + parser.add_argument('--java-filter', type=str, default='', + help='A single Java test class to run.') args = parser.parse_args() if args.type == 'all': - types = ['engine', 'dart', 'benchmarks'] + types = ['engine', 'dart', 'benchmarks', 'java'] else: types = args.type.split(',') build_dir = os.path.join(out_dir, args.variant) - assert os.path.exists(build_dir), 'Build variant directory %s does not exist!' % build_dir + if args.type != 'java': + assert os.path.exists(build_dir), 'Build variant directory %s does not exist!' % build_dir engine_filter = args.engine_filter.split(',') if args.engine_filter else None if 'engine' in types: RunCCTests(build_dir, engine_filter) - # https://github.com/flutter/flutter/issues/36301 - if 'dart' in types and not IsWindows(): + if 'dart' in types: + assert not IsWindows(), "Dart tests can't be run on windows. https://github.com/flutter/flutter/issues/36301." dart_filter = args.dart_filter.split(',') if args.dart_filter else None RunDartTests(build_dir, dart_filter) + if 'java' in types: + assert not IsWindows(), "Android engine files can't be compiled on Windows." + java_filter = args.java_filter + if ',' in java_filter or '*' in java_filter: + print('Can only filter JUnit4 tests by single entire class name, eg "io.flutter.SmokeTest". Ignoring filter=' + java_filter) + java_filter = None + RunJavaTests(java_filter) + # https://github.com/flutter/flutter/issues/36300 if 'benchmarks' in types and not IsWindows(): RunEngineBenchmarks(build_dir, engine_filter)