From 7ab044286875f032ce3fdf0df7a7ef0f90d687a5 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 1 Dec 2020 09:53:50 -0800 Subject: [PATCH] Add testing shard for release mode guard (#71411) --- dev/bots/test.dart | 7 ++++ packages/flutter/test_release/README.md | 3 ++ .../test_release/diagnostics_test.dart | 40 +++++++++++++++++++ .../test_release/precondition_test.dart | 14 +++++++ packages/flutter_tools/lib/src/compile.dart | 11 +++-- .../build_system/targets/common_test.dart | 14 +++---- .../test/general.shard/compile_test.dart | 8 ++++ 7 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 packages/flutter/test_release/README.md create mode 100644 packages/flutter/test_release/diagnostics_test.dart create mode 100644 packages/flutter/test_release/precondition_test.dart diff --git a/dev/bots/test.dart b/dev/bots/test.dart index 61d43830695..66c622dda5b 100644 --- a/dev/bots/test.dart +++ b/dev/bots/test.dart @@ -652,6 +652,13 @@ Future _runFrameworkTests() async { tableData: bigqueryApi?.tabledata, ); } + // Run release mode tests (see packages/flutter/test_release/README.md) + await _runFlutterTest( + path.join(flutterRoot, 'packages', 'flutter'), + options: ['--dart-define=dart.vm.product=true', ...soundNullSafetyOptions], + tableData: bigqueryApi?.tabledata, + tests: [ 'test_release' + path.separator ], + ); } Future runLibraries() async { diff --git a/packages/flutter/test_release/README.md b/packages/flutter/test_release/README.md new file mode 100644 index 00000000000..97d66a62b95 --- /dev/null +++ b/packages/flutter/test_release/README.md @@ -0,0 +1,3 @@ +This folder contains unit tests that are run with `kReleaseMode` set to true. This can be used for unit testing code that is guarded by this boolean, such as in cases where debug code is intentionally ellided for performance or code size reasons. The unit tests are still run in the VM and are not otherwise precompiled. + +To run these test from the command line, use the command: `flutter test --dart-define=dart.vm.product=true test_release/` diff --git a/packages/flutter/test_release/diagnostics_test.dart b/packages/flutter/test_release/diagnostics_test.dart new file mode 100644 index 00000000000..038137d5b80 --- /dev/null +++ b/packages/flutter/test_release/diagnostics_test.dart @@ -0,0 +1,40 @@ +// Copyright 2014 The Flutter 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/foundation.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + test('TextTreeRenderer returns an empty string in release mode', () { + final TextTreeRenderer renderer = TextTreeRenderer(); + final TestDiagnosticsNode node = TestDiagnosticsNode(); + + expect(renderer.render(node), ''); + }); +} + +class TestDiagnosticsNode extends DiagnosticsNode { + TestDiagnosticsNode() : super( + name: 'test', + style: DiagnosticsTreeStyle.singleLine, + ); + + @override + List getChildren() { + return []; + } + + @override + List getProperties() { + return []; + } + + @override + String? toDescription({TextTreeConfiguration? parentConfiguration}) { + return 'Test Description'; + } + + @override + final Object? value = Object(); +} diff --git a/packages/flutter/test_release/precondition_test.dart b/packages/flutter/test_release/precondition_test.dart new file mode 100644 index 00000000000..22c4d264665 --- /dev/null +++ b/packages/flutter/test_release/precondition_test.dart @@ -0,0 +1,14 @@ +// Copyright 2014 The Flutter 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/foundation.dart'; +import 'package:flutter_test/flutter_test.dart'; + +// This test verifies that the test_release shard is configured correctly. +// See README.md in this directory for more information. +void main() { + test('kReleaseMode is set to true', () { + expect(kReleaseMode, true); + }); +} diff --git a/packages/flutter_tools/lib/src/compile.dart b/packages/flutter_tools/lib/src/compile.dart index 9f8ed8d3c00..8ef7e054a01 100644 --- a/packages/flutter_tools/lib/src/compile.dart +++ b/packages/flutter_tools/lib/src/compile.dart @@ -146,12 +146,15 @@ class StdoutHandler { } /// List the preconfigured build options for a given build mode. -List buildModeOptions(BuildMode mode) { +List buildModeOptions(BuildMode mode, List dartDefines) { switch (mode) { case BuildMode.debug: return [ '-Ddart.vm.profile=false', - '-Ddart.vm.product=false', + // This allows the CLI to override the value of this define for unit + // testing the framework. + if (!dartDefines.any((String define) => define.startsWith('dart.vm.product'))) + '-Ddart.vm.product=false', '--enable-asserts', ]; case BuildMode.profile: @@ -241,7 +244,7 @@ class KernelCompiler { '-Ddart.developer.causal_async_stacks=${buildMode == BuildMode.debug}', for (final Object dartDefine in dartDefines) '-D$dartDefine', - ...buildModeOptions(buildMode), + ...buildModeOptions(buildMode, dartDefines), if (trackWidgetCreation) '--track-widget-creation', if (!linkPlatformKernelIn) '--no-link-platform', if (aot) ...[ @@ -672,7 +675,7 @@ class DefaultResidentCompiler implements ResidentCompiler { '--packages', packagesPath, ], - ...buildModeOptions(buildMode), + ...buildModeOptions(buildMode, dartDefines), if (trackWidgetCreation) '--track-widget-creation', if (fileSystemRoots != null) for (final String root in fileSystemRoots) ...[ diff --git a/packages/flutter_tools/test/general.shard/build_system/targets/common_test.dart b/packages/flutter_tools/test/general.shard/build_system/targets/common_test.dart index 772a1cf2176..2e460400b3b 100644 --- a/packages/flutter_tools/test/general.shard/build_system/targets/common_test.dart +++ b/packages/flutter_tools/test/general.shard/build_system/targets/common_test.dart @@ -90,7 +90,7 @@ void main() { '--target=flutter', '--no-print-incremental-dependencies', '-Ddart.developer.causal_async_stacks=false', - ...buildModeOptions(BuildMode.profile), + ...buildModeOptions(BuildMode.profile, []), '--aot', '--tfa', '--packages', @@ -127,7 +127,7 @@ void main() { '--target=flutter', '--no-print-incremental-dependencies', '-Ddart.developer.causal_async_stacks=false', - ...buildModeOptions(BuildMode.profile), + ...buildModeOptions(BuildMode.profile, []), '--aot', '--tfa', '--packages', @@ -164,7 +164,7 @@ void main() { '--target=flutter', '--no-print-incremental-dependencies', '-Ddart.developer.causal_async_stacks=false', - ...buildModeOptions(BuildMode.profile), + ...buildModeOptions(BuildMode.profile, []), '--aot', '--tfa', '--packages', @@ -202,7 +202,7 @@ void main() { '--target=flutter', '--no-print-incremental-dependencies', '-Ddart.developer.causal_async_stacks=false', - ...buildModeOptions(BuildMode.profile), + ...buildModeOptions(BuildMode.profile, []), '--aot', '--tfa', '--packages', @@ -242,7 +242,7 @@ void main() { '--target=flutter', '--no-print-incremental-dependencies', '-Ddart.developer.causal_async_stacks=true', - ...buildModeOptions(BuildMode.debug), + ...buildModeOptions(BuildMode.debug, []), '--no-link-platform', '--packages', '/.dart_tool/package_config.json', @@ -280,7 +280,7 @@ void main() { '--target=flutter', '--no-print-incremental-dependencies', '-Ddart.developer.causal_async_stacks=true', - ...buildModeOptions(BuildMode.debug), + ...buildModeOptions(BuildMode.debug, []), '--packages', '/.dart_tool/package_config.json', '--output-dill', @@ -330,7 +330,7 @@ void main() { '--target=flutter', '--no-print-incremental-dependencies', '-Ddart.developer.causal_async_stacks=true', - ...buildModeOptions(BuildMode.debug), + ...buildModeOptions(BuildMode.debug, []), '--track-widget-creation', '--no-link-platform', '--packages', diff --git a/packages/flutter_tools/test/general.shard/compile_test.dart b/packages/flutter_tools/test/general.shard/compile_test.dart index ac91c615afe..4540f0ca383 100644 --- a/packages/flutter_tools/test/general.shard/compile_test.dart +++ b/packages/flutter_tools/test/general.shard/compile_test.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'package:flutter_tools/src/base/logger.dart'; +import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/compile.dart'; import '../src/common.dart'; @@ -43,4 +44,11 @@ void main() { expect(toMultiRootPath(Uri.parse('org-dartlang-app:///a/b/c'), null, [], false), 'org-dartlang-app:///a/b/c'); expect(toMultiRootPath(Uri.parse('org-dartlang-app:///a/b/c'), 'scheme', ['/d/b'], false), 'org-dartlang-app:///a/b/c'); }); + + testWithoutContext('buildModeOptions removes matching product define', () { + expect(buildModeOptions(BuildMode.debug, ['dart.vm.product=true']), [ + '-Ddart.vm.profile=false', + '--enable-asserts', + ]); + }); }