Skip to content

Conversation

@mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Oct 28, 2025

Fix generate-builder-json to only include test bundles that are needed by enabled test suites.

Before this PR

The script was unconditionally including all test bundles in CI. The result is that the dart2js-canvaskit-experimental-webparagraph bundle was being generated, even though it was only required by the chrome-dart2js-experimental-webparagraph-ui suite, which had enable-ci: false.

After this PR

The script starts by finding all test suites with enable-ci: true, then only adds the bundles required by those suites.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@github-actions github-actions bot added engine flutter/engine related. See also e: labels. platform-web Web applications specifically labels Oct 28, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the generate-builder-json script to only include test bundles for test suites that are enabled in CI. This is done by filtering suites by the enableCi flag at the beginning of the process. Consequently, a redundant enableCi check is removed from a downstream function. The changes are logical and correctly implement the intended optimization. I've provided one suggestion to make the code slightly more concise and efficient.

Comment on lines 32 to 35
final List<TestBundle> testBundles = testSuites
.map((suite) => suite.testBundle)
.toSet()
.toList();
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The .toList() call here is unnecessary because testBundles is only used in the next statement where .map is called, and Set also supports the .map method. You can remove the conversion to a list and keep testBundles as a Set<TestBundle>. This is slightly more efficient and the expression can be made more concise.

Suggested change
final List<TestBundle> testBundles = testSuites
.map((suite) => suite.testBundle)
.toSet()
.toList();
final Set<TestBundle> testBundles = testSuites.map((suite) => suite.testBundle).toSet();

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a valid change but it's not super necessary.

@mdebbar mdebbar force-pushed the autogen_webparagraph branch from 16656bc to c4d8426 Compare November 4, 2025 16:58
Copy link
Contributor

@harryterkelsen harryterkelsen left a comment

Choose a reason for hiding this comment

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

LGTM with AI nit

Comment on lines 32 to 35
final List<TestBundle> testBundles = testSuites
.map((suite) => suite.testBundle)
.toSet()
.toList();
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a valid change but it's not super necessary.

@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 4, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Nov 5, 2025
Merged via the queue into flutter:master with commit 3fd81ed Nov 5, 2025
184 of 185 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 6, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Nov 6, 2025
flutter/flutter@e5d5c01...c5e809a

2025-11-06 [email protected] Remove WindowingOwner.hasTopLevelWindows (flutter/flutter#178033)
2025-11-06 [email protected] Fix DropdownMenu escape key does not close the menu (flutter/flutter#178002)
2025-11-06 [email protected] Update documentation tool reference in image.dart (flutter/flutter#177782)
2025-11-06 [email protected] Roll Skia from 4eb2383d38f2 to 5c4e1352128f (5 revisions) (flutter/flutter#178094)
2025-11-06 [email protected] Revert "Refactor OverlayPortal semantics (#173005)" (flutter/flutter#178007)
2025-11-06 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Fix verified input test failure in CI (attempt 4) (#178018)" (flutter/flutter#178089)
2025-11-06 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[web] Unify Surface code between Skwasm and CanvasKit (#177138)" (flutter/flutter#178085)
2025-11-05 [email protected] Fix verified input test failure in CI (attempt 4) (flutter/flutter#178018)
2025-11-05 [email protected] fix: inconsistent horizontal spacing between hours and mins in time picker for non-english language (flutter/flutter#173706)
2025-11-05 [email protected] Roll Fuchsia Linux SDK from mpsxF1gd-jbKNvmpm... to cm88aTLui5yorSGYQ... (flutter/flutter#178074)
2025-11-05 [email protected] Fix(ios): Remove arm64 exclusion to support Xcode 26 simulators (flutter/flutter#177065)
2025-11-05 [email protected] Roll Skia from 2ff897e9b440 to 4eb2383d38f2 (18 revisions) (flutter/flutter#178070)
2025-11-05 [email protected] [web] Unify Surface code between Skwasm and CanvasKit (flutter/flutter#177138)
2025-11-05 [email protected] Update more missing ninja deps (flutter/flutter#178079)
2025-11-05 [email protected] Add ninja / cmake deps to failing tests (flutter/flutter#178054)
2025-11-05 [email protected] [web] Don't add webparagraph suite to CI (flutter/flutter#177681)
2025-11-05 [email protected] Fixing broken link in engine readme (flutter/flutter#177987)
2025-11-05 [email protected] Print reason for adb command failure in verified input test (attempt 3) (flutter/flutter#178005)
2025-11-05 [email protected] Fix `ReorderableList` items jumping when drag direction reverses mid-animation (flutter/flutter#173241)
2025-11-05 [email protected] Replace deprecated `withOpacity` in `overflow_bar.0.dart‎` example (flutter/flutter#177813)
2025-11-05 [email protected] Replace deprecated `withOpacity` in `data_table.1.dart‎` example (flutter/flutter#177812)
2025-11-05 [email protected] Replace deprecated `withOpacity` in `switch.1.dart` example (flutter/flutter#177811)
2025-11-04 [email protected] Roll Skia from c89b6118266b to 2ff897e9b440 (6 revisions) (flutter/flutter#177999)
2025-11-04 [email protected] Fix verified input test in CI (attempt 2) (flutter/flutter#177961)
2025-11-04 [email protected] Replace rendering for solid color circles (both filled and stroked) to use SDFs (flutter/flutter#177482)
2025-11-04 [email protected] Roll Fuchsia Linux SDK from vxK5obzfr1X9P2kSh... to mpsxF1gd-jbKNvmpm... (flutter/flutter#177996)
2025-11-04 [email protected] Validate that platforms specified in .ci.yaml target names match the platforms defined in the platform_properties section (flutter/flutter#177523)
2025-11-04 [email protected] Roll Packages from 1a7075b to 3d926aa (6 revisions) (flutter/flutter#177998)
2025-11-04 [email protected] Remove dead code from snippet_generator (flutter/flutter#174440)

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] 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
@mdebbar mdebbar deleted the autogen_webparagraph branch November 11, 2025 18:16
IvoneDjaja pushed a commit to IvoneDjaja/flutter that referenced this pull request Nov 22, 2025
Fix `generate-builder-json` to only include test bundles that are needed
by enabled test suites.

## Before this PR
The script was unconditionally including all test bundles in CI. The
result is that the `dart2js-canvaskit-experimental-webparagraph` bundle
was being generated, even though it was only required by the
`chrome-dart2js-experimental-webparagraph-ui` suite, which had
`enable-ci: false`.

## After this PR
The script starts by finding all test suites with `enable-ci: true`,
then only adds the bundles required by those suites.
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
Fix `generate-builder-json` to only include test bundles that are needed
by enabled test suites.

## Before this PR
The script was unconditionally including all test bundles in CI. The
result is that the `dart2js-canvaskit-experimental-webparagraph` bundle
was being generated, even though it was only required by the
`chrome-dart2js-experimental-webparagraph-ui` suite, which had
`enable-ci: false`.

## After this PR
The script starts by finding all test suites with `enable-ci: true`,
then only adds the bundles required by those suites.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine flutter/engine related. See also e: labels. platform-web Web applications specifically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants