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
diff --git a/.analysis_options b/.analysis_options index f6381c5..ad2d78a 100644 --- a/.analysis_options +++ b/.analysis_options
@@ -26,6 +26,7 @@ todo: ignore exclude: - 'bin/cache/**' + - 'packages/flutter_tools/test/data/dart_dependencies_test/**' linter: rules:
diff --git a/.analysis_options_repo b/.analysis_options_repo index 1fe4c4f..7f3fdd3 100644 --- a/.analysis_options_repo +++ b/.analysis_options_repo
@@ -27,6 +27,7 @@ todo: ignore exclude: - 'bin/cache/**' + - 'packages/flutter_tools/test/data/dart_dependencies_test/**' linter: rules:
diff --git a/packages/flutter_tools/lib/src/dart/dependencies.dart b/packages/flutter_tools/lib/src/dart/dependencies.dart new file mode 100644 index 0000000..04240ce --- /dev/null +++ b/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; + } +}
diff --git a/packages/flutter_tools/lib/src/dart/package_map.dart b/packages/flutter_tools/lib/src/dart/package_map.dart index 53ad511..8ea0dbc 100644 --- a/packages/flutter_tools/lib/src/dart/package_map.dart +++ b/packages/flutter_tools/lib/src/dart/package_map.dart
@@ -29,12 +29,27 @@ 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;
diff --git a/packages/flutter_tools/lib/src/dependency_checker.dart b/packages/flutter_tools/lib/src/dependency_checker.dart new file mode 100644 index 0000000..88deffd --- /dev/null +++ b/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; + } +}
diff --git a/packages/flutter_tools/lib/src/hot.dart b/packages/flutter_tools/lib/src/hot.dart index e7d545b..0ec3de2 100644 --- a/packages/flutter_tools/lib/src/hot.dart +++ b/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 '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 @@ 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 @@ 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);
diff --git a/packages/flutter_tools/test/all.dart b/packages/flutter_tools/test/all.dart index de2c8a9..c47203e 100644 --- a/packages/flutter_tools/test/all.dart +++ b/packages/flutter_tools/test/all.dart
@@ -18,6 +18,8 @@ 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 @@ 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();
diff --git a/packages/flutter_tools/test/dart_dependencies_test.dart b/packages/flutter_tools/test/dart_dependencies_test.dart new file mode 100644 index 0000000..068bded --- /dev/null +++ b/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); + } + }); + }); +}
diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/.dartignore b/packages/flutter_tools/test/data/dart_dependencies_test/.dartignore new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/.dartignore
diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/.gitignore b/packages/flutter_tools/test/data/dart_dependencies_test/.gitignore new file mode 100644 index 0000000..160ff88 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/.gitignore
@@ -0,0 +1 @@ +!.packages
diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/good/.analysis_options b/packages/flutter_tools/test/data/dart_dependencies_test/good/.analysis_options new file mode 100644 index 0000000..cb5e559 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/good/.analysis_options
@@ -0,0 +1,3 @@ +analyzer: + exclude: + - '**'
diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/good/.packages b/packages/flutter_tools/test/data/dart_dependencies_test/good/.packages new file mode 100644 index 0000000..453dc36 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/good/.packages
@@ -0,0 +1 @@ +self:lib/
diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/good/foo.dart b/packages/flutter_tools/test/data/dart_dependencies_test/good/foo.dart new file mode 100644 index 0000000..6853db7 --- /dev/null +++ b/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.
diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/good/lib/bar.dart b/packages/flutter_tools/test/data/dart_dependencies_test/good/lib/bar.dart new file mode 100644 index 0000000..6853db7 --- /dev/null +++ b/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.
diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/good/main.dart b/packages/flutter_tools/test/data/dart_dependencies_test/good/main.dart new file mode 100644 index 0000000..659a48d --- /dev/null +++ b/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';
diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/good/pubspec.yaml b/packages/flutter_tools/test/data/dart_dependencies_test/good/pubspec.yaml new file mode 100644 index 0000000..99a0109 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/good/pubspec.yaml
@@ -0,0 +1 @@ +name: self
diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/.analysis_options b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/.analysis_options new file mode 100644 index 0000000..cb5e559 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/.analysis_options
@@ -0,0 +1,3 @@ +analyzer: + exclude: + - '**'
diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/.packages b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/.packages new file mode 100644 index 0000000..453dc36 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/.packages
@@ -0,0 +1 @@ +self:lib/
diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/foo.dart b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/foo.dart new file mode 100644 index 0000000..b72678b6 --- /dev/null +++ b/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!
diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/main.dart b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/main.dart new file mode 100644 index 0000000..43d48c0 --- /dev/null +++ b/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';
diff --git a/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/pubspec.yaml b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/pubspec.yaml new file mode 100644 index 0000000..99a0109 --- /dev/null +++ b/packages/flutter_tools/test/data/dart_dependencies_test/syntax_error/pubspec.yaml
@@ -0,0 +1 @@ +name: self
diff --git a/packages/flutter_tools/test/dependency_checker_test.dart b/packages/flutter_tools/test/dependency_checker_test.dart new file mode 100644 index 0000000..5ae8fa5 --- /dev/null +++ b/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); + }); + }); +}
diff --git a/packages/flutter_tools/test/devfs_test.dart b/packages/flutter_tools/test/devfs_test.dart index eb6907d..179ed90 100644 --- a/packages/flutter_tools/test/devfs_test.dart +++ b/packages/flutter_tools/test/devfs_test.dart
@@ -10,6 +10,7 @@ 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 @@ }); 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();
diff --git a/packages/flutter_tools/test/src/common.dart b/packages/flutter_tools/test/src/common.dart index f85f2d9..a5d6733 100644 --- a/packages/flutter_tools/test/src/common.dart +++ b/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 @@ 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]); +}
diff --git a/packages/flutter_tools/test/src/context.dart b/packages/flutter_tools/test/src/context.dart index 35a286d..92eeb46 100644 --- a/packages/flutter_tools/test/src/context.dart +++ b/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/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 @@ 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 @@ 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) {