Fix local testing, gradle XML errors, and enable on CI. (flutter#152383)

TIL you cannot have XML comments before the initial `<?xml` declaration.

Follow-up to flutter#152326.

Author: matanlurey

.ci.yaml

@@ -1297,11 +1297,15 @@ targets:
   - name: Linux_android_emu flutter_driver_android_test
     recipe: flutter/flutter_drone
     timeout: 60
-    bringup: true
+    bringup: false
     properties:
       shard: flutter_driver_android
       tags: >
          ["framework", "hostonly", "shard", "linux"]
+      dependencies: >-
+        [
+          {"dependency": "goldctl", "version": "git_revision:720a542f6fe4f92922c3b8f0fdcc4d2ac6bb83cd"}
+        ]
 
   - name: Linux realm_checker
     recipe: flutter/flutter_drone

dev/bots/suite_runners/run_flutter_driver_android_tests.dart

@@ -2,18 +2,50 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'dart:convert';
+import 'dart:io' as io;
+
 import 'package:path/path.dart' as path;
 import '../run_command.dart';
 import '../utils.dart';
 
+/// To run this test locally:
+///
+/// 1. Connect an Android device or emulator.
+/// 2. Run the following command from the root of the Flutter repository:
+///
+/// ```sh
+/// SHARD=flutter_driver_android bin/cache/dart-sdk/bin/dart dev/bots/test.dart
+/// ```
+///
+/// For debugging, it is recommended to instead just run and launch these tests
+/// individually _in_ the `dev/integration_tests/android_driver_test` directory.
 Future<void> runFlutterDriverAndroidTests() async {
   print('Running Flutter Driver Android tests...');
 
+  // Print out the results of `adb devices`, for uh, science:
+  print('Listing devices...');
+  final io.ProcessResult devices = await _adb(
+    <String>[
+      'devices',
+    ],
+  );
+  print(devices.stdout);
+  print(devices.stderr);
+
+  // We need to configure the emulator to disable confirmations before the
+  // application starts. Some of these configuration options won't work once
+  // the application is running.
+  print('Configuring device...');
+  await _configureForScreenshotTesting();
+
   // TODO(matanlurey): Should we be using another instrumentation method?
   await runCommand(
     'flutter',
     <String>[
       'drive',
+      '--test-arguments=test',
+      '--test-arguments=--reporter=expanded',
     ],
     workingDirectory: path.join(
       'dev',
@@ -22,3 +54,60 @@ Future<void> runFlutterDriverAndroidTests() async {
     ),
   );
 }
+
+// TODO(matanlurey): Move this code into flutter_driver instead of here.
+Future<void> _configureForScreenshotTesting() async {
+  // Disable confirmation for immersive mode.
+  final io.ProcessResult immersive = await _adb(
+    <String>[
+      'shell',
+      'settings',
+      'put',
+      'secure',
+      'immersive_mode_confirmations',
+      'confirmed',
+    ],
+  );
+
+  if (immersive.exitCode != 0) {
+    throw StateError('Failed to configure device: ${immersive.stderr}');
+  }
+
+  const Map<String, String> settings = <String, String>{
+    'show_surface_updates': '1',
+    'transition_animation_scale': '0',
+    'window_animation_scale': '0',
+    'animator_duration_scale': '0',
+  };
+
+  for (final MapEntry<String, String> entry in settings.entries) {
+    final io.ProcessResult result = await _adb(
+      <String>[
+        'shell',
+        'settings',
+        'put',
+        'global',
+        entry.key,
+        entry.value,
+      ],
+    );
+
+    if (result.exitCode != 0) {
+      throw StateError('Failed to configure device: ${result.stderr}');
+    }
+  }
+}
+
+Future<io.ProcessResult> _adb(
+  List<String> args, {
+  Encoding? stdoutEncoding = io.systemEncoding,
+}) {
+  // TODO(matanlurey): Ideally we should specify the device target here.
+  return io.Process.run(
+    'adb',
+    <String>[
+      ...args,
+    ],
+    stdoutEncoding: stdoutEncoding,
+  );
+}

dev/integration_tests/android_driver_test/android/app/src/main/res/drawable-v21/launch_background.xml

@@ -1,9 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
 <!-- 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. -->
 
-<?xml version="1.0" encoding="utf-8"?>
-<!-- Modify this file to customize your launch splash screen -->
 <layer-list xmlns:android="http://schemas.android.com/apk/res/android">
     <item android:drawable="?android:colorBackground" />
 

dev/integration_tests/android_driver_test/android/app/src/main/res/drawable/launch_background.xml

@@ -1,9 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
 <!-- 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. -->
 
-<?xml version="1.0" encoding="utf-8"?>
-<!-- Modify this file to customize your launch splash screen -->
 <layer-list xmlns:android="http://schemas.android.com/apk/res/android">
     <item android:drawable="@android:color/white" />
 

dev/integration_tests/android_driver_test/android/app/src/main/res/values-night/styles.xml

@@ -1,8 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
 <!-- 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. -->
 
-<?xml version="1.0" encoding="utf-8"?>
 <resources>
     <!-- Theme applied to the Android Window while the process is starting when the OS's Dark Mode setting is on -->
     <style name="LaunchTheme" parent="@android:style/Theme.Black.NoTitleBar">

dev/integration_tests/android_driver_test/android/app/src/main/res/values/styles.xml

@@ -1,8 +1,8 @@
+<?xml version="1.0" encoding="utf-8"?>
 <!-- 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. -->
 
-<?xml version="1.0" encoding="utf-8"?>
 <resources>
     <!-- Theme applied to the Android Window while the process is starting when the OS's Dark Mode setting is off -->
     <style name="LaunchTheme" parent="@android:style/Theme.Light.NoTitleBar">

dev/integration_tests/android_driver_test/test_driver/_flutter_goldens_fork.dart

@@ -19,6 +19,8 @@ import 'package:platform/platform.dart';
 import 'package:process/process.dart';
 
 const LocalFileSystem _localFs = LocalFileSystem();
+const String _kGoldctlKey = 'GOLDCTL';
+const String _kGoldctlPresubmitKey = 'GOLD_TRYJOB';
 
 // TODO(matanlurey): Refactor flutter_goldens to just re-use that code instead.
 Future<void> testExecutable(
@@ -31,19 +33,41 @@ Future<void> testExecutable(
     'where the "goldenFileComparator" has not yet been set. This is to ensure '
     'that the correct comparator is used for the current test environment.',
   );
-  final io.Directory tmpDir = io.Directory.systemTemp.createTempSync('android_driver_test');
+  if (!io.Platform.environment.containsKey(_kGoldctlKey)) {
+    io.stderr.writeln(
+      'Environment variable $_kGoldctlKey is not set. Assuming this is a local '
+      'test run and will not upload results to Skia Gold.',
+    );
+    return testMain();
+  }
+  final io.Directory tmpDir = io.Directory.systemTemp.createTempSync(
+    'android_driver_test',
+  );
+  final bool isPresubmit = io.Platform.environment.containsKey(
+    _kGoldctlPresubmitKey,
+  );
+  io.stderr.writeln(
+    '=== Using Skia Gold ===\n'
+    'Environment variable $_kGoldctlKey is set, using Skia Gold: \n'
+    '  - tmpDir:      ${tmpDir.path}\n'
+    '  - namePrefix:  $namePrefix\n'
+    '  - isPresubmit: $isPresubmit\n',
+  );
+  final SkiaGoldClient skiaGoldClient = SkiaGoldClient(
+    _localFs.directory(tmpDir.path),
+    fs: _localFs,
+    process: const LocalProcessManager(),
+    platform: const LocalPlatform(),
+    httpClient: io.HttpClient(),
+    log: io.stderr.writeln,
+  );
+  await skiaGoldClient.auth();
   goldenFileComparator = _GoldenFileComparator(
-    SkiaGoldClient(
-      _localFs.directory(tmpDir.path),
-      fs: _localFs,
-      process: const LocalProcessManager(),
-      platform: const LocalPlatform(),
-      httpClient: io.HttpClient(),
-      log: io.stderr.writeln,
-    ),
+    skiaGoldClient,
     namePrefix: namePrefix,
-    isPresubmit: false,
+    isPresubmit: isPresubmit,
   );
+  return testMain();
 }
 
 final class _GoldenFileComparator extends GoldenFileComparator {
@@ -68,22 +92,37 @@ final class _GoldenFileComparator extends GoldenFileComparator {
     }
 
     golden = _addPrefix(golden);
-    await update(golden, imageBytes);
-
-    final io.File goldenFile = _getGoldenFile(golden);
+    final io.File goldenFile = await update(golden, imageBytes);
     if (isPresubmit) {
-      await skiaClient.tryjobAdd(golden.path, _localFs.file(goldenFile.path));
+      final String? result = await skiaClient.tryjobAdd(
+        golden.path,
+        _localFs.file(goldenFile.path),
+      );
+      if (result != null) {
+        io.stderr.writeln(
+          'Skia Gold detected an error when comparing "$golden":\n\n$result',
+        );
+        io.stderr.writeln('Still succeeding, will be triaged in Flutter Gold');
+      } else {
+        io.stderr.writeln(
+          'Skia Gold comparison succeeded comparing "$golden".',
+        );
+      }
       return true;
     } else {
       return skiaClient.imgtestAdd(golden.path, _localFs.file(goldenFile.path));
     }
   }
 
   @override
-  Future<void> update(Uri golden, Uint8List imageBytes) async {
+  Future<io.File> update(Uri golden, Uint8List imageBytes) async {
+    io.stderr.writeln(
+      'Updating golden file: $golden (${imageBytes.length} bytes)...',
+    );
     final io.File goldenFile = _getGoldenFile(golden);
     await goldenFile.parent.create(recursive: true);
     await goldenFile.writeAsBytes(imageBytes, flush: true);
+    return goldenFile;
   }
 
   io.File _getGoldenFile(Uri uri) {

packages/flutter_driver/lib/src/native/android.dart

@@ -84,33 +84,6 @@ final class AndroidNativeDriver implements NativeDriver {
     await _tmpDir.delete(recursive: true);
   }
 
-  @override
-  Future<void> configureForScreenshotTesting() async {
-    const Map<String, String> settings = <String, String>{
-      'show_surface_updates': '1',
-      'transition_animation_scale': '0',
-      'window_animation_scale': '0',
-      'animator_duration_scale': '0',
-    };
-
-    for (final MapEntry<String, String> entry in settings.entries) {
-      final io.ProcessResult result = await _adb(
-        <String>[
-          'shell',
-          'settings',
-          'put',
-          'global',
-          entry.key,
-          entry.value,
-        ],
-      );
-
-      if (result.exitCode != 0) {
-        throw StateError('Failed to configure device: ${result.stderr}');
-      }
-    }
-  }
-
   @override
   Future<NativeScreenshot> screenshot() async {
     // Similar pause to the one in `<FlutterDriver>.screenshot()`.

packages/flutter_driver/lib/src/native/driver.dart

@@ -26,16 +26,6 @@ abstract interface class NativeDriver {
   /// After calling this method, the driver is no longer usable.
   Future<void> close();
 
-  /// Configures the device for screenshot testing.
-  ///
-  /// Where possible, this method should suppress system UI elements that are
-  /// not part of the application under test, such as the status bar or
-  /// navigation bar, and disable animations that might interfere with
-  /// screenshot comparison.
-  ///
-  /// The exact details of what is configured are platform-specific.
-  Future<void> configureForScreenshotTesting();
-
   /// Take a screenshot using a platform-specific mechanism.
   ///
   /// The image is returned as an opaque handle that can be used to retrieve

packages/flutter_goldens/lib/skia_client.dart

@@ -331,7 +331,11 @@ class SkiaGoldClient {
   ///
   /// The [testName] and [goldenFile] parameters reference the current
   /// comparison being evaluated by the [FlutterPreSubmitFileComparator].
-  Future<void> tryjobAdd(String testName, File goldenFile) async {
+  ///
+  /// If the tryjob fails due to pixel differences, the method will succeed
+  /// as the failure will be triaged in the 'Flutter Gold' dashboard, and the
+  /// `stdout` will contain the failure message; otherwise will return `null`.
+  Future<String?> tryjobAdd(String testName, File goldenFile) async {
     final List<String> imgtestCommand = <String>[
       _goldctl,
       'imgtest', 'add',
@@ -368,6 +372,7 @@ class SkiaGoldClient {
         ..writeln('result-state.json: ${resultContents ?? 'No result file found.'}');
       throw SkiaException(buf.toString());
     }
+    return result.exitCode == 0 ? null : resultStdout;
   }
 
   // Constructs arguments for `goldctl` for controlling how pixels are compared.

packages/flutter_goldens/test/flutter_goldens_test.dart

@@ -1182,7 +1182,7 @@ class FakeSkiaGoldClient extends Fake implements SkiaGoldClient {
   @override
   Future<void> tryjobInit() async => tryInitCalls += 1;
   @override
-  Future<bool> tryjobAdd(String testName, File goldenFile) async => true;
+  Future<String?> tryjobAdd(String testName, File goldenFile) async => null;
 
   Map<String, List<int>> imageBytesValues = <String, List<int>>{};
   @override