Reland "[data_assets] Cleanup tests" by dcharkes · Pull Request #184714 · flutter/flutter

dcharkes · 2026-04-07T07:48:25Z

Relands #184209 adding a mac shard as suggested in #184209 (comment).

For adding a shard, I followed #154444, which suggests it's contained in the single .ci.yaml file, and the new shard names need to have bringup: true.

Original PR description:

This cleans up the data_assets experiment testing.

Use integration test projects, which are statically analyzed, formatted, etc.
This forces us to be explicit about the package:data_assets dependency.
This means the test projects will roll their dependencies with the dependency auto roller. (The test project was still using native_assets_cli which has been replaced with hooks and data_assets.)
Pin package:data_assets, because it's experimental, and we might do breaking changes.

Also makes the test projects more useful:

The data_assets_app now displays the assets in the UI. (Not only prints to the console for the integration tests.)
The data_assets_package now actually uses its asset.

And cleans up the test projects:

Move the test files into the data/ dir.
Give the test data assets a proper extension.

I saw weird behavior on Windows:

DevFS doesn't pick up the writes to the Dart files, not even with flushing or sleeps: Compiling dart to kernel with 0 updated files leading to a state where the Dart variable contents don't get updated while the data assets do. This integration test is not really about Dart hot restart, so I've skipped those checks on Windows.
This reproduces locally in the integration test, but not when running the app and editing the file.

The checks in dev/bots/analyze.dart also required some updates:

The template files check didn't allow for the HTML license.

P.S. Sorry for all the generated Flutter scaffolding. It seems that that is the way it's done in the integration test directory. (The package with the data assets is clean, but the Flutter app ...)

gemini-code-assist

Code Review

This pull request introduces a comprehensive integration test suite for Dart data assets, including a new test application and a dependency package. It updates CI targets to incorporate these tests, pins the data_assets package version, and refactors license verification logic to handle HTML formats. Feedback suggests enhancing error messages during asset loading, implementing concurrency guards for asynchronous timers in the test application, and adopting more robust regular expressions for string replacements in test scripts.

gemini-code-assist · 2026-04-07T08:00:52Z

