DependencyChecker with tests (#7268)

- [x] Introduce DependencyChecker which can determine if any dependencies have been modified.
- [x] Move the DartDependencyBuilder into a separate file.
- [x] Add unit tests for DartDependencyBuilder.
- [x] Add unit tets for DependencyChecker

Part of #7014

.analysis_options

@@ -26,6 +26,7 @@ analyzer:
     todo: ignore
   exclude:
     - 'bin/cache/**'
+    - 'packages/flutter_tools/test/data/dart_dependencies_test/**'
 
 linter:
   rules:

.analysis_options_repo

@@ -27,6 +27,7 @@ analyzer:
     todo: ignore
   exclude:
     - 'bin/cache/**'
+    - 'packages/flutter_tools/test/data/dart_dependencies_test/**'
 
 linter:
   rules:

packages/flutter_tools/lib/src/dart/dependencies.dart

@@ -0,0 +1,46 @@
+// 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 'dart:convert';
+
+import 'package:path/path.dart' as path;
+
+import '../base/process.dart';
+import '../toolchain.dart';
+
+class DartDependencySetBuilder {
+  DartDependencySetBuilder(this.mainScriptPath,
+                           this.projectRootPath,
+                           this.packagesFilePath);
+
+  final String mainScriptPath;
+  final String projectRootPath;
+  final String packagesFilePath;
+
+  Set<String> build() {
+    final String skySnapshotPath =
+        ToolConfiguration.instance.getHostToolPath(HostTool.SkySnapshot);
+
+    final List<String> args = <String>[
+      skySnapshotPath,
+      '--packages=$packagesFilePath',
+      '--print-deps',
+      mainScriptPath
+    ];
+
+    String output = runSyncAndThrowStdErrOnError(args);
+
+    final List<String> lines = LineSplitter.split(output).toList();
+    final Set<String> minimalDependencies = new Set<String>();
+    for (String line in lines) {
+      if (!line.startsWith('package:')) {
+        // We convert the uris so that they are relative to the project
+        // root.
+        line = path.relative(line, from: projectRootPath);
+      }
+      minimalDependencies.add(line);
+    }
+    return minimalDependencies;
+  }
+}

packages/flutter_tools/lib/src/dart/package_map.dart

@@ -29,12 +29,27 @@ class PackageMap {
 
   final String packagesPath;
 
-  Map<String, Uri> get map {
+  /// Load and parses the .packages file.
+  void load() {
     _map ??= _parse(packagesPath);
+  }
+
+  Map<String, Uri> get map {
+    load();
     return _map;
   }
   Map<String, Uri> _map;
 
+  /// Returns the path to [packageUri].
+  String pathForPackage(Uri packageUri) {
+    assert(packageUri.scheme == 'package');
+    List<String> pathSegments = packageUri.pathSegments.toList();
+    String packageName = pathSegments.removeAt(0);
+    Uri packageBase = map[packageName];
+    String packageRelativePath = path.joinAll(pathSegments);
+    return packageBase.resolve(packageRelativePath).path;
+  }
+
   String checkValid() {
     if (FileSystemEntity.isFileSync(packagesPath))
       return null;

packages/flutter_tools/lib/src/dependency_checker.dart

@@ -0,0 +1,67 @@
+// 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 'dart:io';
+
+import 'globals.dart';
+
+import 'dart/dependencies.dart';
+import 'dart/package_map.dart';
+import 'asset.dart';
+
+import 'package:path/path.dart' as pathos;
+
+class DependencyChecker {
+  final DartDependencySetBuilder builder;
+  final Set<String> _dependencies = new Set<String>();
+  final AssetBundle assets;
+  DependencyChecker(this.builder, this.assets);
+
+  /// Returns [true] if any components have been modified after [threshold] or
+  /// if it cannot be determined.
+  bool check(DateTime threshold) {
+    _dependencies.clear();
+    PackageMap packageMap;
+    // Parse the package map.
+    try {
+      packageMap = new PackageMap(builder.packagesFilePath)..load();
+      _dependencies.add(builder.packagesFilePath);
+    } catch (e, st) {
+      printTrace('DependencyChecker: could not parse .packages file:\n$e\n$st');
+      return true;
+    }
+    // Build the set of Dart dependencies.
+    try {
+      Set<String> dependencies = builder.build();
+      for (String path in dependencies) {
+        // Ensure all paths are absolute.
+        if (path.startsWith('package:')) {
+          path = packageMap.pathForPackage(Uri.parse(path));
+        } else {
+          path = pathos.join(builder.projectRootPath, path);
+        }
+        _dependencies.add(path);
+      }
+    } catch (e, st) {
+      printTrace('DependencyChecker: error determining .dart dependencies:\n$e\n$st');
+      return true;
+    }
+    // TODO(johnmccutchan): Extract dependencies from the AssetBundle too.
+
+    // Check all dependency modification times.
+    for (String path in _dependencies) {
+      File file = new File(path);
+      FileStat stat = file.statSync();
+      if (stat.type == FileSystemEntityType.NOT_FOUND) {
+        printTrace('DependencyChecker: Error stating $path.');
+        return true;
+      }
+      if (stat.modified.isAfter(threshold)) {
+        printTrace('DependencyChecker: $path is newer than $threshold');
+        return true;
+      }
+    }
+    return false;
+  }
+}

packages/flutter_tools/lib/src/hot.dart

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 import 'dart:async';
-import 'dart:convert';
 import 'dart:io';
 
 import 'package:meta/meta.dart';
@@ -14,15 +13,15 @@ import 'application_package.dart';
 import 'asset.dart';
 import 'base/context.dart';
 import 'base/logger.dart';
-import 'base/process.dart';
 import 'base/utils.dart';
 import 'build_info.dart';
 import 'dart/package_map.dart';
+import 'dart/dependencies.dart';
 import 'devfs.dart';
 import 'device.dart';
 import 'globals.dart';
 import 'resident_runner.dart';
-import 'toolchain.dart';
+
 import 'vmservice.dart';
 import 'usage.dart';
 
@@ -37,47 +36,6 @@ HotRunnerConfig get hotRunnerConfig => context[HotRunnerConfig];
 
 const bool kHotReloadDefault = true;
 
-class DartDependencySetBuilder {
-  DartDependencySetBuilder(this.mainScriptPath,
-                           this.projectRootPath,
-                           this.packagesFilePath);
-
-  final String mainScriptPath;
-  final String projectRootPath;
-  final String packagesFilePath;
-
-  Set<String> build() {
-    final String skySnapshotPath =
-        ToolConfiguration.instance.getHostToolPath(HostTool.SkySnapshot);
-
-    final List<String> args = <String>[
-      skySnapshotPath,
-      '--packages=$packagesFilePath',
-      '--print-deps',
-      mainScriptPath
-    ];
-
-    String output = runSyncAndThrowStdErrOnError(args);
-
-    final List<String> lines = LineSplitter.split(output).toList();
-    final Set<String> minimalDependencies = new Set<String>();
-    for (String line in lines) {
-      // We need to convert the uris so that they are relative to the project
-      // root and tweak package: uris so that they reflect their devFS location.
-      if (line.startsWith('package:')) {
-        // Swap out package: for packages/ because we place all package sources
-        // under packages/.
-        line = line.replaceFirst('package:', 'packages/');
-      } else {
-        // Ensure paths are relative to the project root.
-        line = path.relative(line, from: projectRootPath);
-      }
-      minimalDependencies.add(line);
-    }
-    return minimalDependencies;
-  }
-}
-
 class HotRunner extends ResidentRunner {
   HotRunner(
     Device device, {
@@ -151,7 +109,18 @@ class HotRunner extends ResidentRunner {
         new DartDependencySetBuilder(
               _mainPath, _projectRootPath, _packagesFilePath);
     try {
-      _dartDependencies = dartDependencySetBuilder.build();
+      Set<String> dependencies = dartDependencySetBuilder.build();
+      _dartDependencies = new Set<String>();
+      for (String path in dependencies) {
+        // We need to tweak package: uris so that they reflect their devFS
+        // location.
+        if (path.startsWith('package:')) {
+          // Swap out package: for packages/ because we place all package
+          // sources under packages/.
+          path = path.replaceFirst('package:', 'packages/');
+        }
+        _dartDependencies.add(path);
+      }
     } catch (error) {
       printStatus('Error detected in application source code:', emphasis: true);
       printError(error);

packages/flutter_tools/test/all.dart

@@ -18,6 +18,8 @@ import 'android_sdk_test.dart' as android_sdk_test;
 import 'asset_bundle_test.dart' as asset_bundle_test;
 import 'application_package_test.dart' as application_package_test;
 import 'base_utils_test.dart' as base_utils_test;
+import 'dart_dependencies_test.dart' as dart_dependencies_test;
+import 'dependency_checker_test.dart' as dependency_checker_test;
 import 'channel_test.dart' as channel_test;
 import 'config_test.dart' as config_test;
 import 'context_test.dart' as context_test;
@@ -55,11 +57,13 @@ void main() {
   application_package_test.main();
   asset_bundle_test.main();
   base_utils_test.main();
+  dart_dependencies_test.main();
   channel_test.main();
   config_test.main();
   context_test.main();
   create_test.main();
   daemon_test.main();
+  dependency_checker_test.main();
   devfs_test.main();
   device_test.main();
   devices_test.main();

packages/flutter_tools/test/dart_dependencies_test.dart

@@ -0,0 +1,41 @@
+// 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 'dart:io';
+
+import 'package:flutter_tools/src/dart/dependencies.dart';
+import 'package:path/path.dart' as path;
+import 'package:test/test.dart';
+import 'src/context.dart';
+
+void main()  {
+  group('DartDependencySetBuilder', () {
+    final String basePath = path.dirname(Platform.script.path);
+    final String dataPath = path.join(basePath, 'data', 'dart_dependencies_test');
+    testUsingContext('good', () {
+      final String testPath = path.join(dataPath, 'good');
+      final String mainPath = path.join(testPath, 'main.dart');
+      final String packagesPath = path.join(testPath, '.packages');
+      DartDependencySetBuilder builder =
+          new DartDependencySetBuilder(mainPath, testPath, packagesPath);
+      Set<String> dependencies = builder.build();
+      expect(dependencies.contains('main.dart'), isTrue);
+      expect(dependencies.contains('foo.dart'), isTrue);
+    });
+    testUsingContext('syntax_error', () {
+      final String testPath = path.join(dataPath, 'syntax_error');
+      final String mainPath = path.join(testPath, 'main.dart');
+      final String packagesPath = path.join(testPath, '.packages');
+      DartDependencySetBuilder builder =
+          new DartDependencySetBuilder(mainPath, testPath, packagesPath);
+      try {
+        builder.build();
+        fail('expect an assertion to be thrown.');
+      } catch (e) {
+        expect(e, const isInstanceOf<String>());
+        expect(e.contains('unexpected token \'bad\''), isTrue);
+      }
+    });
+  });
+}

packages/flutter_tools/test/data/dart_dependencies_test/.gitignore

@@ -0,0 +1 @@
+!.packages

packages/flutter_tools/test/data/dart_dependencies_test/good/.analysis_options

@@ -0,0 +1,3 @@
+analyzer:
+ exclude:
+    - '**'

packages/flutter_tools/test/data/dart_dependencies_test/good/.packages

@@ -0,0 +1 @@
+self:lib/

packages/flutter_tools/test/data/dart_dependencies_test/good/foo.dart

@@ -0,0 +1,3 @@
+// 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.

packages/flutter_tools/test/data/dart_dependencies_test/good/lib/bar.dart

@@ -0,0 +1,3 @@
+// 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.

packages/flutter_tools/test/data/dart_dependencies_test/good/main.dart

@@ -0,0 +1,6 @@
+// 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 'foo.dart';
+import 'package:self/bar.dart';

packages/flutter_tools/test/data/dart_dependencies_test/good/pubspec.yaml

@@ -0,0 +1 @@
+name: self

packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/.analysis_options

@@ -0,0 +1,3 @@
+analyzer:
+ exclude:
+    - '**'

packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/.packages

@@ -0,0 +1 @@
+self:lib/

packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/foo.dart

@@ -0,0 +1,5 @@
+// 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.
+
+bad programmer!

packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/main.dart

@@ -0,0 +1,5 @@
+// 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 'foo.dart';

packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/pubspec.yaml

@@ -0,0 +1 @@
+name: self

packages/flutter_tools/test/dependency_checker_test.dart

@@ -0,0 +1,72 @@
+// 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 'dart:io';
+
+import 'package:flutter_tools/src/dart/dependencies.dart';
+import 'package:flutter_tools/src/dependency_checker.dart';
+import 'package:path/path.dart' as path;
+import 'package:test/test.dart';
+import 'src/common.dart';
+import 'src/context.dart';
+
+void main()  {
+  group('DependencyChecker', () {
+    final String basePath = path.dirname(Platform.script.path);
+    final String dataPath = path.join(basePath, 'data', 'dart_dependencies_test');
+    testUsingContext('good', () {
+      final String testPath = path.join(dataPath, 'good');
+      final String mainPath = path.join(testPath, 'main.dart');
+      final String fooPath = path.join(testPath, 'foo.dart');
+      final String barPath = path.join(testPath, 'lib', 'bar.dart');
+      final String packagesPath = path.join(testPath, '.packages');
+      DartDependencySetBuilder builder =
+          new DartDependencySetBuilder(mainPath, testPath, packagesPath);
+      DependencyChecker dependencyChecker =
+          new DependencyChecker(builder, null);
+
+      // Set file modification time on all dependencies to be in the past.
+      DateTime baseTime = new DateTime.now();
+      updateFileModificationTime(packagesPath, baseTime, -10);
+      updateFileModificationTime(mainPath, baseTime, -10);
+      updateFileModificationTime(fooPath, baseTime, -10);
+      updateFileModificationTime(barPath, baseTime, -10);
+      expect(dependencyChecker.check(baseTime), isFalse);
+
+      // Set .packages file modification time to be in the future.
+      updateFileModificationTime(packagesPath, baseTime, 20);
+      expect(dependencyChecker.check(baseTime), isTrue);
+
+      // Reset .packages file modification time.
+      updateFileModificationTime(packagesPath, baseTime, 0);
+      expect(dependencyChecker.check(baseTime), isFalse);
+
+      // Set 'package:self/bar.dart' file modification time to be in the future.
+      updateFileModificationTime(barPath, baseTime, 10);
+      expect(dependencyChecker.check(baseTime), isTrue);
+    });
+    testUsingContext('syntax error', () {
+      final String testPath = path.join(dataPath, 'syntax_error');
+      final String mainPath = path.join(testPath, 'main.dart');
+      final String fooPath = path.join(testPath, 'foo.dart');
+      final String packagesPath = path.join(testPath, '.packages');
+
+      DartDependencySetBuilder builder =
+          new DartDependencySetBuilder(mainPath, testPath, packagesPath);
+      DependencyChecker dependencyChecker =
+          new DependencyChecker(builder, null);
+
+      DateTime baseTime = new DateTime.now();
+
+      // Set file modification time on all dependencies to be in the past.
+      updateFileModificationTime(packagesPath, baseTime, -10);
+      updateFileModificationTime(mainPath, baseTime, -10);
+      updateFileModificationTime(fooPath, baseTime, -10);
+
+      // Dependencies are considered dirty because there is a syntax error in
+      // the .dart file.
+      expect(dependencyChecker.check(baseTime), isTrue);
+    });
+  });
+}

packages/flutter_tools/test/devfs_test.dart

@@ -10,6 +10,7 @@ import 'package:flutter_tools/src/devfs.dart';
 import 'package:path/path.dart' as path;
 import 'package:test/test.dart';
 
+import 'src/common.dart';
 import 'src/context.dart';
 import 'src/mocks.dart';
 
@@ -48,8 +49,8 @@ void main() {
     });
     testUsingContext('modify existing file on local file system', () async {
       File file = new File(path.join(basePath, filePath));
-      // Set the last modified time to 5 seconds ago.
-      Process.runSync('touch', <String>['-A', '-05', file.path]);
+      // Set the last modified time to 5 seconds in the past.
+      updateFileModificationTime(file.path, new DateTime.now(), -5);
       await devFS.update();
       await file.writeAsBytes(<int>[1, 2, 3, 4, 5, 6]);
       await devFS.update();

packages/flutter_tools/test/src/common.dart

@@ -3,6 +3,9 @@
 // found in the LICENSE file.
 
 import 'package:args/command_runner.dart';
+
+import 'package:flutter_tools/src/base/context.dart';
+import 'package:flutter_tools/src/base/process_manager.dart';
 import 'package:flutter_tools/src/runner/flutter_command.dart';
 import 'package:flutter_tools/src/runner/flutter_command_runner.dart';
 
@@ -12,3 +15,19 @@ CommandRunner<Null> createTestCommandRunner([FlutterCommand command]) {
     runner.addCommand(command);
   return runner;
 }
+
+/// Updates [path] to have a modification time [seconds] from now.
+void updateFileModificationTime(String path,
+                                DateTime baseTime,
+                                int seconds) {
+  DateTime modificationTime = baseTime.add(new Duration(seconds: seconds));
+  String argument =
+      '${modificationTime.year}'
+      '${modificationTime.month.toString().padLeft(2, "0")}'
+      '${modificationTime.day.toString().padLeft(2, "0")}'
+      '${modificationTime.hour.toString().padLeft(2, "0")}'
+      '${modificationTime.minute.toString().padLeft(2, "0")}'
+      '.${modificationTime.second.toString().padLeft(2, "0")}';
+  ProcessManager processManager = context[ProcessManager];
+  processManager.runSync('touch', <String>['-t', argument, path]);
+}

packages/flutter_tools/test/src/context.dart

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:io';
 
 import 'package:flutter_tools/src/base/config.dart';
 import 'package:flutter_tools/src/base/context.dart';
@@ -20,6 +21,7 @@ import 'package:flutter_tools/src/toolchain.dart';
 import 'package:flutter_tools/src/usage.dart';
 
 import 'package:mockito/mockito.dart';
+import 'package:path/path.dart' as path;
 import 'package:test/test.dart';
 
 /// Return the test logger. This assumes that the current Logger is a BufferLogger.
@@ -64,6 +66,9 @@ void testUsingContext(String description, dynamic testMethod(), {
     testContext.putIfAbsent(SimControl, () => new MockSimControl());
     testContext.putIfAbsent(Usage, () => new MockUsage());
 
+    final String basePath = path.dirname(Platform.script.path);
+    final String flutterRoot =
+        path.normalize(path.join(basePath, '..', '..', '..'));
     try {
       return await testContext.runInZone(() {
         // Apply the overrides to the test context in the zone since their
@@ -71,7 +76,9 @@ void testUsingContext(String description, dynamic testMethod(), {
         overrides.forEach((Type type, dynamic value()) {
           context.setVariable(type, value());
         });
-
+        // Provide a sane default for the flutterRoot directory. Individual
+        // tests can override this.
+        Cache.flutterRoot = flutterRoot;
         return testMethod();
       });
     } catch (error) {