Merge pull request #954 from Hixie/unit-to-flutter

Move flutter tests from packages/unit to packages/flutter/test

CONTRIBUTING.md

@@ -76,7 +76,7 @@ own code by mimicking the `pubspec.yaml` files in the `examples` subdirectories.
 Running the tests
 -----------------
 
-To automatically find all files named `_test.dart` inside a package and run them inside the flutter shell as a test use the `flutter test` command, e.g:
+To automatically find all files named `_test.dart` inside a package's `test/` subdirectory, and run them inside the flutter shell as a test, use the `flutter test` command, e.g:
 
  * `cd examples/stocks`
  * `flutter test`
@@ -90,9 +90,11 @@ Flutter tests use [package:flutter_test](https://github.com/flutter/flutter/tree
  * `cd packages/newton`
  * `pub run test`
 
-`flutter test --flutter-repo` is a shortcut for those working on the flutter repository itself which finds and runs all tests inside the flutter repository regardless of the current working directory.
+`flutter test --flutter-repo` is a shortcut for those working on the flutter repository itself which runs all tests inside the `flutter` package regardless of the current working directory.
+To run all the tests for the entire Flutter repository, the same way that Travis runs them, run `travis/test.sh`.
 
 If you've built [your own flutter engine](#working-on-the-engine-and-the-framework-at-the-same-time), you can pass `--debug` or `--release` to change what flutter shell `flutter test` uses.
+To do this with the `travis/test.sh` script, you can use the `FLUTTER_ENGINE` environment variable.
 
 Note: Flutter tests are headless, you won't see any UI. You can use
 `print` to generate console output or you can interact with the DartVM
@@ -101,9 +103,9 @@ via observatory at [http://localhost:8181/](http://localhost:8181/).
 Adding a test
 -------------
 
-To add a test, simply create a file whose name ends with `_test.dart`
-in the `packages/unit/test` directory. The test should have a `main`
-function and use the `test` package.
+To add a test to the Flutter package, simply create a file whose name
+ends with `_test.dart` in the `packages/flutter/test` directory. The
+test should have a `main` function and use the `test` package.
 
 Contributing code
 -----------------

examples/stocks/pubspec.yaml

@@ -2,8 +2,8 @@ name: stocks
 dependencies:
   flutter:
     path: ../../packages/flutter
-
   intl: '>=0.12.4+2 <0.13.0'
+
 dev_dependencies:
   flutter_test:
     path: ../../packages/flutter_test

packages/cassowary/pubspec.yaml

@@ -6,6 +6,6 @@ homepage: https://github.com/flutter/flutter/tree/master/packages/cassowary
 environment:
   sdk: '>=1.0.0 <2.0.0'
 dev_dependencies:
-  test: 0.12.6+1
-  test_runner: '<=0.2.16'
-  dart_coveralls: '<=0.3.0'
+  flutter_tools:
+    path: ../flutter_tools
+  test: any # constrained by the dependency in flutter_tools

packages/flutter/pubspec.yaml

@@ -3,6 +3,7 @@ version: 0.0.21
 author: Flutter Authors <flutter-dev@googlegroups.com>
 description: A framework for writing Flutter applications
 homepage: http://flutter.io
+
 dependencies:
   collection: '>=1.1.3 <2.0.0'
   intl: '>=0.12.4+2 <0.13.0'
@@ -11,7 +12,9 @@ dependencies:
   sky_services: 0.0.73
   vector_math: '>=1.4.3 <2.0.0'
 
-  # To pin the transitive dependency through mojo_sdk.
+  # See the comment in flutter_tools' pubspec.yaml. We have to pin it
+  # here also because sky_services depends on mojo_sdk which depends
+  # on test.
   test: 0.12.6+1
 
   cassowary:
@@ -19,5 +22,9 @@ dependencies:
   newton:
     path: ../newton
 
+dev_dependencies:
+  flutter_test:
+    path: ../flutter_test
+
 environment:
   sdk: '>=1.12.0 <2.0.0'

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

@@ -62,42 +62,11 @@ class AnalyzeCommand extends FlutterCommand {
         }
       }
 
-      bool foundTest = false;
-      Directory flutterDir = new Directory(path.join(ArtifactStore.flutterRoot, 'packages/unit')); // See https://github.com/flutter/flutter/issues/50
-
-      // .../packages/unit/test/*/*_test.dart
-      Directory tests = new Directory(path.join(flutterDir.path, 'test'));
-      for (FileSystemEntity entry in tests.listSync()) {
-        if (entry is Directory) {
-          for (FileSystemEntity subentry in entry.listSync()) {
-            if (subentry is File && subentry.path.endsWith('_test.dart')) {
-              dartFiles.add(subentry.path);
-              foundTest = true;
-            }
-          }
-        }
-      }
-
-      // .../packages/unit/benchmark/*/*_bench.dart
-      Directory benchmarks = new Directory(path.join(flutterDir.path, 'benchmark'));
-      for (FileSystemEntity entry in benchmarks.listSync()) {
-        if (entry is Directory) {
-          for (FileSystemEntity subentry in entry.listSync()) {
-            if (subentry is File && subentry.path.endsWith('_bench.dart')) {
-              dartFiles.add(subentry.path);
-              foundTest = true;
-            }
-          }
-        }
-      }
-
-      if (foundTest)
-        pubSpecDirectories.add(flutterDir.path);
-
       // .../packages/*/bin/*.dart
       // .../packages/*/lib/main.dart
       // .../packages/*/test/*_test.dart
       // .../packages/*/test/*/*_test.dart
+      // .../packages/*/benchmark/*/*_bench.dart
       Directory packages = new Directory(path.join(ArtifactStore.flutterRoot, 'packages'));
       for (FileSystemEntity entry in packages.listSync()) {
         if (entry is Directory) {
@@ -126,12 +95,29 @@ class AnalyzeCommand extends FlutterCommand {
                 for (FileSystemEntity subentry in entry.listSync()) {
                   if (subentry is File && subentry.path.endsWith('_test.dart')) {
                     dartFiles.add(subentry.path);
-                    foundTest = true;
+                    foundOne = true;
                   }
                 }
               } else if (entry is File && entry.path.endsWith('_test.dart')) {
                 dartFiles.add(entry.path);
-                foundTest = true;
+                foundOne = true;
+              }
+            }
+          }
+
+          Directory benchmarkDirectory = new Directory(path.join(entry.path, 'benchmark'));
+          if (benchmarkDirectory.existsSync()) {
+            for (FileSystemEntity entry in benchmarkDirectory.listSync()) {
+              if (entry is Directory) {
+                for (FileSystemEntity subentry in entry.listSync()) {
+                  if (subentry is File && subentry.path.endsWith('_bench.dart')) {
+                    dartFiles.add(subentry.path);
+                    foundOne = true;
+                  }
+                }
+              } else if (entry is File && entry.path.endsWith('_bench.dart')) {
+                dartFiles.add(entry.path);
+                foundOne = true;
               }
             }
           }

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

@@ -16,14 +16,16 @@ import '../test/loader.dart' as loader;
 
 class TestCommand extends FlutterCommand {
   String get name => 'test';
-  String get description => 'Runs Flutter unit tests for the current project. At least one of --debug and --release must be set.';
+  String get description => 'Runs Flutter unit tests for the current project.';
 
   bool get requiresProjectRoot => false;
 
   String get projectRootValidationErrorMessage {
     return 'Error: No pubspec.yaml file found.\n'
-      'If you wish to run the tests in the flutter repo, pass --flutter-repo before\n'
-      'any test paths. Otherwise, run this command from the root of your project.';
+      'If you wish to run the tests in the Flutter repository\'s \'flutter\' package,\n'
+      'pass --flutter-repo before any test paths. Otherwise, run this command from the\n'
+      'root of your project. Test files must be called *_test.dart and must reside in\n'
+      'the package\'s \'test\' directory (or one of its subdirectories).';
   }
 
   Future<String> _getShellPath(BuildConfiguration config) async {
@@ -44,7 +46,7 @@ class TestCommand extends FlutterCommand {
   }
 
   TestCommand() {
-    argParser.addFlag('flutter-repo', help: 'Run tests from the Flutter repository instead of the current directory.', defaultsTo: false);
+    argParser.addFlag('flutter-repo', help: 'Run tests from the \'flutter\' package in the Flutter repository instead of the current directory.', defaultsTo: false);
   }
 
   Iterable<String> _findTests(Directory directory) {
@@ -54,7 +56,13 @@ class TestCommand extends FlutterCommand {
   }
 
   Directory get _flutterUnitTestDir {
-    return new Directory(path.join(ArtifactStore.flutterRoot, 'packages', 'unit', 'test'));
+    return new Directory(path.join(ArtifactStore.flutterRoot, 'packages', 'flutter', 'test'));
+  }
+
+  Directory get _currentPackageTestDir {
+    // We don't scan the entire package, only the test/ subdirectory, so that
+    // files with names like like "hit_test.dart" don't get run.
+    return new Directory('test');
   }
 
   Future<int> _runTests(List<String> testArgs, Directory testDirectory) async {
@@ -76,11 +84,11 @@ class TestCommand extends FlutterCommand {
       return 1;
 
     // If we're running the flutter tests, we want to use the packages directory
-    // from the unit package in order to find the proper shell binary.
+    // from the flutter package in order to find the proper shell binary.
     if (runFlutterTests && ArtifactStore.packageRoot == 'packages')
-      ArtifactStore.packageRoot = path.join(ArtifactStore.flutterRoot, 'packages', 'unit', 'packages');
+      ArtifactStore.packageRoot = path.join(ArtifactStore.flutterRoot, 'packages', 'flutter', 'packages');
 
-    Directory testDir = runFlutterTests ? _flutterUnitTestDir : Directory.current;
+    Directory testDir = runFlutterTests ? _flutterUnitTestDir : _currentPackageTestDir;
 
     if (testArgs.isEmpty)
       testArgs.addAll(_findTests(testDir));

packages/flutter_tools/pubspec.yaml

@@ -16,12 +16,18 @@ dependencies:
   mustache4dart: ^1.0.0
   path: ^1.3.0
   stack_trace: ^1.4.0
-  test: 0.12.6+1
+  test: 0.12.6+1 # see note below
   yaml: ^2.1.3
 
   flx:
     path: ../flx
 
+# A note about 'test':
+# We depend on very specific internal implementation details of the
+# 'test' package, which change between versions, so here we pin it
+# precisely. This is also pinned in the 'flutter' package.
+# See also https://github.com/dart-lang/test/pull/320
+
 # A note about 'analyzer':
 # We don't actually depend on 'analyzer', but 'test' does. We aren't
 # compatible with some older versions of 'analyzer'. We lie here,

packages/flx/pubspec.yaml

@@ -13,4 +13,6 @@ environment:
   sdk: '>=1.12.0 <2.0.0'
 
 dev_dependencies:
-  test: 0.12.6
+  flutter_tools:
+    path: ../flutter_tools
+  test: any # constrained by the dependency in flutter_tools

packages/newton/pubspec.yaml

@@ -6,6 +6,6 @@ homepage: https://github.com/flutter/flutter/tree/master/packages/newton
 environment:
   sdk: '>=1.0.0 <2.0.0'
 dev_dependencies:
-  test: 0.12.6+1
-  test_runner: '<=0.2.16'
-  dart_coveralls: '<=0.3.0'
+  flutter_tools:
+    path: ../flutter_tools
+  test: any # constrained by the dependency in flutter_tools

packages/playfair/pubspec.yaml

@@ -9,7 +9,9 @@ dependencies:
     path: ../flutter
 
 dev_dependencies:
-  test: 0.12.6+1
+  flutter_tools:
+    path: ../flutter_tools
+  test: any # constrained by the dependency in flutter_tools
 
 environment:
   sdk: '>=1.12.0 <2.0.0'

packages/unit/pubspec.yaml

-name: flutter_unit_tests
-dependencies:
-  flx:
-    path: ../flx
-  flutter:
-    path: ../flutter
-  flutter_test:
-    path: ../flutter_test
-  stocks:
-    path: ../../examples/stocks

travis/setup.sh

@@ -8,7 +8,7 @@ fi
 set -x
 
 dart dev/update_packages.dart
-(cd packages/unit; ../../bin/flutter cache populate)
+(cd packages/flutter; ../../bin/flutter cache populate)
 
 if [ $TRAVIS_PULL_REQUEST = "false" ]; then
   export CLOUDSDK_CORE_DISABLE_PROMPTS=1

travis/test.sh

@@ -6,12 +6,11 @@ export PATH="$PWD/bin:$PATH"
 # analyze all the Dart code in the repo
 flutter analyze --flutter-repo --no-current-directory --no-current-package --congratulate
 
-# flutter package tests
-flutter test --flutter-repo
-
 (cd packages/cassowary; pub run test -j1)
+(cd packages/flutter; flutter test)
 # (cd packages/flutter_sprites; ) # No tests to run.
 (cd packages/flutter_tools; pub run test -j1)
+# (cd packages/flutter_test; ) # No tests to run.
 (cd packages/flx; pub run test -j1)
 (cd packages/newton; pub run test -j1)
 # (cd packages/playfair; ) # No tests to run.