[flutter_tools] only try to take a screenshot from flutter drive if the...

[flutter_tools] only try to take a screenshot from flutter drive if the --screenshot flag is passed (#127150)

Fixes https://github.com/flutter/flutter/issues/123621

packages/flutter_tools/lib/src/commands/drive.dart

@@ -315,8 +315,10 @@ class DriveCommand extends RunCommandBase {
         profileMemory: stringArg('profile-memory'),
       );
 
-      // If the test is sent a signal or times out, take a screenshot
-      _registerScreenshotCallbacks(device);
+      if (screenshot != null) {
+        // If the test is sent a signal or times out, take a screenshot
+        _registerScreenshotCallbacks(device, _fileSystem.directory(screenshot));
+      }
 
       final int testResult = await testResultFuture;
 
@@ -327,7 +329,7 @@ class DriveCommand extends RunCommandBase {
 
       if (testResult != 0 && screenshot != null) {
         // Take a screenshot while the app is still running.
-        await _takeScreenshot(device);
+        await _takeScreenshot(device, _fileSystem.directory(screenshot));
         screenshotTaken = true;
       }
 
@@ -346,7 +348,7 @@ class DriveCommand extends RunCommandBase {
       // On exceptions, including ToolExit, take a screenshot on the device
       // unless a screenshot was already taken on test failure.
       if (!screenshotTaken && screenshot != null) {
-        await _takeScreenshot(device);
+        await _takeScreenshot(device, _fileSystem.directory(screenshot));
       }
       rethrow;
     }
@@ -369,7 +371,7 @@ class DriveCommand extends RunCommandBase {
     return timeoutSeconds;
   }
 
-  void _registerScreenshotCallbacks(Device device) {
+  void _registerScreenshotCallbacks(Device device, Directory screenshotDir) {
     _logger.printTrace('Registering signal handlers...');
     final Map<ProcessSignal, Object> tokens = <ProcessSignal, Object>{};
     for (final ProcessSignal signal in signalsToHandle) {
@@ -378,7 +380,7 @@ class DriveCommand extends RunCommandBase {
         (ProcessSignal signal) {
           _unregisterScreenshotCallbacks();
           _logger.printError('Caught $signal');
-          return _takeScreenshot(device);
+          return _takeScreenshot(device, screenshotDir);
         },
       );
     }
@@ -390,7 +392,7 @@ class DriveCommand extends RunCommandBase {
         Duration(seconds: timeoutSeconds),
         () {
           _unregisterScreenshotCallbacks();
-          _takeScreenshot(device);
+          _takeScreenshot(device, screenshotDir);
           throwToolExit('Timed out after $timeoutSeconds seconds');
         }
       );
@@ -450,13 +452,12 @@ class DriveCommand extends RunCommandBase {
     return '${pathWithNoExtension}_test${_fileSystem.path.extension(appFile)}';
   }
 
-  Future<void> _takeScreenshot(Device device) async {
+  Future<void> _takeScreenshot(Device device, Directory outputDirectory) async {
     if (!device.supportsScreenshot) {
       return;
     }
     try {
-      final Directory outputDirectory = _fileSystem.directory(screenshot)
-        ..createSync(recursive: true);
+      outputDirectory.createSync(recursive: true);
       final File outputFile = _fsUtils.getUniqueFile(
         outputDirectory,
         'drive',

packages/flutter_tools/test/commands.shard/hermetic/drive_test.dart

@@ -35,7 +35,7 @@ void main() {
   late BufferLogger logger;
   late Platform platform;
   late FakeDeviceManager fakeDeviceManager;
-  late Signals signals;
+  late FakeSignals signals;
 
   setUp(() {
     fileSystem = MemoryFileSystem.test();
@@ -90,6 +90,43 @@ void main() {
     DeviceManager: () => fakeDeviceManager,
   });
 
+  testUsingContext('does not register screenshot signal handler if --screenshot not provided', () async {
+    final DriveCommand command = DriveCommand(
+      fileSystem: fileSystem,
+      logger: logger,
+      platform: platform,
+      signals: signals,
+      flutterDriverFactory: FailingFakeFlutterDriverFactory(),
+    );
+    fileSystem.file('lib/main.dart').createSync(recursive: true);
+    fileSystem.file('test_driver/main_test.dart').createSync(recursive: true);
+    fileSystem.file('pubspec.yaml').createSync();
+    fileSystem.directory('drive_screenshots').createSync();
+
+    final Device screenshotDevice = ScreenshotDevice();
+    fakeDeviceManager.attachedDevices = <Device>[screenshotDevice];
+
+    await expectLater(() => createTestCommandRunner(command).run(
+      <String>[
+        'drive',
+        '--no-pub',
+        '-d',
+        screenshotDevice.id,
+        '--use-existing-app',
+        'http://localhost:8181',
+        '--keep-app-running',
+      ]),
+      throwsToolExit(),
+    );
+    expect(logger.statusText, isNot(contains('Screenshot written to ')));
+    expect(signals.addedHandlers, isEmpty);
+  }, overrides: <Type, Generator>{
+    FileSystem: () => fileSystem,
+    ProcessManager: () => FakeProcessManager.any(),
+    Pub: () => FakePub(),
+    DeviceManager: () => fakeDeviceManager,
+  });
+
   testUsingContext('takes screenshot and rethrows on drive exception', () async {
     final DriveCommand command = DriveCommand(
       fileSystem: fileSystem,
@@ -672,3 +709,16 @@ class FakeIosDevice extends Fake implements IOSDevice {
   @override
   Future<TargetPlatform> get targetPlatform async => TargetPlatform.ios;
 }
+
+class FakeSignals extends Fake implements Signals {
+  List<SignalHandler> addedHandlers = <SignalHandler>[];
+
+  @override
+  Object addHandler(ProcessSignal signal, SignalHandler handler) {
+    addedHandlers.add(handler);
+    return const Object();
+  }
+
+  @override
+  Future<bool> removeHandler(ProcessSignal signal, Object token) async => true;
+}

packages/flutter_tools/test/src/context.dart

@@ -393,7 +393,7 @@ class LocalFileSystemBlockingSetCurrentDirectory extends LocalFileSystem {
 class FakeSignals implements Signals {
   @override
   Object addHandler(ProcessSignal signal, SignalHandler handler) {
-    return Object();
+    return const Object();
   }
 
   @override