Fix tests (#3977)

Also, make sure that broken tests actually break the bots.

And add a test to make sure that keeps happening.

bin/flutter

@@ -37,22 +37,20 @@ if [ ! -f "$SNAPSHOT_PATH" ] || [ ! -f "$STAMP_PATH" ] || [ `cat "$STAMP_PATH"`
   echo $REVISION > "$STAMP_PATH"
 fi
 
-set +e
-
 if [ $FLUTTER_DEV ]; then
   "$DART" --packages="$FLUTTER_TOOLS_DIR/.packages" -c "$SCRIPT_PATH" "$@"
 else
+  set +e
   "$DART" "$SNAPSHOT_PATH" "$@"
-fi
 
-# The VM exits with code 253 if the snapshot version is out-of-date.
-# If it is, we need to snapshot it again.
-EXIT_CODE=$?
-if [ $EXIT_CODE != 253 ]; then
-  exit $EXIT_CODE
-fi
+  # The VM exits with code 253 if the snapshot version is out-of-date.
+  # If it is, we need to snapshot it again.
+  EXIT_CODE=$?
+  if [ $EXIT_CODE != 253 ]; then
+    exit $EXIT_CODE
+  fi
 
-set -e
-
-"$DART" --snapshot="$SNAPSHOT_PATH" --package="$FLUTTER_TOOLS_DIR/.packages" "$SCRIPT_PATH"
-"$DART" "$SNAPSHOT_PATH" "$@"
+  set -e
+  "$DART" --snapshot="$SNAPSHOT_PATH" --package="$FLUTTER_TOOLS_DIR/.packages" "$SCRIPT_PATH"
+  "$DART" "$SNAPSHOT_PATH" "$@"
+fi

dev/automated_tests/.gitignore

+.atom
+.DS_Store
+.buildlog
+.idea
+.packages
+.pub/
+build/
+packages
+pubspec.lock

dev/automated_tests/README.md

+This is a fake package for use by automated testing.
+For example, the `flutter_tools` package uses this to test `flutter test`.

dev/automated_tests/flutter.yaml

+uses-material-design: true

dev/automated_tests/pubspec.yaml

+name: flutter_automated_tests
+dependencies:
+  flutter:
+    path: ../../packages/flutter
+  flutter_test:
+    path: ../../packages/flutter_test

dev/automated_tests/test_smoke_test/README.md

+This directory is used by ///flutter/travis/test.sh to verify that
+`flutter test` actually correctly fails when a test fails.

dev/automated_tests/test_smoke_test/fail_test.dart

+// Copyright 2016 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 'package:flutter_test/flutter_test.dart';
+
+// this is a test to make sure our tests actually catch failures
+// see ///flutter/travis/test.sh
+
+void main() {
+  test('test smoke test -- this test SHOULD FAIL', () async {
+    expect(false, isTrue);
+  });
+}
\ No newline at end of file

dev/automated_tests/test_smoke_test/pass_test.dart

+// Copyright 2016 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 'package:flutter_test/flutter_test.dart';
+
+// this is a test to make sure our tests actually catch failures
+// see ///flutter/travis/test.sh
+
+void main() {
+  test('test smoke test -- this test should pass', () async {
+    expect(true, isTrue);
+  });
+}
\ No newline at end of file

packages/flutter_test/lib/src/test_async_utils.dart

@@ -289,7 +289,7 @@ class TestAsyncUtils {
           break;
         }
         if (index < stack.length) {
-          final RegExp callerPattern = new RegExp(r'^#[0-9]+ .* \((.+):([0-9]+)(?::[0-9]+)?\)$');
+          final RegExp callerPattern = new RegExp(r'^#[0-9]+ .* \((.+?):([0-9]+)(?::[0-9]+)?\)$');
           final Match callerMatch = callerPattern.matchAsPrefix(stack[index]); // extract the caller's info
           if (callerMatch != null) {
             assert(callerMatch.groupCount == 2);

packages/flutter_test/test/test_async_utils_test.dart

@@ -48,8 +48,8 @@ void main() {
     } on FlutterError catch (e) {
       List<String> lines = e.message.split('\n');
       real_test.expect(lines[0], 'Guarded function conflict. You must use "await" with all Future-returning test APIs.');
-      real_test.expect(lines[1], matches(r'The guarded method "testGuard1" from class TestAPI was called from .*test_async_utils.dart on line [0-9]+\.'));
-      real_test.expect(lines[2], matches(r'Then, the "testGuard2" method \(also from class TestAPI\) was called from .*test_async_utils.dart on line [0-9]+\.'));
+      real_test.expect(lines[1], matches(r'The guarded method "testGuard1" from class TestAPI was called from .*test_async_utils_test.dart on line [0-9]+\.'));
+      real_test.expect(lines[2], matches(r'Then, the "testGuard2" method \(also from class TestAPI\) was called from .*test_async_utils_test.dart on line [0-9]+\.'));
       real_test.expect(lines[3], 'The first method (TestAPI.testGuard1) had not yet finished executing at the time that the second method (TestAPI.testGuard2) was called. Since both are guarded, and the second was not a nested call inside the first, the first must complete its execution before the second can be called. Typically, this is achieved by putting an "await" statement in front of the call to the first.');
       real_test.expect(lines[4], '');
       real_test.expect(lines[5], 'When the first method (TestAPI.testGuard1) was called, this was the stack:');
@@ -69,8 +69,8 @@ void main() {
     } on FlutterError catch (e) {
       List<String> lines = e.message.split('\n');
       real_test.expect(lines[0], 'Guarded function conflict. You must use "await" with all Future-returning test APIs.');
-      real_test.expect(lines[1], matches(r'^The guarded method "testGuard1" from class TestAPI was called from .*test_async_utils.dart on line [0-9]+\.$'));
-      real_test.expect(lines[2], matches(r'^Then, the "testGuard2" method \(also from class TestAPI\) was called from .*test_async_utils.dart on line [0-9]+\.$'));
+      real_test.expect(lines[1], matches(r'^The guarded method "testGuard1" from class TestAPI was called from .*test_async_utils_test.dart on line [0-9]+\.$'));
+      real_test.expect(lines[2], matches(r'^Then, the "testGuard2" method \(also from class TestAPI\) was called from .*test_async_utils_test.dart on line [0-9]+\.$'));
       real_test.expect(lines[3], 'The first method (TestAPI.testGuard1) had not yet finished executing at the time that the second method (TestAPI.testGuard2) was called. Since both are guarded, and the second was not a nested call inside the first, the first must complete its execution before the second can be called. Typically, this is achieved by putting an "await" statement in front of the call to the first.');
       real_test.expect(lines[4], '');
       real_test.expect(lines[5], 'When the first method (TestAPI.testGuard1) was called, this was the stack:');
@@ -90,8 +90,8 @@ void main() {
     } on FlutterError catch (e) {
       List<String> lines = e.message.split('\n');
       real_test.expect(lines[0], 'Guarded function conflict. You must use "await" with all Future-returning test APIs.');
-      real_test.expect(lines[1], matches(r'^The guarded method "testGuard1" from class TestAPI was called from .*test_async_utils.dart on line [0-9]+\.$'));
-      real_test.expect(lines[2], matches(r'^Then, the "testGuard3" method from class TestAPISubclass was called from .*test_async_utils.dart on line [0-9]+\.$'));
+      real_test.expect(lines[1], matches(r'^The guarded method "testGuard1" from class TestAPI was called from .*test_async_utils_test.dart on line [0-9]+\.$'));
+      real_test.expect(lines[2], matches(r'^Then, the "testGuard3" method from class TestAPISubclass was called from .*test_async_utils_test.dart on line [0-9]+\.$'));
       real_test.expect(lines[3], 'The first method (TestAPI.testGuard1) had not yet finished executing at the time that the second method (TestAPISubclass.testGuard3) was called. Since both are guarded, and the second was not a nested call inside the first, the first must complete its execution before the second can be called. Typically, this is achieved by putting an "await" statement in front of the call to the first.');
       real_test.expect(lines[4], '');
       real_test.expect(lines[5], 'When the first method (TestAPI.testGuard1) was called, this was the stack:');
@@ -111,8 +111,8 @@ void main() {
     } on FlutterError catch (e) {
       List<String> lines = e.message.split('\n');
       real_test.expect(lines[0], 'Guarded function conflict. You must use "await" with all Future-returning test APIs.');
-      real_test.expect(lines[1], matches(r'^The guarded method "testGuard1" from class TestAPI was called from .*test_async_utils.dart on line [0-9]+\.$'));
-      real_test.expect(lines[2], matches(r'^Then, the "expect" function was called from .*test_async_utils.dart on line [0-9]+\.$'));
+      real_test.expect(lines[1], matches(r'^The guarded method "testGuard1" from class TestAPI was called from .*test_async_utils_test.dart on line [0-9]+\.$'));
+      real_test.expect(lines[2], matches(r'^Then, the "expect" function was called from .*test_async_utils_test.dart on line [0-9]+\.$'));
       real_test.expect(lines[3], 'The first method (TestAPI.testGuard1) had not yet finished executing at the time that the second function (expect) was called. Since both are guarded, and the second was not a nested call inside the first, the first must complete its execution before the second can be called. Typically, this is achieved by putting an "await" statement in front of the call to the first.');
       real_test.expect(lines[4], 'If you are confident that all test APIs are being called using "await", and this expect() call is not being invoked at the top level but is itself being called from some sort of callback registered before the testGuard1 method was called, then consider using expectSync() instead.');
       real_test.expect(lines[5], '');
@@ -131,8 +131,8 @@ void main() {
     } on FlutterError catch (e) {
       List<String> lines = e.message.split('\n');
       real_test.expect(lines[0], 'Guarded function conflict. You must use "await" with all Future-returning test APIs.');
-      real_test.expect(lines[1], matches(r'^The guarded method "pump" from class WidgetTester was called from .*test_async_utils.dart on line [0-9]+\.$'));
-      real_test.expect(lines[2], matches(r'^Then, it was called from .*test_async_utils.dart on line [0-9]+\.$'));
+      real_test.expect(lines[1], matches(r'^The guarded method "pump" from class WidgetTester was called from .*test_async_utils_test.dart on line [0-9]+\.$'));
+      real_test.expect(lines[2], matches(r'^Then, it was called from .*test_async_utils_test.dart on line [0-9]+\.$'));
       real_test.expect(lines[3], 'The first method had not yet finished executing at the time that the second method was called. Since both are guarded, and the second was not a nested call inside the first, the first must complete its execution before the second can be called. Typically, this is achieved by putting an "await" statement in front of the call to the first.');
       real_test.expect(lines[4], '');
       real_test.expect(lines[5], 'When the first method was called, this was the stack:');
@@ -151,7 +151,7 @@ void main() {
     } on FlutterError catch (e) {
       List<String> lines = e.message.split('\n');
       real_test.expect(lines[0], 'Asynchronous call to guarded function leaked. You must use "await" with all Future-returning test APIs.');
-      real_test.expect(lines[1], matches(r'^The guarded method "pump" from class WidgetTester was called from .*test_async_utils.dart on line [0-9]+, but never completed before its parent scope closed\.$'));
+      real_test.expect(lines[1], matches(r'^The guarded method "pump" from class WidgetTester was called from .*test_async_utils_test.dart on line [0-9]+, but never completed before its parent scope closed\.$'));
       real_test.expect(lines[2], matches(r'^The guarded method "pump" from class AutomatedTestWidgetsFlutterBinding was called from [^ ]+ on line [0-9]+, but never completed before its parent scope closed\.'));
       real_test.expect(lines.length, 3);
     }
@@ -167,7 +167,7 @@ void main() {
     } on FlutterError catch (e) {
       List<String> lines = e.message.split('\n');
       real_test.expect(lines[0], 'Asynchronous call to guarded function leaked. You must use "await" with all Future-returning test APIs.');
-      real_test.expect(lines[1], matches(r'^The guarded method "pump" from class WidgetTester was called from .*test_async_utils.dart on line [0-9]+, but never completed before its parent scope closed\.$'));
+      real_test.expect(lines[1], matches(r'^The guarded method "pump" from class WidgetTester was called from .*test_async_utils_test.dart on line [0-9]+, but never completed before its parent scope closed\.$'));
       real_test.expect(lines[2], matches(r'^The guarded method "pump" from class AutomatedTestWidgetsFlutterBinding was called from [^ ]+ on line [0-9]+, but never completed before its parent scope closed\.'));
       real_test.expect(lines.length, 3);
     }

packages/flutter_tools/lib/executable.dart

@@ -180,6 +180,7 @@ Future<Null> _exit(int code) async {
 
   // Give the task / timer queue one cycle through before we hard exit.
   await Timer.run(() {
+    printTrace('exiting with code $code');
     exit(code);
   });
 }

packages/flutter_tools/lib/src/commands/test.dart

@@ -56,8 +56,14 @@ class TestCommand extends FlutterCommand {
   Future<int> _runTests(List<String> testArgs, Directory testDirectory) async {
     Directory currentDirectory = Directory.current;
     try {
-      Directory.current = testDirectory;
-      return await executable.main(testArgs);
+      if (testDirectory != null) {
+        printTrace('switching to directory $testDirectory to run tests');
+        Directory.current = testDirectory;
+      }
+      printTrace('running test package with arguments: $testArgs');
+      await executable.main(testArgs);
+      printTrace('test package returned with exit code $exitCode');
+      return exitCode;
     } finally {
       Directory.current = currentDirectory;
     }
@@ -70,9 +76,10 @@ class TestCommand extends FlutterCommand {
     if (!projectRootValidator())
       return 1;
 
-    Directory testDir = _currentPackageTestDir;
+    Directory testDir;
 
     if (testArgs.isEmpty) {
+      testDir = _currentPackageTestDir;
       if (!testDir.existsSync()) {
         printError("Test directory '${testDir.path}' not found.");
         return 1;

travis/test.sh

@@ -6,6 +6,10 @@ export PATH="$PWD/bin:$PWD/bin/cache/dart-sdk/bin:$PATH"
 # analyze all the Dart code in the repo
 flutter analyze --flutter-repo
 
+# verify that the tests actually return failure on failure and success on success
+(cd dev/automated_tests; ! flutter test test_smoke_test/fail_test.dart > /dev/null)
+(cd dev/automated_tests; flutter test test_smoke_test/pass_test.dart > /dev/null)
+
 # run tests
 (cd packages/flutter; flutter test)
 (cd packages/flutter_driver; dart -c test/all.dart)