Make tests more hermetic. (#8765)

1. Add `PortScanner` abstraction so that we don't do actual port scanning
   in tests.
2. Don't change the real `cwd` of the isolate during tests, as it affects
   all tests, not just the current running test.

Fixes #8761

packages/flutter_tools/lib/src/android/android_device.dart

@@ -10,7 +10,7 @@ import '../application_package.dart';
 import '../base/file_system.dart';
 import '../base/io.dart';
 import '../base/logger.dart';
-import '../base/os.dart';
+import '../base/port_scanner.dart';
 import '../base/process.dart';
 import '../base/process_manager.dart';
 import '../build_info.dart';
@@ -724,7 +724,7 @@ class _AndroidDevicePortForwarder extends DevicePortForwarder {
   Future<int> forward(int devicePort, { int hostPort }) async {
     if ((hostPort == null) || (hostPort == 0)) {
       // Auto select host port.
-      hostPort = await findAvailablePort();
+      hostPort = await portScanner.findAvailablePort();
     }
 
     runCheckedSync(device.adbCommandForDevice(

packages/flutter_tools/lib/src/base/os.dart

@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:async';
-
 import 'package:archive/archive.dart';
 import 'context.dart';
 import 'file_system.dart';
@@ -154,41 +152,6 @@ class _WindowsUtils extends OperatingSystemUtils {
   }
 }
 
-Future<int> findAvailablePort() async {
-  final ServerSocket socket = await ServerSocket.bind(InternetAddress.LOOPBACK_IP_V4, 0);
-  final int port = socket.port;
-  await socket.close();
-  return port;
-}
-
-const int _kMaxSearchIterations = 20;
-
-/// This method will attempt to return a port close to or the same as
-/// [defaultPort]. Failing that, it will return any available port.
-Future<int> findPreferredPort(int defaultPort, { int searchStep: 2 }) async {
-  int iterationCount = 0;
-
-  while (iterationCount < _kMaxSearchIterations) {
-    final int port = defaultPort + iterationCount * searchStep;
-    if (await _isPortAvailable(port))
-      return port;
-    iterationCount++;
-  }
-
-  return findAvailablePort();
-}
-
-Future<bool> _isPortAvailable(int port) async {
-  try {
-    // TODO(ianh): This is super racy.
-    final ServerSocket socket = await ServerSocket.bind(InternetAddress.LOOPBACK_IP_V4, port);
-    await socket.close();
-    return true;
-  } catch (error) {
-    return false;
-  }
-}
-
 /// Find and return the project root directory relative to the specified
 /// directory or the current working directory if none specified.
 /// Return `null` if the project root could not be found

packages/flutter_tools/lib/src/base/port_scanner.dart

+// Copyright 2017 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:async';
+
+import 'context.dart';
+import 'io.dart';
+
+const PortScanner _kLocalPortScanner = const HostPortScanner();
+const int _kMaxSearchIterations = 20;
+
+PortScanner get portScanner {
+  return context == null
+      ? _kLocalPortScanner
+      : context.putIfAbsent(PortScanner, () => _kLocalPortScanner);
+}
+
+abstract class PortScanner {
+  const PortScanner();
+
+  /// Returns true if the specified [port] is available to bind to.
+  Future<bool> isPortAvailable(int port);
+
+  /// Returns an available ephemeral port.
+  Future<int> findAvailablePort();
+
+  /// Returns an available port as close to [defaultPort] as possible.
+  ///
+  /// If [defaultPort] is available, this will return it. Otherwise, it will
+  /// search for an avaiable port close to [defaultPort]. If it cannot find one,
+  /// it will return any available port.
+  Future<int> findPreferredPort(int defaultPort, { int searchStep: 2 }) async {
+    int iterationCount = 0;
+
+    while (iterationCount < _kMaxSearchIterations) {
+      final int port = defaultPort + iterationCount * searchStep;
+      if (await isPortAvailable(port))
+        return port;
+      iterationCount++;
+    }
+
+    return findAvailablePort();
+  }
+}
+
+class HostPortScanner extends PortScanner {
+  const HostPortScanner();
+
+  @override
+  Future<bool> isPortAvailable(int port) async {
+    try {
+      // TODO(ianh): This is super racy.
+      final ServerSocket socket = await ServerSocket.bind(InternetAddress.LOOPBACK_IP_V4, port);
+      await socket.close();
+      return true;
+    } catch (error) {
+      return false;
+    }
+  }
+
+  @override
+  Future<int> findAvailablePort() async {
+    final ServerSocket socket = await ServerSocket.bind(InternetAddress.LOOPBACK_IP_V4, 0);
+    final int port = socket.port;
+    await socket.close();
+    return port;
+  }
+}

packages/flutter_tools/lib/src/device.dart

@@ -10,7 +10,7 @@ import 'application_package.dart';
 import 'base/common.dart';
 import 'base/context.dart';
 import 'base/file_system.dart';
-import 'base/os.dart';
+import 'base/port_scanner.dart';
 import 'base/utils.dart';
 import 'build_info.dart';
 import 'devfs.dart';
@@ -308,7 +308,7 @@ class DebuggingOptions {
   Future<int> findBestObservatoryPort() {
     if (hasObservatoryPort)
       return new Future<int>.value(observatoryPort);
-    return findPreferredPort(observatoryPort ?? kDefaultObservatoryPort);
+    return portScanner.findPreferredPort(observatoryPort ?? kDefaultObservatoryPort);
   }
 
   bool get hasDiagnosticPort => diagnosticPort != null;
@@ -318,7 +318,7 @@ class DebuggingOptions {
   Future<int> findBestDiagnosticPort() {
     if (hasDiagnosticPort)
       return new Future<int>.value(diagnosticPort);
-    return findPreferredPort(diagnosticPort ?? kDefaultDiagnosticPort);
+    return portScanner.findPreferredPort(diagnosticPort ?? kDefaultDiagnosticPort);
   }
 }
 

packages/flutter_tools/lib/src/ios/devices.dart

@@ -8,8 +8,8 @@ import 'dart:convert';
 import '../application_package.dart';
 import '../base/file_system.dart';
 import '../base/io.dart';
-import '../base/os.dart';
 import '../base/platform.dart';
+import '../base/port_scanner.dart';
 import '../base/process.dart';
 import '../base/process_manager.dart';
 import '../build_info.dart';
@@ -447,7 +447,7 @@ class _IOSDevicePortForwarder extends DevicePortForwarder {
   Future<int> forward(int devicePort, {int hostPort: null}) async {
     if ((hostPort == null) || (hostPort == 0)) {
       // Auto select host port.
-      hostPort = await findAvailablePort();
+      hostPort = await portScanner.findAvailablePort();
     }
 
     // Usage: iproxy LOCAL_TCP_PORT DEVICE_TCP_PORT UDID

packages/flutter_tools/lib/src/protocol_discovery.dart

@@ -5,7 +5,7 @@
 import 'dart:async';
 
 import 'base/common.dart';
-import 'base/os.dart';
+import 'base/port_scanner.dart';
 import 'device.dart';
 import 'globals.dart';
 
@@ -59,7 +59,7 @@ class ProtocolDiscovery {
     Uri hostUri;
     if (portForwarder != null) {
       final int devicePort = deviceUri.port;
-      hostPort ??= await findPreferredPort(defaultHostPort);
+      hostPort ??= await portScanner.findPreferredPort(defaultHostPort);
       hostPort = await portForwarder
           .forward(devicePort, hostPort: hostPort)
           .timeout(const Duration(seconds: 60), onTimeout: () {

packages/flutter_tools/test/analyze_test.dart

@@ -2,49 +2,48 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:file/file.dart';
+import 'package:file/memory.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/commands/analyze_base.dart';
-import 'package:flutter_tools/src/runner/flutter_command_runner.dart';
 import 'package:test/test.dart';
 
 import 'src/context.dart';
 
+const String _kFlutterRoot = '/data/flutter';
+
 void main() {
+  FileSystem fs;
   Directory tempDir;
 
   setUp(() {
-    FlutterCommandRunner.initFlutterRoot();
+    fs = new MemoryFileSystem();
+    fs.directory(_kFlutterRoot).createSync(recursive: true);
+    Cache.flutterRoot = _kFlutterRoot;
     tempDir = fs.systemTempDirectory.createTempSync('analysis_test');
   });
 
-  tearDown(() {
-    tempDir?.deleteSync(recursive: true);
-  });
-
   group('analyze', () {
-
     testUsingContext('inRepo', () {
       // Absolute paths
       expect(inRepo(<String>[tempDir.path]), isFalse);
       expect(inRepo(<String>[fs.path.join(tempDir.path, 'foo')]), isFalse);
       expect(inRepo(<String>[Cache.flutterRoot]), isTrue);
       expect(inRepo(<String>[fs.path.join(Cache.flutterRoot, 'foo')]), isTrue);
+
       // Relative paths
-      final String oldWorkingDirectory = fs.currentDirectory.path;
-      try {
-        fs.currentDirectory = Cache.flutterRoot;
-        expect(inRepo(<String>['.']), isTrue);
-        expect(inRepo(<String>['foo']), isTrue);
-        fs.currentDirectory = tempDir.path;
-        expect(inRepo(<String>['.']), isFalse);
-        expect(inRepo(<String>['foo']), isFalse);
-      } finally {
-        fs.currentDirectory = oldWorkingDirectory;
-      }
+      fs.currentDirectory = Cache.flutterRoot;
+      expect(inRepo(<String>['.']), isTrue);
+      expect(inRepo(<String>['foo']), isTrue);
+      fs.currentDirectory = tempDir.path;
+      expect(inRepo(<String>['.']), isFalse);
+      expect(inRepo(<String>['foo']), isFalse);
+
       // Ensure no exceptions
       inRepo(null);
       inRepo(<String>[]);
+    }, overrides: <Type, Generator>{
+      FileSystem: () => fs,
     });
   });
 }

packages/flutter_tools/test/replay/common.dart

@@ -7,7 +7,9 @@ import 'dart:async';
 import 'package:file/local.dart';
 import 'package:flutter_tools/executable.dart' as tools;
 import 'package:flutter_tools/src/cache.dart';
+import 'package:flutter_tools/src/base/context.dart';
 import 'package:flutter_tools/src/base/io.dart' as io;
+import 'package:flutter_tools/src/base/port_scanner.dart';
 import 'package:flutter_tools/src/runner/flutter_command.dart';
 import 'package:test/test.dart';
 
@@ -40,7 +42,9 @@ void testReplay(
     timeout: timeout,
     overrides: overrides,
     skip: skip,
-    initializeContext: (_) {},
+    initializeContext: (AppContext testContext) {
+      testContext.putIfAbsent(PortScanner, () => new MockPortScanner());
+    },
   );
 }
 

packages/flutter_tools/test/src/context.dart

@@ -11,6 +11,7 @@ import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/base/os.dart';
 import 'package:flutter_tools/src/base/platform.dart';
+import 'package:flutter_tools/src/base/port_scanner.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/devfs.dart';
 import 'package:flutter_tools/src/device.dart';
@@ -36,21 +37,23 @@ typedef dynamic Generator();
 typedef void ContextInitializer(AppContext testContext);
 
 void _defaultInitializeContext(AppContext testContext) {
-  testContext.putIfAbsent(DeviceManager, () => new MockDeviceManager());
-  testContext.putIfAbsent(DevFSConfig, () => new DevFSConfig());
-  testContext.putIfAbsent(Doctor, () => new MockDoctor());
-  testContext.putIfAbsent(HotRunnerConfig, () => new HotRunnerConfig());
-  testContext.putIfAbsent(Cache, () => new Cache());
-  testContext.putIfAbsent(Artifacts, () => new CachedArtifacts());
-  testContext.putIfAbsent(OperatingSystemUtils, () => new MockOperatingSystemUtils());
-  testContext.putIfAbsent(Xcode, () => new Xcode());
-  testContext.putIfAbsent(IOSSimulatorUtils, () {
-    final MockIOSSimulatorUtils mock = new MockIOSSimulatorUtils();
-    when(mock.getAttachedDevices()).thenReturn(<IOSSimulator>[]);
-    return mock;
-  });
-  testContext.putIfAbsent(SimControl, () => new MockSimControl());
-  testContext.putIfAbsent(Usage, () => new MockUsage());
+  testContext
+    ..putIfAbsent(DeviceManager, () => new MockDeviceManager())
+    ..putIfAbsent(DevFSConfig, () => new DevFSConfig())
+    ..putIfAbsent(Doctor, () => new MockDoctor())
+    ..putIfAbsent(HotRunnerConfig, () => new HotRunnerConfig())
+    ..putIfAbsent(Cache, () => new Cache())
+    ..putIfAbsent(Artifacts, () => new CachedArtifacts())
+    ..putIfAbsent(OperatingSystemUtils, () => new MockOperatingSystemUtils())
+    ..putIfAbsent(PortScanner, () => new MockPortScanner())
+    ..putIfAbsent(Xcode, () => new Xcode())
+    ..putIfAbsent(IOSSimulatorUtils, () {
+      final MockIOSSimulatorUtils mock = new MockIOSSimulatorUtils();
+      when(mock.getAttachedDevices()).thenReturn(<IOSSimulator>[]);
+      return mock;
+    })
+    ..putIfAbsent(SimControl, () => new MockSimControl())
+    ..putIfAbsent(Usage, () => new MockUsage());
 }
 
 void testUsingContext(String description, dynamic testMethod(), {
@@ -63,11 +66,12 @@ void testUsingContext(String description, dynamic testMethod(), {
     final AppContext testContext = new AppContext();
 
     // The context always starts with these value since others depend on them.
-    testContext.putIfAbsent(Platform, () => const LocalPlatform());
-    testContext.putIfAbsent(FileSystem, () => const LocalFileSystem());
-    testContext.putIfAbsent(ProcessManager, () => const LocalProcessManager());
-    testContext.putIfAbsent(Logger, () => new BufferLogger());
-    testContext.putIfAbsent(Config, () => new Config());
+    testContext
+      ..putIfAbsent(Platform, () => const LocalPlatform())
+      ..putIfAbsent(FileSystem, () => const LocalFileSystem())
+      ..putIfAbsent(ProcessManager, () => const LocalProcessManager())
+      ..putIfAbsent(Logger, () => new BufferLogger())
+      ..putIfAbsent(Config, () => new Config());
 
     // Apply the initializer after seeding the base value above.
     initializeContext(testContext);
@@ -81,9 +85,11 @@ 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;
+        // tests can override this either in the test or during setup.
+        Cache.flutterRoot ??= flutterRoot;
+
         return await testMethod();
       }, onError: (dynamic error, StackTrace stackTrace) {
         _printBufferedErrors(testContext);
@@ -106,6 +112,16 @@ void _printBufferedErrors(AppContext testContext) {
   }
 }
 
+class MockPortScanner extends PortScanner {
+  static int _nextAvailablePort = 12345;
+
+  @override
+  Future<bool> isPortAvailable(int port) async => true;
+
+  @override
+  Future<int> findAvailablePort() async => _nextAvailablePort++;
+}
+
 class MockDeviceManager implements DeviceManager {
   List<Device> devices = <Device>[];