+    try {
+      found[assetId] = await rootBundle.loadString(assetId);
+    } catch (e) {
+      print('EXCEPTION $e');


The error message EXCEPTION $e is not very descriptive. Including the assetId that failed to load would make debugging much easier if the integration test fails.

Suggested change

print('EXCEPTION $e');

print('EXCEPTION loading $assetId: $e');

gemini-code-assist · 2026-04-07T08:00:53Z

+    _timer = Timer.periodic(const Duration(milliseconds: 500), (_) async {
+      final AssetData? appAssetData = await loadAppAssets();
+      final String dependencyAssetContent = await data_asset_package.loadAssetContent();
+      // Print for integration test discovery
+      if (appAssetData != null) {
+        print('VERSION: ${appAssetData.version}');
+        for (final MapEntry(:key, :value) in appAssetData.found.entries) {
+          print('FOUND "$key": "$value".');
+        }
+        for (final String id in appAssetData.notFound) {
+          print('NOT_FOUND "$id".');
+        }
+      }
+      print('DEPENDENCY_ASSET: $dependencyAssetContent');
+
+      if (mounted) {
+        setState(() {
+          if (appAssetData != null) {
+            _assetData = appAssetData;
+          }
+          _dependencyAsset = dependencyAssetContent;
+        });
+      }
+    });


Using Timer.periodic with an async callback can lead to overlapping executions if the asynchronous operations take longer than the timer interval. This can cause race conditions or noisy output in integration tests. Consider using a flag to prevent concurrent executions.

bool isProcessing = false; _timer = Timer.periodic(const Duration(milliseconds: 500), (_) async { if (isProcessing) { return; } isProcessing = true; try { final AssetData? appAssetData = await loadAppAssets(); final String dependencyAssetContent = await data_asset_package.loadAssetContent(); // Print for integration test discovery if (appAssetData != null) { print('VERSION: ${appAssetData.version}'); for (final MapEntry(:key, :value) in appAssetData.found.entries) { print('FOUND "$key": "$value".'); } for (final String id in appAssetData.notFound) { print('NOT_FOUND "$id".'); } } print('DEPENDENCY_ASSET: $dependencyAssetContent'); if (mounted) { setState(() { if (appAssetData != null) { _assetData = appAssetData; } _dependencyAsset = dependencyAssetContent; }); } } finally { isProcessing = false; } });

gemini-code-assist · 2026-04-07T08:00:53Z

+      entity.copySync(targetFile.path);
+      if (relativePath == 'pubspec.yaml') {
+        String content = targetFile.readAsStringSync();
+        content = content.replaceFirst('resolution: workspace', '');


The string replacement replaceFirst('resolution: workspace', '') is fragile as it depends on exact spacing and doesn't handle potential comments or trailing whitespace on the same line. Consider using a regular expression to match the line more robustly.

Suggested change

content = content.replaceFirst('resolution: workspace', '');

content = content.replaceFirst(RegExp(r'^\s*resolution:\s*workspace\s*$', multiLine: true), '');

flutter-dashboard · 2026-04-07T08:41:12Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #184714 at sha 18a26b6

goderbauer

LGTM

flutter/flutter@9cd60b5...a0924c7 2026-04-07 [email protected] Reland "[data_assets] Cleanup tests" (flutter/flutter#184714) 2026-04-07 [email protected] Use the WindowRegistry in the multiple_windows example app (flutter/flutter#184579) 2026-04-07 [email protected] Introduce command to build a swift package for SwiftPM add to app integration (flutter/flutter#184660) 2026-04-07 [email protected] Have `flutter create` create a pubspec.lock to ensure pinned versions are being used. (flutter/flutter#175352) 2026-04-07 [email protected] [widgets/raw_menu_anchor.dart] Always call onClose and onCloseRequested on descendants before parent. (flutter/flutter#182357) 2026-04-07 [email protected] `WindowsPlugin` should not crash when ffiPlugin enabled (flutter/flutter#184695) 2026-04-06 [email protected] Use full goto.google.com hostname for go/ links (flutter/flutter#184679) 2026-04-06 [email protected] Apply rect clipping to surface views (flutter/flutter#184471) 2026-04-06 [email protected] [A11y] Allow percentage strings like "50%" as `SemanticsValue` for `ProgressIndicator` (flutter/flutter#183670) 2026-04-06 [email protected] Fix invisible accessibility element before scroll view (flutter/flutter#184155) 2026-04-06 [email protected] Roll Skia from 163dfdf500c7 to e264d870a380 (2 revisions) (flutter/flutter#184674) 2026-04-06 [email protected] Keep last character obscured when toggling obscureText (flutter/flutter#183488) 2026-04-06 [email protected] Parse scheme file with XML parser for SwiftPM migrator (flutter/flutter#184525) 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 Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Relands flutter#184209 adding a mac shard as suggested in flutter#184209 (comment). For adding a shard, I followed flutter#154444, which suggests it's contained in the single `.ci.yaml` file, and the new shard names need to have `bringup: true`. ---- Original PR description: This cleans up the data_assets experiment testing. * Use integration test projects, which are statically analyzed, formatted, etc. * This forces us to be explicit about the `package:data_assets` dependency. * This means the test projects will roll their dependencies with the dependency auto roller. (The test project was still using `native_assets_cli` which has been replaced with `hooks` and `data_assets`.) * Pin `package:data_assets`, because it's experimental, and we might do breaking changes. Also makes the test projects more useful: * The `data_assets_app` now displays the assets in the UI. (Not only prints to the console for the integration tests.) * The `data_assets_package` now actually uses its asset. And cleans up the test projects: * Move the test files into the `data/` dir. * Give the test data assets a proper extension. I saw weird behavior on Windows: * DevFS doesn't pick up the writes to the Dart files, not even with flushing or sleeps: `Compiling dart to kernel with 0 updated files` leading to a state where the Dart variable contents don't get updated while the data assets do. This integration test is not really about Dart hot restart, so I've skipped those checks on Windows. * This reproduces locally in the integration test, but not when running the app and editing the file. The checks in dev/bots/analyze.dart also required some updates: * The template files check didn't allow for the HTML license. P.S. Sorry for all the generated Flutter scaffolding. It seems that that is the way it's done in the integration test directory. (The package with the data assets is clean, but the Flutter app ...)

Reland "[data_assets] Cleanup tests"

2413a18

github-actions Bot added tool Affects the "flutter" command-line tool. See also t: labels. a: desktop Running on desktop labels Apr 7, 2026

dcharkes added CICD Run CI/CD and removed tool Affects the "flutter" command-line tool. See also t: labels. a: desktop Running on desktop CICD Run CI/CD labels Apr 7, 2026

github-actions Bot added tool Affects the "flutter" command-line tool. See also t: labels. a: desktop Running on desktop labels Apr 7, 2026

dcharkes added the CICD Run CI/CD label Apr 7, 2026

add mac shards

18a26b6

dcharkes force-pushed the reland-data-asset-cleanup branch from 349f0c3 to 18a26b6 Compare April 7, 2026 07:56

github-actions Bot removed the CICD Run CI/CD label Apr 7, 2026

dcharkes added the CICD Run CI/CD label Apr 7, 2026

dcharkes marked this pull request as ready for review April 7, 2026 07:58

dcharkes requested a review from goderbauer April 7, 2026 07:58

gemini-code-assist Bot reviewed Apr 7, 2026

View reviewed changes

flutter-dashboard Bot added the will affect goldens Changes to golden files label Apr 7, 2026

goderbauer approved these changes Apr 7, 2026

View reviewed changes

dcharkes mentioned this pull request Apr 7, 2026

[record_use] Add experimental flag and test project #184719

Merged

dcharkes added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 7, 2026

auto-submit Bot added this pull request to the merge queue Apr 7, 2026

Merged via the queue into flutter:master with commit a0924c7 Apr 7, 2026
153 checks passed

flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 7, 2026

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 7, 2026

a0924c76b Reland "[data_assets] Cleanup tests" (flutter/flutter#184714)

c3c72b3

engine-flutter-autoroll mentioned this pull request Apr 7, 2026

Roll Flutter from 9cd60b512fbc to a0924c76b0fd (13 revisions) flutter/packages#11462

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 7, 2026

a0924c76b Reland "[data_assets] Cleanup tests" (flutter/flutter#184714)

12280d3

engine-flutter-autoroll mentioned this pull request Apr 7, 2026

Roll Flutter from 9cd60b512fbc to a0924c76b0fd (13 revisions) flutter/packages#11463

Merged

engine-flutter-autoroll mentioned this pull request Apr 7, 2026

Roll Flutter from 9cd60b512fbc to a0924c76b0fd (13 revisions) flutter/packages#11464

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 7, 2026

a0924c76b Reland "[data_assets] Cleanup tests" (flutter/flutter#184714)

3f10319

engine-flutter-autoroll mentioned this pull request Apr 7, 2026

Roll Flutter from 9cd60b512fbc to b185b02c1d07 (14 revisions) flutter/packages#11465

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 7, 2026

a0924c76b Reland "[data_assets] Cleanup tests" (flutter/flutter#184714)

6e47571

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reland "[data_assets] Cleanup tests"#184714

Reland "[data_assets] Cleanup tests"#184714
auto-submit[bot] merged 2 commits into
flutter:masterfrom
dcharkes:reland-data-asset-cleanup

dcharkes commented Apr 7, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Apr 7, 2026

Uh oh!

gemini-code-assist Bot Apr 7, 2026

Uh oh!

gemini-code-assist Bot Apr 7, 2026

Uh oh!

flutter-dashboard Bot commented Apr 7, 2026

Uh oh!

goderbauer left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	print('EXCEPTION $e');
	print('EXCEPTION loading $assetId: $e');

	content = content.replaceFirst('resolution: workspace', '');
	content = content.replaceFirst(RegExp(r'^\sresolution:\sworkspace\s*$', multiLine: true), '');

Conversation

dcharkes commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard Bot commented Apr 7, 2026

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dcharkes commented Apr 7, 2026 •

edited

Loading