[ci] Add a legacy Android build-all test #4005

stuartmorgan · 2023-05-16T14:00:31Z

Adds the ability to replace portions of the flutter created app with saved copies, and adds a second build-all phase for Android that uses a Flutter 2.0.6-created android/ directory (AGP 4.1, Gradle 6.7) to catch issues like flutter/flutter#125621 and flutter/flutter#125482 prior to release.

Includes some incidental cleanup:

Extracts a helper method for adjusting files, so that this doesn't add even more copies of basically identical code.
- (This was motivated by an earlier version of the PR that added modifications to several more files, which I ended up undoing, but the cleanup seemed worth keeping.)
Adds missing unit test coverage.
Reworks the unit tests to use a mock process manager and dummy files, instead of each test actually calling flutter create, which made each new test add several seconds to the unit test suite.
- While this reduces the integration-style coverage, in practice the integration tests of the repo tooling is the CI itself, so the unit tests should be true unit tests.
Changes the non-legacy test to Kotlin; we were still testing with Java even though Kotlin has been the default for quite a while, so we weren't testing what most new users would actually be running. Since we now have a legacy test, I used Java there to cover both.
Removes some dead code for modifying the AndroidManifest.xml; when trying to set up unit tests for it I discovered that it no longer matches anything in an actual project. It dates back to the original command, and seems to have been a camera-related hack, which we clearly no longer need since it wasn't working and camera still works in build-all.

This is captured somewhat in the README in the legacy project directory, but to document the approach here: originally I was going to add flags to change individual items (AGP version, Gradle version), but quickly ran into the fact that selective downgrading is extremely fragile. E.g.,:

The Kotlin version set in current projects doesn't work when downgrading AGP and Gradle.
The app template can unconditionally use anything (e.g., namespace) that the AGP version it uses supports, so arbitrary future breakage is possible.

It's also less useful as a real test of what a plugin client's project likely looks like. Starting with a complete platform directory, and doing whatever the minimal changes are to keep it working, will likely reflect a common real-world scenario. On the flip side, the reason this doesn't use a complete 2.0.6 project, but instead is based on specific platform directories, is that we don't want to waste time manually maintaining, e.g., old Dart code that is irrelevant to the goal of the test. For now this only uses Android because that's where we've seen problems in practice, but we can alway add other legacy platform tests later if we find a need.

Fixes flutter/flutter#125689

Pre-launch Checklist

reidbaker · 2023-05-22T15:06:22Z

.ci/legacy_project/README.md

+
+It was originally created with Flutter 2.0.6. In general the guidelines are:
+- Pieces here should be self-contained; avoid making a hybrid of old and new
+  projects except at major cut points (e.g., platform directories). This is


Can you expound on this a bit in the review? What is a major cut point? The example of platform directories didn't help me understand how to evaluate if flutter has hit another major cut point or not.

I rewrote it to hopefully be clearer. What I was describing as "cut points" was meant to be about project structure, not Flutter releases. So for instance "the android directory" is a major cut point, whereas "the Java code from the android directory" would not be, because it can be strongly coupled to resources or configs that are not within that subset.

.ci/legacy_project/README.md

.ci/legacy_project/all_packages/android/app/build.gradle

.cirrus.yml

reidbaker · 2023-05-22T15:16:04Z

script/tool/lib/src/create_all_packages_app_command.dart

 import 'common/package_command.dart';
 import 'common/process_runner.dart';
 import 'common/repository_package.dart';

-const String _outputDirectoryFlag = 'output-dir';
+@visibleForTesting



Whitespace after visibleForTesting

Fixed, I forgot the autoformatter wants it after the declaration comment.

reidbaker · 2023-05-22T15:17:50Z

script/tool/lib/src/create_all_packages_app_command.dart


-const String _projectName = 'all_packages';
+/// The name of the build-all-packages project.


Can the comment state the intended format? For example The name of the build-all-packages project to be passed into flutter create This helps differentiate it from the name of the app or if this is really just a directory.

script/tool/lib/src/create_all_packages_app_command.dart

reidbaker · 2023-05-22T15:21:55Z

script/tool/lib/src/create_all_packages_app_command.dart

