Skip to content

[data_assets] Cleanup tests#184209

Merged
dcharkes merged 3 commits into
flutter:masterfrom
dcharkes:data-assets-test-cleanup
Apr 2, 2026
Merged

[data_assets] Cleanup tests#184209
dcharkes merged 3 commits into
flutter:masterfrom
dcharkes:data-assets-test-cleanup

Conversation

@dcharkes

@dcharkes dcharkes commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

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 ...)

@github-actions github-actions Bot added tool Affects the "flutter" command-line tool. See also t: labels. a: desktop Running on desktop labels Mar 26, 2026
@dcharkes dcharkes added the CICD Run CI/CD label Mar 26, 2026
@dcharkes dcharkes force-pushed the data-assets-test-cleanup branch from 9b26dbe to c727628 Compare March 26, 2026 17:40
@github-actions github-actions Bot removed the CICD Run CI/CD label Mar 26, 2026
@dcharkes dcharkes added the CICD Run CI/CD label Mar 27, 2026
@dcharkes dcharkes force-pushed the data-assets-test-cleanup branch from c727628 to b91ad02 Compare March 27, 2026 09:24
@github-actions github-actions Bot removed the CICD Run CI/CD label Mar 27, 2026
@dcharkes dcharkes added the CICD Run CI/CD label Mar 27, 2026
@dcharkes dcharkes force-pushed the data-assets-test-cleanup branch from b91ad02 to a0230a7 Compare March 27, 2026 11:38
@github-actions github-actions Bot removed the CICD Run CI/CD label Mar 27, 2026
@dcharkes dcharkes added the CICD Run CI/CD label Mar 27, 2026
@dcharkes dcharkes force-pushed the data-assets-test-cleanup branch from a0230a7 to 0e4e081 Compare March 27, 2026 13:49
@github-actions github-actions Bot removed the CICD Run CI/CD label Mar 27, 2026
@dcharkes dcharkes added the CICD Run CI/CD label Mar 27, 2026
@github-actions github-actions Bot removed the CICD Run CI/CD label Mar 27, 2026
@dcharkes dcharkes added the CICD Run CI/CD label Mar 27, 2026
@github-actions github-actions Bot removed the CICD Run CI/CD label Mar 27, 2026
@dcharkes dcharkes added the CICD Run CI/CD label Mar 27, 2026
@github-actions github-actions Bot removed the CICD Run CI/CD label Mar 27, 2026
@dcharkes dcharkes added the CICD Run CI/CD label Mar 27, 2026
@dcharkes

dcharkes commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

@jtmcdole I'm not sure who to ping on this one. You? If not please cc the right person.

I can't seem to get the bots happy with new integration test projects:

  • The icons are not allowed because they are not utf8? But all the other apps in this dir have icons.
    • Probably the existing project were landed before these checks were added?
    • Edit: Deleted all the icons
  • The generated scaffolding must have a license header or the bots go red AND they must be identical to the templates or the bots go red. This is a contraction, preventing adding new integration test projects.
    • I'm thinking the existing projects were probably landed before the checks were added and then the checks on run on changed files.
    • Edit: Fixed the template check to allow for the HTML copyright comment

