From df6e4d4872fb17e480dd6fc6a6700746301eabc9 Mon Sep 17 00:00:00 2001 From: David Shuckerow Date: Fri, 28 Jun 2019 10:37:27 -0700 Subject: [PATCH] Make it possible to override the FLUTTER_TEST env variable (#34301) * Make it possible to override the FLUTTER_TEST env variable without unsetting it. * Switch to using platform instead of Platform. * Document the bindings, and introduce tests that initialize multiple WidgetsBindings with different environments. * Add tests for the flutter platform test. * Add license headers * Fix lints * Remove trailing whitespace * Respond to Jonahs comments * Respond to Ians comments * Mock out the HttpServer in flutter_platform_test * Mock out the HttpServer in flutter_platform_test * Explain why we mock out the HttpServer in flutter_platform_test --- packages/flutter_test/lib/src/binding.dart | 26 ++++- .../flutter_test_variable_is_false_test.dart | 16 +++ ...st_variable_is_not_true_or_false_test.dart | 14 +++ .../flutter_test_variable_is_null_test.dart | 14 +++ .../flutter_test_variable_is_true_test.dart | 14 +++ .../no_flutter_test_variable_test.dart | 14 +++ .../lib/src/test/flutter_platform.dart | 22 ++++- .../test/flutter_platform_test.dart | 98 +++++++++++++++++-- 8 files changed, 204 insertions(+), 14 deletions(-) create mode 100644 packages/flutter_test/test/bindings_environment/flutter_test_variable_is_false_test.dart create mode 100644 packages/flutter_test/test/bindings_environment/flutter_test_variable_is_not_true_or_false_test.dart create mode 100644 packages/flutter_test/test/bindings_environment/flutter_test_variable_is_null_test.dart create mode 100644 packages/flutter_test/test/bindings_environment/flutter_test_variable_is_true_test.dart create mode 100644 packages/flutter_test/test/bindings_environment/no_flutter_test_variable_test.dart diff --git a/packages/flutter_test/lib/src/binding.dart b/packages/flutter_test/lib/src/binding.dart index 492810989e..21507916d8 100644 --- a/packages/flutter_test/lib/src/binding.dart +++ b/packages/flutter_test/lib/src/binding.dart @@ -173,11 +173,29 @@ abstract class TestWidgetsFlutterBinding extends BindingBase /// This function will use [AutomatedTestWidgetsFlutterBinding] if /// the test was run using `flutter test`, and /// [LiveTestWidgetsFlutterBinding] otherwise (e.g. if it was run - /// using `flutter run`). (This is determined by looking at the - /// environment variables for a variable called `FLUTTER_TEST`.) - static WidgetsBinding ensureInitialized() { + /// using `flutter run`). This is determined by looking at the + /// environment variables for a variable called `FLUTTER_TEST`. + /// + /// If `FLUTTER_TEST` is set with a value of 'true', then this test was + /// invoked by `flutter test`. If `FLUTTER_TEST` is not set, or if it is set + /// to 'false', then this test was invoked by `flutter run`. + /// + /// Browser environments do not currently support the + /// [LiveTestWidgetsFlutterBinding], so this function will always set up an + /// [AutomatedTestWidgetsFlutterBinding] when run in a web browser. + /// + /// The parameter `environment` is exposed to test different environment + /// variable values, and should not be used. + static WidgetsBinding ensureInitialized([@visibleForTesting Map environment]) { + if (!isBrowser) { + // Accessing Platform may throw from a browser, so we guard this. + environment ??= Platform.environment; + } if (WidgetsBinding.instance == null) { - if (isBrowser || Platform.environment.containsKey('FLUTTER_TEST')) { + if (isBrowser) { + // Browser environments do not support the LiveTestWidgetsFlutterBinding. + AutomatedTestWidgetsFlutterBinding(); + } else if (environment.containsKey('FLUTTER_TEST') && environment['FLUTTER_TEST'] != 'false') { AutomatedTestWidgetsFlutterBinding(); } else { LiveTestWidgetsFlutterBinding(); diff --git a/packages/flutter_test/test/bindings_environment/flutter_test_variable_is_false_test.dart b/packages/flutter_test/test/bindings_environment/flutter_test_variable_is_false_test.dart new file mode 100644 index 0000000000..926dcafaae --- /dev/null +++ b/packages/flutter_test/test/bindings_environment/flutter_test_variable_is_false_test.dart @@ -0,0 +1,16 @@ +// Copyright 2019 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/material.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + test('$WidgetsBinding initializes with $LiveTestWidgetsFlutterBinding when FLUTTER_TEST = "false"', () { + TestWidgetsFlutterBinding.ensureInitialized({'FLUTTER_TEST': 'false'}); + expect(WidgetsBinding.instance, isInstanceOf()); + }, onPlatform: const { + 'browser': [Skip('Browser will not use the live binding')] + }); +} diff --git a/packages/flutter_test/test/bindings_environment/flutter_test_variable_is_not_true_or_false_test.dart b/packages/flutter_test/test/bindings_environment/flutter_test_variable_is_not_true_or_false_test.dart new file mode 100644 index 0000000000..ceafb3bac4 --- /dev/null +++ b/packages/flutter_test/test/bindings_environment/flutter_test_variable_is_not_true_or_false_test.dart @@ -0,0 +1,14 @@ +// Copyright 2019 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/material.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + test('$WidgetsBinding initializes with $AutomatedTestWidgetsFlutterBinding when FLUTTER_TEST has a value that is not "true" or "false"', () { + TestWidgetsFlutterBinding.ensureInitialized({'FLUTTER_TEST': 'value that is neither "true" nor "false"'}); + expect(WidgetsBinding.instance, isInstanceOf()); + }); +} diff --git a/packages/flutter_test/test/bindings_environment/flutter_test_variable_is_null_test.dart b/packages/flutter_test/test/bindings_environment/flutter_test_variable_is_null_test.dart new file mode 100644 index 0000000000..7636fe83dd --- /dev/null +++ b/packages/flutter_test/test/bindings_environment/flutter_test_variable_is_null_test.dart @@ -0,0 +1,14 @@ +// Copyright 2019 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/material.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + test('$WidgetsBinding initializes with $AutomatedTestWidgetsFlutterBinding when FLUTTER_TEST is defined but null', () { + TestWidgetsFlutterBinding.ensureInitialized({'FLUTTER_TEST': null}); + expect(WidgetsBinding.instance, isInstanceOf()); + }); +} diff --git a/packages/flutter_test/test/bindings_environment/flutter_test_variable_is_true_test.dart b/packages/flutter_test/test/bindings_environment/flutter_test_variable_is_true_test.dart new file mode 100644 index 0000000000..a14c4cd2f5 --- /dev/null +++ b/packages/flutter_test/test/bindings_environment/flutter_test_variable_is_true_test.dart @@ -0,0 +1,14 @@ +// Copyright 2019 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/material.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + test('$WidgetsBinding initializes with $AutomatedTestWidgetsFlutterBinding when FLUTTER_TEST = "true"', () { + TestWidgetsFlutterBinding.ensureInitialized({'FLUTTER_TEST': 'true'}); + expect(WidgetsBinding.instance, isInstanceOf()); + }); +} diff --git a/packages/flutter_test/test/bindings_environment/no_flutter_test_variable_test.dart b/packages/flutter_test/test/bindings_environment/no_flutter_test_variable_test.dart new file mode 100644 index 0000000000..3e3a095ce1 --- /dev/null +++ b/packages/flutter_test/test/bindings_environment/no_flutter_test_variable_test.dart @@ -0,0 +1,14 @@ +// Copyright 2019 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/material.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + test('$WidgetsBinding initializes with $LiveTestWidgetsFlutterBinding when the environment does not contain FLUTTER_TEST', () { + TestWidgetsFlutterBinding.ensureInitialized({}); + expect(WidgetsBinding.instance, isInstanceOf()); + }); +} diff --git a/packages/flutter_tools/lib/src/test/flutter_platform.dart b/packages/flutter_tools/lib/src/test/flutter_platform.dart index 83f2bef02d..64e50ad5b9 100644 --- a/packages/flutter_tools/lib/src/test/flutter_platform.dart +++ b/packages/flutter_tools/lib/src/test/flutter_platform.dart @@ -19,6 +19,7 @@ import 'package:test_core/src/runner/environment.dart'; // ignore: implementatio import '../base/common.dart'; import '../base/file_system.dart'; import '../base/io.dart'; +import '../base/platform.dart'; import '../base/process_manager.dart'; import '../compile.dart'; import '../convert.dart'; @@ -384,6 +385,13 @@ class FlutterPlatform extends PlatformPlugin { throw 'Failed to compile $expression'; } + /// Binds an [HttpServer] serving from `host` on `port`. + /// + /// Only intended to be overridden in tests for [FlutterPlatform]. + @protected + @visibleForTesting + Future bind(InternetAddress host, int port) => HttpServer.bind(host, port); + Future<_AsyncError> _startTest( String testPath, StreamChannel controller, @@ -403,7 +411,7 @@ class FlutterPlatform extends PlatformPlugin { })); // Prepare our WebSocket server to talk to the engine subproces. - final HttpServer server = await HttpServer.bind(host, port); + final HttpServer server = await bind(host, port); finalizers.add(() async { printTrace('test $ourTestCount: shutting down test harness socket server'); await server.close(force: true); @@ -846,14 +854,22 @@ class FlutterPlatform extends PlatformPlugin { testPath, ]); printTrace(command.join(' ')); + // If the FLUTTER_TEST environment variable has been set, then pass it on + // for package:flutter_test to handle the value. + // + // If FLUTTER_TEST has not been set, assume from this context that this + // call was invoked by the command 'flutter test'. + final String flutterTest = platform.environment.containsKey('FLUTTER_TEST') + ? platform.environment['FLUTTER_TEST'] + : 'true'; final Map environment = { - 'FLUTTER_TEST': 'true', + 'FLUTTER_TEST': flutterTest, 'FONTCONFIG_FILE': _fontConfigFile.path, 'SERVER_PORT': serverPort.toString(), }; if (buildTestAssets) { environment['UNIT_TEST_ASSETS'] = fs.path.join( - flutterProject.directory.path, 'build', 'unit_test_assets'); + flutterProject?.directory?.path ?? '', 'build', 'unit_test_assets'); } return processManager.start(command, environment: environment); } diff --git a/packages/flutter_tools/test/flutter_platform_test.dart b/packages/flutter_tools/test/flutter_platform_test.dart index 83f5c92a32..1e33201ecb 100644 --- a/packages/flutter_tools/test/flutter_platform_test.dart +++ b/packages/flutter_tools/test/flutter_platform_test.dart @@ -4,9 +4,12 @@ import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/io.dart'; +import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/test/flutter_platform.dart'; +import 'package:meta/meta.dart'; import 'package:mockito/mockito.dart'; +import 'package:process/process.dart'; import 'package:test_core/backend.dart'; import 'src/common.dart'; @@ -15,15 +18,70 @@ import 'src/context.dart'; void main() { group('FlutterPlatform', () { testUsingContext('ensureConfiguration throws an error if an explicitObservatoryPort is specified and more than one test file', () async { - final FlutterPlatform flutterPlatfrom = FlutterPlatform(shellPath: '/', explicitObservatoryPort: 1234); - flutterPlatfrom.loadChannel('test1.dart', MockPlatform()); - expect(() => flutterPlatfrom.loadChannel('test2.dart', MockPlatform()), throwsA(isA())); + final FlutterPlatform flutterPlatform = FlutterPlatform(shellPath: '/', explicitObservatoryPort: 1234); + flutterPlatform.loadChannel('test1.dart', MockSuitePlatform()); + expect(() => flutterPlatform.loadChannel('test2.dart', MockSuitePlatform()), throwsA(isA())); }); testUsingContext('ensureConfiguration throws an error if a precompiled entrypoint is specified and more that one test file', () { - final FlutterPlatform flutterPlatfrom = FlutterPlatform(shellPath: '/', precompiledDillPath: 'example.dill'); - flutterPlatfrom.loadChannel('test1.dart', MockPlatform()); - expect(() => flutterPlatfrom.loadChannel('test2.dart', MockPlatform()), throwsA(isA())); + final FlutterPlatform flutterPlatform = FlutterPlatform(shellPath: '/', precompiledDillPath: 'example.dill'); + flutterPlatform.loadChannel('test1.dart', MockSuitePlatform()); + expect(() => flutterPlatform.loadChannel('test2.dart', MockSuitePlatform()), throwsA(isA())); + }); + + group('The FLUTTER_TEST environment variable is passed to the test process', () { + MockPlatform mockPlatform; + MockProcessManager mockProcessManager; + FlutterPlatform flutterPlatform; + final Map contextOverrides = { + Platform: () => mockPlatform, + ProcessManager: () => mockProcessManager, + }; + + setUp(() { + mockPlatform = MockPlatform(); + mockProcessManager = MockProcessManager(); + flutterPlatform = TestFlutterPlatform(); + }); + + Future> captureEnvironment() async { + flutterPlatform.loadChannel('test1.dart', MockSuitePlatform()); + await untilCalled(mockProcessManager.start(any, environment: anyNamed('environment'))); + final VerificationResult toVerify = verify(mockProcessManager.start(any, environment: captureAnyNamed('environment'))); + expect(toVerify.captured, hasLength(1)); + expect(toVerify.captured.first, isInstanceOf>()); + return toVerify.captured.first; + } + + testUsingContext('as true when not originally set', () async { + when(mockPlatform.environment).thenReturn({}); + final Map capturedEnvironment = await captureEnvironment(); + expect(capturedEnvironment['FLUTTER_TEST'], 'true'); + }, overrides: contextOverrides); + + testUsingContext('as true when set to true', () async { + when(mockPlatform.environment).thenReturn({'FLUTTER_TEST': 'true'}); + final Map capturedEnvironment = await captureEnvironment(); + expect(capturedEnvironment['FLUTTER_TEST'], 'true'); + }, overrides: contextOverrides); + + testUsingContext('as false when set to false', () async { + when(mockPlatform.environment).thenReturn({'FLUTTER_TEST': 'false'}); + final Map capturedEnvironment = await captureEnvironment(); + expect(capturedEnvironment['FLUTTER_TEST'], 'false'); + }, overrides: contextOverrides); + + testUsingContext('unchanged when set', () async { + when(mockPlatform.environment).thenReturn({'FLUTTER_TEST': 'neither true nor false'}); + final Map capturedEnvironment = await captureEnvironment(); + expect(capturedEnvironment['FLUTTER_TEST'], 'neither true nor false'); + }, overrides: contextOverrides); + + testUsingContext('as null when set to null', () async { + when(mockPlatform.environment).thenReturn({'FLUTTER_TEST': null}); + final Map capturedEnvironment = await captureEnvironment(); + expect(capturedEnvironment['FLUTTER_TEST'], null); + }, overrides: contextOverrides); }); testUsingContext('installHook creates a FlutterPlatform', () { @@ -80,4 +138,30 @@ void main() { }); } -class MockPlatform extends Mock implements SuitePlatform {} +class MockSuitePlatform extends Mock implements SuitePlatform {} + +class MockProcessManager extends Mock implements ProcessManager {} + +class MockPlatform extends Mock implements Platform {} + +class MockHttpServer extends Mock implements HttpServer {} + +// A FlutterPlatform with enough fields set to load and start a test. +// +// Uses a mock HttpServer. We don't want to bind random ports in our CI hosts. +class TestFlutterPlatform extends FlutterPlatform { + TestFlutterPlatform() : super( + shellPath: '/', + precompiledDillPath: 'example.dill', + host: InternetAddress.loopbackIPv6, + port: 0, + updateGoldens: false, + startPaused: false, + enableObservatory: false, + buildTestAssets: false, + ); + + @override + @protected + Future bind(InternetAddress host, int port) async => MockHttpServer(); +} \ No newline at end of file