flutter/packages/flutter_tools/test/general.shard/commands/build_test.dart
Christopher Fujino a6118612ac
Fix flutter attach local engine (#131825)
Fixes: https://github.com/flutter/flutter/issues/124970
Part of https://github.com/flutter/flutter/issues/47161

Before this change, there were two places we overrode the `Artifacts` in a Zone:

1. if/when we parse local-engine CLI options: 1cf3907407/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart (L281)
2. an additional override for fuchsia platform dill (no longer used, deleted in this PR): 1cf3907407/packages/flutter_tools/lib/src/commands/attach.dart (L274)

Note 1 above creates a new instance of `Artifacts.getLocalEngine()`. In this flow, there exist two instances of `Artifacts`:

1. The default fallback instance of `CachedArtifacts` (which gets all artifacts from flutter/bin/cache), instantiated in context_runner.dart: 1cf3907407/packages/flutter_tools/lib/src/context_runner.dart (L137)
2. An instance of `CachedLocalEngineArtifacts` created in the command runner once the CLI options have been parsed: 1cf3907407/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart (L281)

The regression happened when we direct injected the Artifacts 1 from above BEFORE we parsed the local-engine flag, and then used this in the second zone override, and then when creating the `FlutterDevice` there are multiple calls to `globals.artifacts` returned it when it should have returned Artifacts 2: 1cf3907407/packages/flutter_tools/lib/src/resident_runner.dart (L80)

Device.artifactOverrides was originally introduced in https://github.com/flutter/flutter/pull/32071, but is no longer used, so I deleted it.

I also removed direct injection of `Artifacts` to the attach sub-command, because that class now no longer references artifacts.

I believe the ideal true fix for this would be to:

1. Migrate all leaf calls to `globals.artifacts` to use direct injection (in this case, the offending invocations were in [`FlutterDevice.create()`](1cf3907407/packages/flutter_tools/lib/src/resident_runner.dart (L80-L218)), but I'm not sure that something else would not have broken later)
2. Ensure we are always direct injecting the desired instance of `Artifacts`--that is, if the user desires local engine artifacts, that we are passing an instance of `CachedLocalEngineArtifacts`.
  a. Alternatively, and probably simpler, teach `CachedArtifacts` to know about the local engine. This would mean parsing the global CLI options BEFORE we ever construct any instance of `Artifacts`.
  
As an overall recommendation for implementing https://github.com/flutter/flutter/issues/47161, in the overall tree of tool function calls, we should probably migrate the leaves first (that is, migrate the sub-commands last). We should also audit and reconsider any usage of `runZoned()` or `context.run()` for the purpose overriding zoneValues.
2023-08-10 17:51:05 +00:00

145 lines
5.0 KiB
Dart

// 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:args/args.dart';
import 'package:args/command_runner.dart';
import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/base/signals.dart';
import 'package:flutter_tools/src/base/terminal.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/build_system/build_system.dart';
import 'package:flutter_tools/src/commands/attach.dart';
import 'package:flutter_tools/src/commands/build.dart';
import 'package:flutter_tools/src/commands/build_aar.dart';
import 'package:flutter_tools/src/commands/build_apk.dart';
import 'package:flutter_tools/src/commands/build_appbundle.dart';
import 'package:flutter_tools/src/commands/build_ios.dart';
import 'package:flutter_tools/src/commands/build_ios_framework.dart';
import 'package:flutter_tools/src/commands/build_linux.dart';
import 'package:flutter_tools/src/commands/build_macos.dart';
import 'package:flutter_tools/src/commands/build_web.dart';
import 'package:flutter_tools/src/commands/build_windows.dart';
import 'package:flutter_tools/src/runner/flutter_command.dart';
import 'package:test/fake.dart';
import '../../src/common.dart';
import '../../src/context.dart';
import '../../src/fakes.dart';
import '../../src/test_build_system.dart';
class FakeTerminal extends Fake implements AnsiTerminal {
FakeTerminal({this.stdinHasTerminal = true});
@override
final bool stdinHasTerminal;
}
class FakeProcessInfo extends Fake implements ProcessInfo {
@override
int maxRss = 0;
}
void main() {
testUsingContext('All build commands support null safety options', () {
final FileSystem fileSystem = MemoryFileSystem.test();
final Platform platform = FakePlatform();
final BufferLogger logger = BufferLogger.test();
final List<FlutterCommand> commands = <FlutterCommand>[
BuildWindowsCommand(logger: BufferLogger.test()),
BuildLinuxCommand(logger: BufferLogger.test(), operatingSystemUtils: FakeOperatingSystemUtils()),
BuildMacosCommand(logger: BufferLogger.test(), verboseHelp: false),
BuildWebCommand(fileSystem: fileSystem, logger: BufferLogger.test(), verboseHelp: false),
BuildApkCommand(logger: BufferLogger.test()),
BuildIOSCommand(logger: BufferLogger.test(), verboseHelp: false),
BuildIOSArchiveCommand(logger: BufferLogger.test(), verboseHelp: false),
BuildAppBundleCommand(logger: BufferLogger.test()),
BuildAarCommand(
logger: BufferLogger.test(),
androidSdk: FakeAndroidSdk(),
fileSystem: fileSystem,
verboseHelp: false,
),
BuildIOSFrameworkCommand(
logger: BufferLogger.test(),
verboseHelp: false,
buildSystem: FlutterBuildSystem(
fileSystem: fileSystem,
platform: platform,
logger: logger,
),
),
AttachCommand(
stdio: FakeStdio(),
logger: logger,
terminal: FakeTerminal(),
signals: Signals.test(),
platform: platform,
processInfo: FakeProcessInfo(),
fileSystem: MemoryFileSystem.test(),
),
];
for (final FlutterCommand command in commands) {
final ArgResults results = command.argParser.parse(<String>[
'--sound-null-safety',
'--enable-experiment=non-nullable',
]);
expect(results.wasParsed('sound-null-safety'), true);
expect(results.wasParsed('enable-experiment'), true);
}
});
testUsingContext('BuildSubCommand displays current null safety mode',
() async {
const BuildInfo unsound = BuildInfo(
BuildMode.debug,
'',
nullSafetyMode: NullSafetyMode.unsound,
treeShakeIcons: false,
);
final BufferLogger logger = BufferLogger.test();
FakeBuildSubCommand(logger).test(unsound);
expect(logger.statusText,
contains('Building without sound null safety ⚠️'));
});
testUsingContext('Include only supported sub commands', () {
final BuildCommand command = BuildCommand(
androidSdk: FakeAndroidSdk(),
buildSystem: TestBuildSystem.all(BuildResult(success: true)),
fileSystem: MemoryFileSystem.test(),
logger: BufferLogger.test(),
osUtils: FakeOperatingSystemUtils(),
);
for (final Command<void> x in command.subcommands.values) {
expect((x as BuildSubCommand).supported, isTrue);
}
});
}
class FakeBuildSubCommand extends BuildSubCommand {
FakeBuildSubCommand(Logger logger) : super(logger: logger, verboseHelp: false);
@override
String get description => throw UnimplementedError();
@override
String get name => throw UnimplementedError();
void test(BuildInfo buildInfo) {
displayNullSafetyMode(buildInfo);
}
@override
Future<FlutterCommandResult> runCommand() {
throw UnimplementedError();
}
}