@@ -98,80 +114,152 @@ class CreateAllPackagesAppCommand extends PackageCommand {
  }

  Future<int> _createApp() async {
-    final io.ProcessResult result = io.Process.runSync(
+    return processRunner.runAndStream(


Is this modifying the current all packages test to use kotlin?

Yes; this is mentioned in the PR description. I'm not sure why it wasn't already, given that it's been the default for Flutter developers for a long time.

yeah I read that in the description and was looking for the place where we made the swap. Where in the cl do we test the legacy version with java?

auto-submit · 2023-05-22T19:39:17Z

auto label is removed for flutter/packages, pr: 4005, due to - The status or check suite downgraded_analyze has failed. Please fix the issues identified (or deflake) before re-applying this label.

The status or check suite legacy_version_analyze CHANNEL:3.3.10 DART_VERSION:2.18.6 has failed. Please fix the issues identified (or deflake) before re-applying this label.

* main: (104 commits) [various] Remove unnecessary null checks (flutter#4060) [ci] Add a legacy Android build-all test (flutter#4005) Roll Flutter from ab57304 to 3437189 (5 revisions) (flutter#4062) [go_router] Nested stateful navigation with ShellRoute (flutter#2650) [go_router] Nested stateful navigation with ShellRoute (flutter#2650) [go_router] Nested stateful navigation with ShellRoute (flutter#2650) [in_app_purchases] Fix mismatching method signature strings (flutter#4040) [go_router] Nested stateful navigation with ShellRoute (flutter#2650) [go_router] Nested stateful navigation with ShellRoute (flutter#2650) [go_router] Nested stateful navigation with ShellRoute (flutter#2650) Roll Flutter from 077d644 to ab57304 (18 revisions) (flutter#4051) Roll Flutter from 5ae6438 to 077d644 (23 revisions) (flutter#4043) [local_auth] Migrate iOS to Pigeon (flutter#3974) [go_router] fix context extension for replaceNamed (flutter#3927) [image_picker] Fix crash due to `SecurityException` (flutter#4004) Roll Flutter from d0d1feb to 5ae6438 (42 revisions) (flutter#4038) [ci] Lower iOS LUCI timeouts (flutter#4035) [ci] Increase Android sharding (flutter#4029) [flutter_plugin_android_lifecycle] Fix lints (flutter#4030) [rfw] Fix a typo in the API documentation (flutter#4023) ...

flutter/packages@83959fb...d449a17 2023-05-22 [email protected] [various] Remove unnecessary null checks (flutter/packages#4060) 2023-05-22 [email protected] [ci] Add a legacy Android build-all test (flutter/packages#4005) 2023-05-22 [email protected] Roll Flutter from ab57304 to 3437189 (5 revisions) (flutter/packages#4062) 2023-05-22 [email protected] [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650) 2023-05-22 [email protected] [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650) 2023-05-22 [email protected] [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

stuartmorgan added 19 commits May 16, 2023 09:48

WIP

ffa519c

Simplify updates with common code

4c37f43

Add flags, switch default to Kotlin

216a43a

Add platforms flag, and rework tests

99f0f96

Add Android tests, remove dead code

79c39e5

Remove --platforms, test --android-language

8191176

Add legacy build-all for Android in CI

b30a4b4

Fix platform mocking for changes

e077b37

Fix different dependencies formats

6a2b491

Specify Kotlin version as well

ece81a4

Add a 2.0.6-created project

ce2bef3

Remove all but Android

4d8f614

Minor cleanup

ae1650d

README explaining directory

3c6e658

Add a flag for a legacy project partial replacement

ed1f5dc

Undo all of the added flags

a27dfa9

Update CI step

78455c6

Move legacy source

27b2751

Fix licensee and README

eeb2c16

stuartmorgan requested a review from reidbaker May 17, 2023 14:36

Merge branch 'main' into tool-build-all-old-gradle

453f909

reidbaker approved these changes May 22, 2023

View reviewed changes

Address review comments

8bab358

reidbaker approved these changes May 22, 2023

View reviewed changes

stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label May 22, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 22, 2023

stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label May 22, 2023

auto-submit bot merged commit 30ebcf3 into flutter:main May 22, 2023

engine-flutter-autoroll mentioned this pull request May 23, 2023

Roll Packages from 83959fb7de37 to d449a17f8706 (6 revisions) flutter/flutter#127394

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 23, 2023

30ebcf3 [ci] Add a legacy Android build-all test (flutter/packages#4005)

96a2efe

stuartmorgan mentioned this pull request Jun 29, 2023

Convert plugin tooling all-plugins-app test to mock process flutter/flutter#114153

Closed

This was referenced Jul 7, 2023

[Tool] New tool to download android dependencies #4408

Merged

[camera] tree fix format 2023 05 12 #4450

Closed

[tooling] mark infra steps as infra steps in yaml files #4473

Merged

reidbaker mentioned this pull request Jul 26, 2023

[Tooling] Add google owned cache for dependencies as an option in ci #4567

Merged

11 tasks

reidbaker mentioned this pull request Aug 4, 2023

[All] Expand artifact hub to all plugins #4645

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ci] Add a legacy Android build-all test #4005

[ci] Add a legacy Android build-all test #4005

stuartmorgan commented May 16, 2023 •

edited

Loading

reidbaker May 22, 2023

stuartmorgan May 22, 2023

reidbaker May 22, 2023

stuartmorgan May 22, 2023

reidbaker May 22, 2023

stuartmorgan May 22, 2023

reidbaker May 22, 2023

stuartmorgan May 22, 2023 •

edited

Loading

reidbaker May 22, 2023

auto-submit bot commented May 22, 2023


		const String _projectName = 'all_packages';
		/// The name of the build-all-packages project.

[ci] Add a legacy Android build-all test #4005

[ci] Add a legacy Android build-all test #4005

Conversation

stuartmorgan commented May 16, 2023 • edited Loading

Pre-launch Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuartmorgan May 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

auto-submit bot commented May 22, 2023

stuartmorgan commented May 16, 2023 •

edited

Loading

stuartmorgan May 22, 2023 •

edited

Loading