Skip to content

Commit

Permalink
[flutter_tools] Remove --no-sound-null-safety #4 (flutter#124039)
Browse files Browse the repository at this point in the history 
Re-land of flutter#123297 without any of
the commits at the end attempting to fix customer-testing.

Fixes flutter#118810
  • Loading branch information
@christopherfujino
christopherfujino authored Apr 4, 2023
1 parent 34d2c8d commit b2c65ac
Show file tree
Hide file tree
Showing 9 changed files with 104 additions and 187 deletions.
51 changes: 16 additions & 35 deletions dev/bots/test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,9 +573,7 @@ Future<void> _runExampleProjectBuildTests(Directory exampleDirectory, [File? mai
// Only verify caching with flutter gallery.
final bool verifyCaching = exampleDirectory.path.contains('flutter_gallery');
final String examplePath = path.relative(exampleDirectory.path, from: Directory.current.path);
final bool hasNullSafety = File(path.join(examplePath, 'null_safety')).existsSync();
final List<String> additionalArgs = <String>[
if (hasNullSafety) '--no-sound-null-safety',
if (mainFile != null) path.relative(mainFile.path, from: exampleDirectory.absolute.path),
];
if (Directory(path.join(examplePath, 'android')).existsSync()) {
Expand Down Expand Up @@ -771,16 +769,14 @@ Future<void> _runAddToAppLifeCycleTests() async {
}

Future<void> _runFrameworkTests() async {
final List<String> soundNullSafetyOptions = <String>['--null-assertions', '--sound-null-safety'];
final List<String> mixedModeNullSafetyOptions = <String>['--null-assertions', '--no-sound-null-safety'];
final List<String> trackWidgetCreationAlternatives = <String>['--track-widget-creation', '--no-track-widget-creation'];

Future<void> runWidgets() async {
printProgress('${green}Running packages/flutter tests $reset for ${cyan}test/widgets/$reset');
for (final String trackWidgetCreationOption in trackWidgetCreationAlternatives) {
await _runFlutterTest(
path.join(flutterRoot, 'packages', 'flutter'),
options: <String>[trackWidgetCreationOption, ...soundNullSafetyOptions],
options: <String>[trackWidgetCreationOption],
tests: <String>[ path.join('test', 'widgets') + path.separator ],
);
}
Expand All @@ -795,13 +791,13 @@ Future<void> _runFrameworkTests() async {
// Run release mode tests (see packages/flutter/test_release/README.md)
await _runFlutterTest(
path.join(flutterRoot, 'packages', 'flutter'),
options: <String>['--dart-define=dart.vm.product=true', ...soundNullSafetyOptions],
options: <String>['--dart-define=dart.vm.product=true'],
tests: <String>['test_release${path.separator}'],
);
// Run profile mode tests (see packages/flutter/test_profile/README.md)
await _runFlutterTest(
path.join(flutterRoot, 'packages', 'flutter'),
options: <String>['--dart-define=dart.vm.product=false', '--dart-define=dart.vm.profile=true', ...soundNullSafetyOptions],
options: <String>['--dart-define=dart.vm.product=false', '--dart-define=dart.vm.profile=true'],
tests: <String>['test_profile${path.separator}'],
);
}
Expand All @@ -817,7 +813,7 @@ Future<void> _runFrameworkTests() async {
for (final String trackWidgetCreationOption in trackWidgetCreationAlternatives) {
await _runFlutterTest(
path.join(flutterRoot, 'packages', 'flutter'),
options: <String>[trackWidgetCreationOption, ...soundNullSafetyOptions],
options: <String>[trackWidgetCreationOption],
tests: tests,
);
}
Expand All @@ -837,9 +833,9 @@ Future<void> _runFrameworkTests() async {
workingDirectory: path.join(flutterRoot, 'examples', 'api'),
);
}
await _runFlutterTest(path.join(flutterRoot, 'examples', 'api'), options: soundNullSafetyOptions);
await _runFlutterTest(path.join(flutterRoot, 'examples', 'hello_world'), options: soundNullSafetyOptions);
await _runFlutterTest(path.join(flutterRoot, 'examples', 'layers'), options: soundNullSafetyOptions);
await _runFlutterTest(path.join(flutterRoot, 'examples', 'api'));
await _runFlutterTest(path.join(flutterRoot, 'examples', 'hello_world'));
await _runFlutterTest(path.join(flutterRoot, 'examples', 'layers'));
}

Future<void> runTracingTests() async {
Expand Down Expand Up @@ -945,7 +941,6 @@ Future<void> _runFrameworkTests() async {

Future<void> runPrivateTests() async {
final List<String> args = <String>[
'--sound-null-safety',
'run',
'bin/test_private.dart',
];
Expand Down Expand Up @@ -989,17 +984,17 @@ Future<void> _runFrameworkTests() async {
await _runFlutterTest(path.join(flutterRoot, 'dev', 'tools', 'gen_defaults'));
await _runFlutterTest(path.join(flutterRoot, 'dev', 'tools', 'gen_keycodes'));
await _runFlutterTest(path.join(flutterRoot, 'dev', 'benchmarks', 'test_apps', 'stocks'));
await _runFlutterTest(path.join(flutterRoot, 'packages', 'flutter_driver'), tests: <String>[path.join('test', 'src', 'real_tests')], options: soundNullSafetyOptions);
await _runFlutterTest(path.join(flutterRoot, 'packages', 'flutter_driver'), tests: <String>[path.join('test', 'src', 'real_tests')]);
await _runFlutterTest(path.join(flutterRoot, 'packages', 'integration_test'), options: <String>[
'--enable-vmservice',
// Web-specific tests depend on Chromium, so they run as part of the web_long_running_tests shard.
'--exclude-tags=web',
]);
await _runFlutterTest(path.join(flutterRoot, 'packages', 'flutter_goldens'), options: soundNullSafetyOptions);
await _runFlutterTest(path.join(flutterRoot, 'packages', 'flutter_localizations'), options: soundNullSafetyOptions);
await _runFlutterTest(path.join(flutterRoot, 'packages', 'flutter_test'), options: soundNullSafetyOptions);
await _runFlutterTest(path.join(flutterRoot, 'packages', 'fuchsia_remote_debug_protocol'), options: soundNullSafetyOptions);
await _runFlutterTest(path.join(flutterRoot, 'dev', 'integration_tests', 'non_nullable'), options: mixedModeNullSafetyOptions);
await _runFlutterTest(path.join(flutterRoot, 'packages', 'flutter_goldens'));
await _runFlutterTest(path.join(flutterRoot, 'packages', 'flutter_localizations'));
await _runFlutterTest(path.join(flutterRoot, 'packages', 'flutter_test'));
await _runFlutterTest(path.join(flutterRoot, 'packages', 'fuchsia_remote_debug_protocol'));
await _runFlutterTest(path.join(flutterRoot, 'dev', 'integration_tests', 'non_nullable'));
const String httpClientWarning =
'Warning: At least one test in this suite creates an HttpClient. When\n'
'running a test suite that uses TestWidgetsFlutterBinding, all HTTP\n'
Expand Down Expand Up @@ -1234,7 +1229,7 @@ Future<void> _runWebLongRunningTests() async {
'--dart-define=TEST_FLUTTER_ENGINE_VERSION=$engineVersion',
]),
() => _runWebDebugTest('test/test.dart'),
() => _runWebDebugTest('lib/null_safe_main.dart', enableNullSafety: true),
() => _runWebDebugTest('lib/null_safe_main.dart'),
() => _runWebDebugTest('lib/web_define_loading.dart',
additionalArguments: <String>[
'--dart-define=test.valueA=Example,A',
Expand All @@ -1247,12 +1242,8 @@ Future<void> _runWebLongRunningTests() async {
'--dart-define=test.valueB=Value',
]
),
() => _runWebDebugTest('lib/sound_mode.dart', additionalArguments: <String>[
'--sound-null-safety',
]),
() => _runWebReleaseTest('lib/sound_mode.dart', additionalArguments: <String>[
'--sound-null-safety',
]),
() => _runWebDebugTest('lib/sound_mode.dart'),
() => _runWebReleaseTest('lib/sound_mode.dart'),
() => _runFlutterWebTest(
'html',
path.join(flutterRoot, 'packages', 'integration_test'),
Expand Down Expand Up @@ -1311,7 +1302,6 @@ Future<void> _runFlutterDriverWebTest({
if (driver != null) '--driver=$driver',
'--target=$target',
'--browser-name=chrome',
'--no-sound-null-safety',
'-d',
'web-server',
'--$buildMode',
Expand Down Expand Up @@ -1353,7 +1343,6 @@ Future<void> _runWebTreeshakeTest() async {
'build',
'web',
'--target=$target',
'--no-sound-null-safety',
'--profile',
],
workingDirectory: testAppDirectory,
Expand Down Expand Up @@ -1579,7 +1568,6 @@ Future<void> _runGalleryE2eWebTest(String buildMode, { bool canvasKit = false })
'--driver=test_driver/transitions_perf_e2e_test.dart',
'--target=test_driver/transitions_perf_e2e.dart',
'--browser-name=chrome',
'--no-sound-null-safety',
'-d',
'web-server',
'--$buildMode',
Expand Down Expand Up @@ -1686,7 +1674,6 @@ Future<void> _runWebReleaseTest(String target, {
///
/// Instead, we use `flutter run --debug` and sniff out the standard output.
Future<void> _runWebDebugTest(String target, {
bool enableNullSafety = false,
List<String> additionalArguments = const<String>[],
}) async {
final String testAppDirectory = path.join(flutterRoot, 'dev', 'integration_tests', 'web');
Expand All @@ -1700,11 +1687,6 @@ Future<void> _runWebDebugTest(String target, {
<String>[
'run',
'--debug',
if (enableNullSafety)
...<String>[
'--no-sound-null-safety',
'--null-assertions',
],
'-d',
'chrome',
'--web-run-headless',
Expand Down Expand Up @@ -1747,7 +1729,6 @@ Future<void> _runFlutterWebTest(String webRenderer, String workingDirectory, Lis
'--platform=chrome',
'--web-renderer=$webRenderer',
'--dart-define=DART_HHH_BOT=$_runningInDartHHHBot',
'--sound-null-safety',
...flutterTestArgs,
...tests,
],
Expand Down
7 changes: 2 additions & 5 deletions packages/flutter/test/foundation/isolates_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ Future<int> test5CallCompute(int value) {
return compute(test5, value);
}

Future<void> expectFileSuccessfullyCompletes(String filename,
[bool unsound = false]) async {
Future<void> expectFileSuccessfullyCompletes(String filename) async {
// Run a Dart script that calls compute().
// The Dart process will terminate only if the script exits cleanly with
// all isolate ports closed.
Expand All @@ -93,12 +92,10 @@ Future<void> expectFileSuccessfullyCompletes(String filename,
final String packageRoot = fs.path.dirname(fs.path.fromUri(platform.script));
final String scriptPath =
fs.path.join(packageRoot, 'test', 'foundation', filename);
final String nullSafetyArg =
unsound ? '--no-sound-null-safety' : '--sound-null-safety';

// Enable asserts to also catch potentially invalid assertions.
final ProcessResult result = await Process.run(
dartPath, <String>[nullSafetyArg, 'run', '--enable-asserts', scriptPath]);
dartPath, <String>['run', '--enable-asserts', scriptPath]);
expect(result.exitCode, 0);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test_private/bin/test_private.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ class TestCase {
for (final File test in tests) {
final String testPath = path.join(path.dirname(test.path), 'lib', path.basenameWithoutExtension(test.path));
final ProcessRunnerResult result = await runner.runProcess(
<String>[flutter, 'test', '--enable-experiment=non-nullable', '--no-sound-null-safety', '--null-assertions', testPath],
<String>[flutter, 'test', testPath],
failOk: true,
);
if (result.exitCode != 0) {
Expand Down
5 changes: 5 additions & 0 deletions packages/flutter_tools/lib/src/globals.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import 'pre_run_validator.dart';
import 'project.dart';
import 'reporting/crash_reporting.dart';
import 'reporting/reporting.dart';
import 'runner/flutter_command.dart';
import 'runner/local_engine.dart';
import 'version.dart';

Expand Down Expand Up @@ -285,3 +286,7 @@ const String kDefaultFrameworkChannel = 'master';

// Used to build RegExp instances which can detect the VM service message.
final RegExp kVMServiceMessageRegExp = RegExp(r'The Dart VM service is listening on ((http|//)[a-zA-Z0-9:/=_\-\.\[\]]+)');

// The official tool no longer allows non-null safe builds. This can be
// overridden in other clients.
NonNullSafeBuilds get nonNullSafeBuilds => context.get<NonNullSafeBuilds>() ?? NonNullSafeBuilds.notAllowed;
71 changes: 30 additions & 41 deletions packages/flutter_tools/lib/src/runner/flutter_command.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import '../bundle.dart' as bundle;
import '../cache.dart';
import '../convert.dart';
import '../dart/generate_synthetic_packages.dart';
import '../dart/language_version.dart';
import '../dart/package_map.dart';
import '../dart/pub.dart';
import '../device.dart';
Expand Down Expand Up @@ -822,20 +821,13 @@ abstract class FlutterCommand extends Command<void> {

void addNullSafetyModeOptions({ required bool hide }) {
argParser.addFlag(FlutterOptions.kNullSafety,
help:
'Whether to override the inferred null safety mode. This allows null-safe '
'libraries to depend on un-migrated (non-null safe) libraries. By default, '
'Flutter mobile & desktop applications will attempt to run at the null safety '
'level of their entrypoint library (usually lib/main.dart). Flutter web '
'applications will default to sound null-safety, unless specifically configured.',
help: 'This flag is deprecated as only null-safe code is supported.',
defaultsTo: true,
hide: hide,
hide: true,
);
argParser.addFlag(FlutterOptions.kNullAssertions,
help:
'Perform additional null assertions on the boundaries of migrated and '
'un-migrated code. This setting is not currently supported on desktop '
'devices.'
help: 'This flag is deprecated as only null-safe code is supported.',
hide: true,
);
}

Expand Down Expand Up @@ -1148,39 +1140,17 @@ abstract class FlutterCommand extends Command<void> {

NullSafetyMode nullSafetyMode = NullSafetyMode.sound;
if (argParser.options.containsKey(FlutterOptions.kNullSafety)) {
// Explicitly check for `true` and `false` so that `null` results in not
// passing a flag. Examine the entrypoint file to determine if it
// is opted in or out.
final bool wasNullSafetyFlagParsed = argResults?.wasParsed(FlutterOptions.kNullSafety) ?? false;
if (!wasNullSafetyFlagParsed && (argParser.options.containsKey('target') || forcedTargetFile != null)) {
final File entrypointFile = forcedTargetFile ?? globals.fs.file(targetFile);
final LanguageVersion languageVersion = determineLanguageVersion(
entrypointFile,
packageConfig.packageOf(entrypointFile.absolute.uri),
Cache.flutterRoot!,
);
// Extra frontend options are only provided if explicitly
// requested.
if ((languageVersion.major > nullSafeVersion.major) ||
(languageVersion.major == nullSafeVersion.major && languageVersion.minor >= nullSafeVersion.minor)) {
// Extra frontend options are only provided if explicitly
// requested.
if (wasNullSafetyFlagParsed) {
if (boolArg(FlutterOptions.kNullSafety)) {
nullSafetyMode = NullSafetyMode.sound;
extraFrontEndOptions.add('--sound-null-safety');
} else {
throwToolExit(
'This application does not support sound null-safety (its language version is $languageVersion).\n'
'To build this application, you must provide the CLI flag --no-sound-null-safety. Dart 3 will only '
'support sound null safety, see https://dart.dev/null-safety.',
);
nullSafetyMode = NullSafetyMode.unsound;
extraFrontEndOptions.add('--no-sound-null-safety');
}
} else if (!wasNullSafetyFlagParsed) {
// This mode is only used for commands which do not build a single target like
// 'flutter test'.
nullSafetyMode = NullSafetyMode.autodetect;
} else if (boolArg(FlutterOptions.kNullSafety)) {
nullSafetyMode = NullSafetyMode.sound;
extraFrontEndOptions.add('--sound-null-safety');
} else {
nullSafetyMode = NullSafetyMode.unsound;
extraFrontEndOptions.add('--no-sound-null-safety');
}
}

Expand Down Expand Up @@ -1487,6 +1457,16 @@ abstract class FlutterCommand extends Command<void> {
/// rather than calling [runCommand] directly.
@mustCallSuper
Future<FlutterCommandResult> verifyThenRunCommand(String? commandPath) async {
if (argParser.options.containsKey(FlutterOptions.kNullSafety) &&
argResults![FlutterOptions.kNullSafety] == false &&
globals.nonNullSafeBuilds == NonNullSafeBuilds.notAllowed) {
throwToolExit('''
Could not find an option named "no-${FlutterOptions.kNullSafety}".
Run 'flutter -h' (or 'flutter <command> -h') for available flutter commands and options.
''');
}

globals.preRunValidator.validate();

if (refreshWirelessDevices) {
Expand Down Expand Up @@ -1754,3 +1734,12 @@ DevelopmentArtifact? artifactFromTargetPlatform(TargetPlatform targetPlatform) {

/// Returns true if s is either null, empty or is solely made of whitespace characters (as defined by String.trim).
bool _isBlank(String s) => s.trim().isEmpty;

/// Whether the tool should allow non-null safe builds.
///
/// The Dart SDK no longer supports non-null safe builds, so this value in the
/// tool's context should always be [NonNullSafeBuilds.notAllowed].
enum NonNullSafeBuilds {
allowed,
notAllowed,
}
46 changes: 46 additions & 0 deletions packages/flutter_tools/test/commands.shard/hermetic/run_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,52 @@ void main() {
Logger: () => BufferLogger.test(),
});

testUsingContext('does not support --no-sound-null-safety by default', () async {
fileSystem.file('lib/main.dart').createSync(recursive: true);
fileSystem.file('pubspec.yaml').createSync();
fileSystem.file('.packages').createSync();

final TestRunCommandThatOnlyValidates command = TestRunCommandThatOnlyValidates();
await expectLater(
() => createTestCommandRunner(command).run(<String>[
'run',
'--use-application-binary=app/bar/faz',
'--no-sound-null-safety',
]),
throwsA(isException.having(
(Exception exception) => exception.toString(),
'toString',
contains('Could not find an option named "no-sound-null-safety"'),
)),
);
}, overrides: <Type, Generator>{
FileSystem: () => fileSystem,
ProcessManager: () => FakeProcessManager.any(),
Logger: () => BufferLogger.test(),
});

testUsingContext('supports --no-sound-null-safety with an overridden NonNullSafeBuilds', () async {
fileSystem.file('lib/main.dart').createSync(recursive: true);
fileSystem.file('pubspec.yaml').createSync();
fileSystem.file('.packages').createSync();

final FakeDevice device = FakeDevice(isLocalEmulator: true, platformType: PlatformType.android);

testDeviceManager.devices = <Device>[device];
final TestRunCommandThatOnlyValidates command = TestRunCommandThatOnlyValidates();
await createTestCommandRunner(command).run(const <String>[
'run',
'--use-application-binary=app/bar/faz',
'--no-sound-null-safety',
]);
}, overrides: <Type, Generator>{
DeviceManager: () => testDeviceManager,
FileSystem: () => fileSystem,
Logger: () => BufferLogger.test(),
NonNullSafeBuilds: () => NonNullSafeBuilds.allowed,
ProcessManager: () => FakeProcessManager.any(),
});

testUsingContext('does not support "--use-application-binary" and "--fast-start"', () async {
fileSystem.file('lib/main.dart').createSync(recursive: true);
fileSystem.file('pubspec.yaml').createSync();
Expand Down
5 changes: 4 additions & 1 deletion packages/flutter_tools/test/general.shard/args_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,10 @@ void verifyOptions(String? command, Iterable<Option> options) {
expect(option.name, matches(_allowedArgumentNamePattern), reason: '$_header$target--${option.name}" is not a valid name for a command line argument. (Is it all lowercase? Does it use hyphens rather than underscores?)');
}
expect(option.name, isNot(matches(_bannedArgumentNamePattern)), reason: '$_header$target--${option.name}" is not a valid name for a command line argument. (We use "--foo-url", not "--foo-uri", for example.)');
expect(option.hide, isFalse, reason: '${_header}Help for $target--${option.name}" is always hidden. $_needHelp');
// The flag --sound-null-safety is deprecated
if (option.name != FlutterOptions.kNullSafety && option.name != FlutterOptions.kNullAssertions) {
expect(option.hide, isFalse, reason: '${_header}Help for $target--${option.name}" is always hidden. $_needHelp');
}
expect(option.help, isNotNull, reason: '${_header}Help for $target--${option.name}" has null help. $_needHelp');
expect(option.help, isNotEmpty, reason: '${_header}Help for $target--${option.name}" has empty help. $_needHelp');
expect(option.help, isNot(matches(_bannedLeadingPatterns)), reason: '${_header}A line in the help for $target--${option.name}" starts with a lowercase letter. For stylistic consistency, all help messages must start with a capital letter.');
Expand Down
Loading

0 comments on commit b2c65ac

Please sign in to comment.