Follow-on work to goldens (#17267)

* Exclude flutter_goldens package from dartdoc because it's for internal
  use only
* Document why flutter_tools doesn' tneed to be excluded from the list of
  packages to document
* Performance optimization in the flutter comparator, and associated
  test updates.

dev/tools/dartdoc.dart

@@ -121,13 +121,15 @@ Future<Null> main(List<String> arguments) async {
     dartdocBaseArgs.add('--no-validate-links');
   }
   // Generate the documentation.
+  // We don't need to exclude flutter_tools in this list because it's not in the
+  // recursive dependencies of the package defined at dev/docs/pubspec.yaml
   final List<String> dartdocArgs = <String>[]..addAll(dartdocBaseArgs)..addAll(<String>[
     '--header', 'styles.html',
     '--header', 'analytics.html',
     '--header', 'survey.html',
     '--footer-text', 'lib/footer.html',
     '--exclude-packages',
-'analyzer,args,barback,cli_util,csslib,front_end,glob,html,http_multi_server,io,isolate,js,kernel,logging,mime,mockito,node_preamble,plugin,shelf,shelf_packages_handler,shelf_static,shelf_web_socket,utf,watcher,yaml',
+'analyzer,args,barback,cli_util,csslib,flutter_goldens,front_end,glob,html,http_multi_server,io,isolate,js,kernel,logging,mime,mockito,node_preamble,plugin,shelf,shelf_packages_handler,shelf_static,shelf_web_socket,utf,watcher,yaml',
     '--exclude',
   'package:Flutter/temp_doc.dart,package:http/browser_client.dart,package:intl/intl_browser.dart,package:matcher/mirror_matchers.dart,package:quiver/mirrors.dart,package:quiver/io.dart,package:vm_service_client/vm_service_client.dart,package:web_socket_channel/html.dart',
     '--favicon=favicon.ico',

packages/flutter_goldens/lib/flutter_goldens.dart

@@ -35,15 +35,21 @@ Future<void> main(FutureOr<void> testMain()) async {
 /// the `$FLUTTER_ROOT/bin/cache/pkg/goldens` folder, then perform the comparison against
 /// the files therein.
 class FlutterGoldenFileComparator implements GoldenFileComparator {
+  /// Creates a [FlutterGoldenFileComparator] that will resolve golden file
+  /// URIs relative to the specified [basedir].
+  ///
+  /// The [fs] parameter exists for testing purposes only.
   @visibleForTesting
   FlutterGoldenFileComparator(
-    this.goldens,
     this.basedir, {
     this.fs: const LocalFileSystem(),
   });
 
-  final GoldensClient goldens;
+  /// The directory to which golden file URIs will be resolved in [compare] and [update].
   final Uri basedir;
+
+  /// The file system used to perform file access.
+  @visibleForTesting
   final FileSystem fs;
 
   /// Creates a new [FlutterGoldenFileComparator] that mirrors the relative
@@ -51,11 +57,24 @@ class FlutterGoldenFileComparator implements GoldenFileComparator {
   ///
   /// By the time the future completes, the clone of the `flutter/goldens`
   /// repository is guaranteed to be ready use.
-  static Future<FlutterGoldenFileComparator> fromDefaultComparator() async {
+  ///
-    final LocalFileComparator defaultComparator = goldenFileComparator;
+  /// The [goldens] and [defaultComparator] parameters are visible for testing
-    final GoldensClient goldens = new GoldensClient();
+  /// purposes only.
+  static Future<FlutterGoldenFileComparator> fromDefaultComparator({
+    GoldensClient goldens,
+    LocalFileComparator defaultComparator,
+  }) async {
+    defaultComparator ??= goldenFileComparator;
+
+    // Prepare the goldens repo.
+    goldens ??= new GoldensClient();
     await goldens.prepare();
-    return new FlutterGoldenFileComparator(goldens, defaultComparator.basedir);
+
+    // Calculate the appropriate basedir for the current test context.
+    final FileSystem fs = goldens.fs;
+    final Directory testDirectory = fs.directory(defaultComparator.basedir);
+    final String testDirectoryRelativePath = fs.path.relative(testDirectory.path, from: goldens.flutterRoot.path);
+    return new FlutterGoldenFileComparator(goldens.repositoryRoot.childDirectory(testDirectoryRelativePath).uri);
   }
 
   @override
@@ -77,10 +96,7 @@ class FlutterGoldenFileComparator implements GoldenFileComparator {
   }
 
   File _getGoldenFile(Uri uri) {
-    final File relativeFile = fs.file(uri);
+    return fs.directory(basedir).childFile(fs.file(uri).path);
-    final Directory testDirectory = fs.directory(basedir);
-    final String relativeBase = fs.path.relative(testDirectory.path, from: goldens.flutterRoot.path);
-    return goldens.repositoryRoot.childDirectory(relativeBase).childFile(relativeFile.path);
   }
 }
 

packages/flutter_goldens/test/flutter_goldens_test.dart

@@ -61,19 +61,31 @@ void main() {
   });
 
   group('FlutterGoldenFileComparator', () {
-    GoldensClient goldens;
     MemoryFileSystem fs;
     FlutterGoldenFileComparator comparator;
 
     setUp(() {
-      goldens = new MockGoldensClient();
       fs = new MemoryFileSystem();
       final Directory flutterRoot = fs.directory('/path/to/flutter')..createSync(recursive: true);
       final Directory goldensRoot = flutterRoot.childDirectory('bin/cache/goldens')..createSync(recursive: true);
-      final Directory testDirectory = flutterRoot.childDirectory('test/foo/bar')..createSync(recursive: true);
+      final Directory testDirectory = goldensRoot.childDirectory('test/foo/bar')..createSync(recursive: true);
-      comparator = new FlutterGoldenFileComparator(goldens, new Uri.directory(testDirectory.path), fs: fs);
+      comparator = new FlutterGoldenFileComparator(testDirectory.uri, fs: fs);
-      when(goldens.flutterRoot).thenReturn(flutterRoot);
+    });
-      when(goldens.repositoryRoot).thenReturn(goldensRoot);
+
+    group('fromDefaultComparator', () {
+      test('calculates the basedir correctly', () async {
+        final MockGoldensClient goldens = new MockGoldensClient();
+        final MockLocalFileComparator defaultComparator = new MockLocalFileComparator();
+        final Directory flutterRoot = fs.directory('/foo')..createSync(recursive: true);
+        final Directory goldensRoot = flutterRoot.childDirectory('bar')..createSync(recursive: true);
+        when(goldens.fs).thenReturn(fs);
+        when(goldens.flutterRoot).thenReturn(flutterRoot);
+        when(goldens.repositoryRoot).thenReturn(goldensRoot);
+        when(defaultComparator.basedir).thenReturn(flutterRoot.childDirectory('baz').uri);
+        comparator = await FlutterGoldenFileComparator.fromDefaultComparator(
+            goldens: goldens, defaultComparator: defaultComparator);
+        expect(comparator.basedir, fs.directory('/foo/bar/baz').uri);
+      });
     });
 
     group('compare', () {
@@ -125,3 +137,4 @@ void main() {
 
 class MockProcessManager extends Mock implements ProcessManager {}
 class MockGoldensClient extends Mock implements GoldensClient {}
+class MockLocalFileComparator extends Mock implements LocalFileComparator {}

packages/flutter_test/lib/src/goldens.dart

@@ -66,8 +66,8 @@ abstract class GoldenFileComparator {
 ///
 ///  * [flutter_test] for more information about how to configure tests at the
 ///    directory-level.
-GoldenFileComparator _goldenFileComparator = const _UninitializedComparator();
 GoldenFileComparator get goldenFileComparator => _goldenFileComparator;
+GoldenFileComparator _goldenFileComparator = const _UninitializedComparator();
 set goldenFileComparator(GoldenFileComparator comparator) {
   _goldenFileComparator = comparator ?? const _UninitializedComparator();
 }