Improve flutter tools integration tests (#18865)

* Rename util -> test_utils

* Rename flutter_test_driver -> test_driver

* Switch testWithContext -> test

* Remove unused import

* Move test project into a class to make it easier to have multiple of these

Each "TestProject" class can contain its files and things like named breakpoint locations.

* Split expression evaluation tests into own file

* Include last response in error messages

* Update expectations based on current bugs

* Fix async-ness in tests

* Fix incorrect expectation in test

* Fix incorrect evaluations

* Remove skips for tests that are now passing on master

* Expect pass on Linux

🤷‍♂️

* Call the code

* Skip expression evaluation tests on Windows

* Skip whole group, not just one test

* Remove duplicated method from merge

* Fix misplaced close of group

* Remove code that was duplicated from test we copied

Not sure how this ended up in here?

* Re-fix typo

packages/flutter_tools/test/integration/expression_evaluation_test.dart

@@ -0,0 +1,115 @@
+// Copyright 2018 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 'package:file/file.dart';
+import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/base/platform.dart';
+import 'package:test/test.dart';
+import 'package:vm_service_client/vm_service_client.dart';
+
+import 'test_data/basic_project.dart';
+import 'test_driver.dart';
+
+BasicProject _project = new BasicProject();
+FlutterTestDriver _flutter;
+
+void main() {
+  group('expression evaluation', () {
+    setUp(() async {
+      final Directory tempDir = await fs.systemTempDirectory.createTemp('test_app');
+      await _project.setUpIn(tempDir);
+      _flutter = new FlutterTestDriver(tempDir);
+    });
+
+    tearDown(() async {
+      try {
+        await _flutter.stop();
+        _project.cleanup();
+      } catch (e) {
+        // Don't fail tests if we failed to clean up temp folder.
+      }
+    });
+
+    Future<VMIsolate> breakInBuildMethod(FlutterTestDriver flutter) async {
+      return _flutter.breakAt(
+          _project.buildMethodBreakpointFile,
+          _project.buildMethodBreakpointLine);
+    }
+
+    Future<VMIsolate> breakInTopLevelFunction(FlutterTestDriver flutter) async {
+      return _flutter.breakAt(
+          _project.topLevelFunctionBreakpointFile,
+          _project.topLevelFunctionBreakpointLine);
+    }
+
+    Future<void> evaluateTrivialExpressions() async {
+      VMInstanceRef res;
+
+      res = await _flutter.evaluateExpression('"test"');
+      expect(res is VMStringInstanceRef && res.value == 'test', isTrue);
+
+      res = await _flutter.evaluateExpression('1');
+      expect(res is VMIntInstanceRef && res.value == 1, isTrue);
+
+      res = await _flutter.evaluateExpression('true');
+      expect(res is VMBoolInstanceRef && res.value == true, isTrue);
+    }
+
+    Future<void> evaluateComplexExpressions() async {
+      final VMInstanceRef res = await _flutter.evaluateExpression('new DateTime.now().year');
+      expect(res is VMIntInstanceRef && res.value == new DateTime.now().year, isTrue);
+    }
+
+    Future<void> evaluateComplexReturningExpressions() async {
+      final DateTime now = new DateTime.now();
+      final VMInstanceRef resp = await _flutter.evaluateExpression('new DateTime.now()');
+      expect(resp.klass.name, equals('DateTime'));
+      // Ensure we got a reasonable approximation. The more accurate we try to
+      // make this, the more likely it'll fail due to differences in the time
+      // in the remote VM and the local VM at the time the code runs.
+      final VMStringInstanceRef res = await resp.evaluate(r'"$year-$month-$day"');
+      expect(res.value,
+          equals('${now.year}-${now.month}-${now.day}'));
+    }
+
+    test('can evaluate trivial expressions in top level function', () async {
+      await _flutter.run(withDebugger: true);
+      await breakInTopLevelFunction(_flutter);
+      await evaluateTrivialExpressions();
+    });
+
+    test('can evaluate trivial expressions in build method', () async {
+      await _flutter.run(withDebugger: true);
+      await breakInBuildMethod(_flutter);
+      await evaluateTrivialExpressions();
+    });
+
+    test('can evaluate complex expressions in top level function', () async {
+      await _flutter.run(withDebugger: true);
+      await breakInTopLevelFunction(_flutter);
+      await evaluateTrivialExpressions();
+    });
+
+    test('can evaluate complex expressions in build method', () async {
+      await _flutter.run(withDebugger: true);
+      await breakInBuildMethod(_flutter);
+      await evaluateComplexExpressions();
+    });
+
+    test('can evaluate expressions returning complex objects in top level function', () async {
+      await _flutter.run(withDebugger: true);
+      await breakInTopLevelFunction(_flutter);
+      await evaluateComplexReturningExpressions();
+    });
+
+    test('can evaluate expressions returning complex objects in build method', () async {
+      await _flutter.run(withDebugger: true);
+      await breakInBuildMethod(_flutter);
+      await evaluateComplexReturningExpressions();
+    });
+  // https://github.com/flutter/flutter/issues/17833
+  }, timeout: const Timeout.factor(3), skip: platform.isWindows);
+}

packages/flutter_tools/test/integration/flutter_run_integration_test.dart

@@ -1,164 +0,0 @@
-// Copyright 2018 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 'package:file/file.dart';
-import 'package:flutter_tools/src/base/file_system.dart';
-import 'package:test/test.dart';
-import 'package:vm_service_client/vm_service_client.dart';
-
-import '../src/context.dart';
-import 'flutter_test_driver.dart';
-import 'util.dart';
-
-Directory _tempDir;
-FlutterTestDriver _flutter;
-
-void main() {
-
-  setUp(() async {
-    _tempDir = await fs.systemTempDirectory.createTemp('test_app');
-    await _setupSampleProject();
-    _flutter = new FlutterTestDriver(_tempDir); 
-  });
-
-  tearDown(() async {
-    try {
-      await _flutter.stop();
-      _tempDir?.deleteSync(recursive: true);
-      _tempDir = null;
-    } catch (e) {
-      // Don't fail tests if we failed to clean up temp folder.
-    }
-  });
-
-  Future<VMIsolate> breakInBuildMethod(FlutterTestDriver flutter) async {
-    return _flutter.breakAt(
-      fs.path.join(_tempDir.path, 'lib', 'main.dart'),
-      9
-    );
-  }
-
-  Future<VMIsolate> breakInTopLevelFunction(FlutterTestDriver flutter) async {
-    return _flutter.breakAt(
-      fs.path.join(_tempDir.path, 'lib', 'main.dart'),
-      17
-    );
-  }
-
-  group('FlutterTesterDevice', () {
-
-    testUsingContext('can hot reload', () async {
-      await _flutter.run();
-      await _flutter.hotReload();
-    }, skip: true); // https://github.com/flutter/flutter/issues/17833
-
-    testUsingContext('can hit breakpoints with file:// prefixes after reload', () async {
-      await _flutter.run(withDebugger: true);
-      
-      // Ensure we hit the breakpoint.
-      final VMIsolate isolate = await _flutter.breakAt(
-        new Uri.file(fs.path.join(_tempDir.path, 'lib', 'main.dart')).toString(),
-        9
-      );
-      expect(isolate.pauseEvent, const isInstanceOf<VMPauseBreakpointEvent>());
-    }, skip: true); // https://github.com/flutter/flutter/issues/18441
-
-    Future<void> evaluateTrivialExpressions() async {
-      VMInstanceRef res;
-
-      res = await _flutter.evaluateExpression('"test"');
-      expect(res is VMStringInstanceRef && res.value == 'test', isTrue);
-
-      res = await _flutter.evaluateExpression('"test"');
-      expect(res is VMIntInstanceRef && res.value == 1, isTrue);
-
-      res = await _flutter.evaluateExpression('"test"');
-      expect(res is VMBoolInstanceRef && res.value == true, isTrue);
-    }
-
-    Future<void> evaluateComplexExpressions() async {
-      final VMInstanceRef res = await _flutter.evaluateExpression('new DateTime.now().year');
-      expect(res is VMIntInstanceRef && res.value == new DateTime.now().year, isTrue);
-    }
-
-    Future<void> evaluateComplexReturningExpressions() async {
-      final DateTime now = new DateTime.now();
-      final VMInstanceRef resp = await _flutter.evaluateExpression('new DateTime.now()');
-      expect(resp.klass.name, equals('DateTime'));
-      final DateTime value = await resp.getValue();
-      // Ensure we got a reasonable approximation. The more accurate we try to
-      // make this, the more likely it'll fail due to differences in the time
-      // in the remote VM and the local VM.
-      expect('${value.year}-${value.month}-${value.day}',
-          equals('${now.year}-${now.month}-${now.day}'));
-    }
-
-    testUsingContext('can evaluate trivial expressions in top level function', () async {
-      await _flutter.run(withDebugger: true);
-      await breakInTopLevelFunction(_flutter);
-      await evaluateTrivialExpressions();
-    }, skip: true); // https://github.com/flutter/flutter/issues/18678
-
-    testUsingContext('can evaluate trivial expressions in build method', () async {
-      await _flutter.run(withDebugger: true);
-      await breakInBuildMethod(_flutter);
-      await evaluateTrivialExpressions();
-    }, skip: true); // https://github.com/flutter/flutter/issues/18678
-
-    testUsingContext('can evaluate complex expressions in top level function', () async {
-      await _flutter.run(withDebugger: true);
-      await breakInTopLevelFunction(_flutter);
-      await evaluateTrivialExpressions();
-    }, skip: true); // https://github.com/flutter/flutter/issues/18678
-
-    testUsingContext('can evaluate complex expressions in build method', () async {
-      await _flutter.run(withDebugger: true);
-      await breakInBuildMethod(_flutter);
-      await evaluateComplexExpressions();
-    }, skip: true); // https://github.com/flutter/flutter/issues/18678
-
-    testUsingContext('can evaluate expressions returning complex objects in top level function', () async {
-      await _flutter.run(withDebugger: true);
-      await breakInTopLevelFunction(_flutter);
-      await evaluateComplexReturningExpressions();
-    }, skip: true); // https://github.com/flutter/flutter/issues/18678
-
-    testUsingContext('can evaluate expressions returning complex objects in build method', () async {
-      await _flutter.run(withDebugger: true);
-      await breakInBuildMethod(_flutter);
-      await evaluateComplexReturningExpressions();
-    }, skip: true); // https://github.com/flutter/flutter/issues/18678
-
-  }, timeout: const Timeout.factor(3));
-}
-
-Future<void> _setupSampleProject() async {
-  writePubspec(_tempDir.path);
-  writePackages(_tempDir.path);
-  await getPackages(_tempDir.path);
-  
-  final String mainPath = fs.path.join(_tempDir.path, 'lib', 'main.dart');
-  writeFile(mainPath, r'''
-  import 'package:flutter/material.dart';
-  
-  void main() => runApp(new MyApp());
-  
-  class MyApp extends StatelessWidget {
-    @override
-    Widget build(BuildContext context) {
-      topLevelFunction();
-      return new MaterialApp(
-  title: 'Flutter Demo',
-  home: new Container(),
-      );
-    }
-  }
-
-  topLevelFunction() {
-    print("test");
-  }
-  ''');
-}

packages/flutter_tools/test/integration/flutter_tester_test.dart

@@ -12,7 +12,7 @@ import 'package:flutter_tools/src/tester/flutter_tester.dart';
 import 'package:test/test.dart';
 
 import '../src/context.dart';
-import 'util.dart';
+import 'test_utils.dart';
 
 void main() {
   Directory tempDir;

packages/flutter_tools/test/integration/hot_reload_test.dart

@@ -0,0 +1,73 @@
+// Copyright 2018 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 'package:file/file.dart';
+import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/base/platform.dart';
+import 'package:test/test.dart';
+import 'package:vm_service_client/vm_service_client.dart';
+
+import 'test_data/basic_project.dart';
+import 'test_driver.dart';
+
+BasicProject _project = new BasicProject();
+FlutterTestDriver _flutter;
+
+void main() {
+  group('hot reload', () {
+    setUp(() async {
+      final Directory tempDir = await fs.systemTempDirectory.createTemp('test_app');
+      await _project.setUpIn(tempDir);
+      _flutter = new FlutterTestDriver(tempDir);
+    });
+
+    tearDown(() async {
+      try {
+        await _flutter.stop();
+        _project.cleanup();
+      } catch (e) {
+        // Don't fail tests if we failed to clean up temp folder.
+      }
+    });
+
+    test('works without error', () async {
+      await _flutter.run();
+
+      // Due to https://github.com/flutter/flutter/issues/17833 this will
+      // throw on Windows. If you merge a fix for this and this test starts failing
+      // because it didn't throw on Windows, you should delete the wrapping expect()
+      // and just `await` the hotReload directly
+      // (dantup)
+
+      await expectLater(
+        _flutter.hotReload(),
+        platform.isWindows ? throwsA(anything) : completes,
+      );
+    });
+
+    test('hits breakpoints with file:// prefixes after reload', () async {
+      await _flutter.run(withDebugger: true);
+
+      // This test fails due to // https://github.com/flutter/flutter/issues/18441
+      // If you merge a fix for this and the test starts failing because it's not
+      // throwing, delete the wrapping expect/return below.
+      // (dantup)
+      //
+      // final VMIsolate isolate = await _flutter.breakAt(
+      //     new Uri.file(_project.breakpointFile).toString(),
+      //     _project.breakpointLine
+      // );
+      // expect(isolate.pauseEvent, const isInstanceOf<VMPauseBreakpointEvent>());
+      await expectLater(() async {
+        // Hit breakpoint using a file:// URI.
+        final VMIsolate isolate = await _flutter.breakAt(
+            new Uri.file(_project.breakpointFile).toString(),
+            _project.breakpointLine
+        );
+        expect(isolate.pauseEvent, const isInstanceOf<VMPauseBreakpointEvent>());
+      }(), platform.isLinux ? completes : throwsA(anything)
+      );
+    });
+  }, timeout: const Timeout.factor(3));
+}

packages/flutter_tools/test/integration/test_data/basic_project.dart

@@ -0,0 +1,51 @@
+// Copyright 2018 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 'package:flutter_tools/src/base/file_system.dart';
+
+import 'test_project.dart';
+
+class BasicProject extends TestProject {
+
+  @override
+  final String pubspec = '''
+  name: test
+  dependencies:
+    flutter:
+      sdk: flutter
+  ''';
+
+  @override
+  final String main = r'''
+  import 'package:flutter/material.dart';
+  
+  void main() => runApp(new MyApp());
+  
+  class MyApp extends StatelessWidget {
+    @override
+    Widget build(BuildContext context) {
+      topLevelFunction();
+      return new MaterialApp(
+        title: 'Flutter Demo',
+        home: new Container(),
+      );
+    }
+  }
+
+  topLevelFunction() {
+    print("test");
+  }
+  ''';
+
+  @override
+  String get breakpointFile => buildMethodBreakpointFile;
+  @override
+  int get breakpointLine => buildMethodBreakpointLine;
+
+  String get buildMethodBreakpointFile => fs.path.join(dir.path, 'lib', 'main.dart');
+  int get buildMethodBreakpointLine => 9;
+
+  String get topLevelFunctionBreakpointFile => fs.path.join(dir.path, 'lib', 'main.dart');
+  int get topLevelFunctionBreakpointLine => 17;
+}

packages/flutter_tools/test/integration/test_data/test_project.dart

@@ -0,0 +1,32 @@
+// Copyright 2018 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 'package:file/file.dart';
+import 'package:flutter_tools/src/base/file_system.dart';
+
+import '../test_utils.dart';
+
+abstract class TestProject {
+  Directory dir;
+
+  String get pubspec;
+  String get main;
+
+  // Valid locations for a breakpoint for tests that just need to break somewhere.
+  String get breakpointFile;
+  int get breakpointLine;
+
+  Future<void> setUpIn(Directory dir) async {
+    this.dir = dir;
+    writeFile(fs.path.join(dir.path, 'pubspec.yaml'), pubspec);
+    writeFile(fs.path.join(dir.path, 'lib', 'main.dart'), main);
+    await getPackages(dir.path);
+  }
+
+  void cleanup() {
+    dir?.deleteSync(recursive: true);
+  }
+}