What is the way forward here? (Having test projects in Dart Strings is not the way forward IMO, that's what this PR is trying to fix.)

@dcharkes dcharkes added CICD Run CI/CD and removed CICD Run CI/CD labels Mar 30, 2026
@dcharkes dcharkes force-pushed the data-assets-test-cleanup branch from 2656b4c to 7c67468 Compare March 31, 2026 08:19
@github-actions github-actions Bot removed the CICD Run CI/CD label Apr 2, 2026
@dcharkes dcharkes added the CICD Run CI/CD label Apr 2, 2026

@goderbauer goderbauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Nice clean-up! 🚀

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

auto-submit Bot commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

auto label is removed for flutter/flutter/184209, Failed to enqueue flutter/flutter/184209 with HTTP 400: GraphQL mutate failed.

@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Apr 2, 2026
@dcharkes dcharkes added this pull request to the merge queue Apr 2, 2026
Merged via the queue into flutter:master with commit e13b5e6 Apr 2, 2026
159 checks passed
@dcharkes dcharkes deleted the data-assets-test-cleanup branch April 2, 2026 20:16
@hellohuanlin

hellohuanlin commented Apr 3, 2026

Copy link
Copy Markdown
Contributor
Screenshot 2026-04-02 at 10 07 47 PM

Hi @dcharkes, it looks like this PR is the first commit that fails build_tests_2_4: https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_x64%20build_tests_2_4/11979/overview

I've suppressed it while investigating it

@dcharkes

dcharkes commented Apr 3, 2026

Copy link
Copy Markdown
Contributor Author
Screenshot 2026-04-02 at 10 07 47 PM Hi @dcharkes, it looks like this PR is the first commit that fails `build_tests_2_4`: https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_x64%20build_tests_2_4/11979/overview

I've suppressed it while investigating it

@hellohuanlin It looks like this PR might have made the test slower? I didn't change the test significantly it still runs the same amount of Flutter commands. (But 16 minutes to 35-36 minutes is a big jump!) Edit: I don't see the test that I changed in the relevant logs. Maybe something else became slower?

@hellohuanlin

Copy link
Copy Markdown
Contributor

@dcharkes noob question: how can you tell it fails because things get slower?

I think i'm gonna revert it first. the longer we suppress the test the higher the risk.

@hellohuanlin hellohuanlin added the revert Autorevert PR (with "Reason for revert:" comment) label Apr 3, 2026
@hellohuanlin

Copy link
Copy Markdown
Contributor

Reason for revert: breaks the tree

auto-submit Bot pushed a commit that referenced this pull request Apr 3, 2026
@auto-submit auto-submit Bot removed the revert Autorevert PR (with "Reason for revert:" comment) label Apr 3, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 3, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Apr 3, 2026
flutter/flutter@0f401ee...7245c3f

2026-04-03 [email protected] Roll Skia from c07c67045b6d to 5d847ba5c4aa (1 revision) (flutter/flutter#184570)
2026-04-03 [email protected] Roll Dart SDK from 3c7a79045b8b to 46f49142acd9 (1 revision) (flutter/flutter#184567)
2026-04-03 [email protected] Roll ICU from ee5f27adc28b to ff7995a708a1 (5 revisions) (flutter/flutter#184566)
2026-04-03 [email protected] Roll Skia from 9ae8231be181 to c07c67045b6d (4 revisions) (flutter/flutter#184562)
2026-04-03 [email protected] Roll Fuchsia Linux SDK from BFLjk6Uwd0gs_Hkdk... to PpL3Bn2YMb2h9LbdK... (flutter/flutter#184556)
2026-04-03 [email protected] Roll Skia from 0566b2f5f0d1 to 9ae8231be181 (1 revision) (flutter/flutter#184547)
2026-04-03 [email protected] Roll Dart SDK from 6008eaddd589 to 3c7a79045b8b (3 revisions) (flutter/flutter#184551)
2026-04-03 [email protected] Fix wide gamut macos integration test (flutter/flutter#184427)
2026-04-02 [email protected] forward an application name to DDS (flutter/flutter#184459)
2026-04-02 [email protected] Roll Skia from 973117cfa875 to 0566b2f5f0d1 (8 revisions) (flutter/flutter#184534)
2026-04-02 [email protected] Support different joins for stroked rects in uber_sdf, fix incorrect aa (flutter/flutter#184395)
2026-04-02 [email protected] [ Widget Preview ] Handle collections and records in custom preview annotations (flutter/flutter#184518)
2026-04-02 [email protected] Moves android_semantics_integration_test out of staging (flutter/flutter#184079)
2026-04-02 [email protected] Roll Packages from b3fcf14 to 66bf7ec (4 revisions) (flutter/flutter#184514)
2026-04-02 [email protected] Fix line breaks being lost when copying after selection gesture in SelectableRegion (flutter/flutter#184421)
2026-04-02 [email protected] Add plugin version to SwiftPM package symlink directory (flutter/flutter#183668)
2026-04-02 [email protected] Add our own wrapper for `CommonExtension` due to change in signature from 8.x->9.0 (flutter/flutter#184433)
2026-04-02 [email protected] [Android] Use EdgeToEdge.enable/WindowCompat for edge-to-edge mode instead of deprecated View flags (flutter/flutter#183072)
2026-04-02 [email protected] [data_assets] Cleanup tests (flutter/flutter#184209)
2026-04-02 [email protected] Enable SPM by default on Stable (flutter/flutter#184495)
2026-04-02 [email protected] Roll Dart SDK from d84bdfeb45eb to 6008eaddd589 (2 revisions) (flutter/flutter#184513)
2026-04-02 [email protected] Reland "Even more awaits" (flutter/flutter#184467)
2026-04-02 [email protected] Roll Skia from bb9fd8653739 to 973117cfa875 (2 revisions) (flutter/flutter#184498)
2026-04-02 [email protected] [ Widget Preview ] Use analysis server for widget preview detection (flutter/flutter#184473)
2026-04-02 [email protected] [web_ui] Fix avoid_type_to_string lint violation (flutter/flutter#184342)

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
github-merge-queue Bot pushed a commit that referenced this pull request Apr 3, 2026
<!-- start_original_pr_link -->
Reverts: #184209
<!-- end_original_pr_link -->
<!-- start_initiating_author -->
Initiated by: hellohuanlin
<!-- end_initiating_author -->
<!-- start_revert_reason -->
Reason for reverting: 

Breaks `Mac_x64 build_tests_2_4`. See
https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_x64%20build_tests_2_4/11979/overview.
Test running time regressed from 15-16 minutes ish to 36 minutes. The
tree is toggling between red & green, so i guess our timeout is likely
close to 35 minutes or so?

<!-- end_revert_reason -->
<!-- start_original_pr_author -->
Original PR Author: dcharkes
<!-- end_original_pr_author -->

<!-- start_reviewers -->
Reviewed By: {goderbauer}
<!-- end_reviewers -->

<!-- start_revert_body -->
This change reverts the following previous change:
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 ...)
<!-- end_revert_body -->

Co-authored-by: auto-submit[bot] <[email protected]>
@jason-simmons

Copy link
Copy Markdown
Member

This PR caused the Mac_x64 build_tests_2_4 builder to exceed a timeout becuase it added a new test that reshuffled the contents of the build_tests shards.

The build_tests suite currently has 4 shards. The test script tries to assign an equal number of tests to each shard. But the tests can vary widely in how much time they consume.

This PR added a new test to build_tests (dev/integration_tests/data_asset_app). That changed the contents of each shard, and many of the most time-consuming tests were assigned to shard 2. So execution of shard 2 now exceeded the 30 minute timeout for the test runner step.

It should be possible to reland this PR by increasing the number of build_tests shards. Or by increasing the timeout for each shard.

@dcharkes

dcharkes commented Apr 7, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the help @jason-simmons 🙏

github-merge-queue Bot pushed a commit that referenced this pull request Apr 7, 2026
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 ...)
mbcorona pushed a commit to mbcorona/flutter that referenced this pull request Apr 15, 2026
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 ...)
mbcorona pushed a commit to mbcorona/flutter that referenced this pull request Apr 15, 2026
<!-- start_original_pr_link -->
Reverts: flutter#184209
<!-- end_original_pr_link -->
<!-- start_initiating_author -->
Initiated by: hellohuanlin
<!-- end_initiating_author -->
<!-- start_revert_reason -->
Reason for reverting: 

Breaks `Mac_x64 build_tests_2_4`. See
https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_x64%20build_tests_2_4/11979/overview.
Test running time regressed from 15-16 minutes ish to 36 minutes. The
tree is toggling between red & green, so i guess our timeout is likely
close to 35 minutes or so?

<!-- end_revert_reason -->
<!-- start_original_pr_author -->
Original PR Author: dcharkes
<!-- end_original_pr_author -->

<!-- start_reviewers -->
Reviewed By: {goderbauer}
<!-- end_reviewers -->

<!-- start_revert_body -->
This change reverts the following previous change:
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 ...)
<!-- end_revert_body -->

Co-authored-by: auto-submit[bot] <[email protected]>
mbcorona pushed a commit to mbcorona/flutter that referenced this pull request Apr 15, 2026
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 ...)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: desktop Running on desktop CICD Run CI/CD tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants