Skip to content

Conversation

@gaaclarke
Copy link
Member

@gaaclarke gaaclarke commented Sep 5, 2024

This is a forward fix for the failures seen in https://github.com/flutter/engine/pull/54714/checks?check_run_id=29742599916 on flutter/engine#54714

issue: #127855

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Sep 5, 2024
@gaaclarke gaaclarke marked this pull request as ready for review September 5, 2024 20:17
final Paint paint = call.positionalArguments[3] as Paint;
expect(paint.colorFilter, colorFilter);
expect(paint.color, const Color(0x7F000000)); // 0.5 opacity
expect(paint.color, _matchesColor(const Color(0x7F000000))); // 0.5 opacity
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the only failure? if so, it looks like all its doing is testing for 0.5 opacity. You could query that directly

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the only one that showed up 🤞 The next PR is going to be the one that's probably going to break a lot of stuff when I update things like lerp. We may need to put this color matcher somewhere else where it can be shared for that.

You mean something like expect(paint.color.a, closeTo(0.5))? It seems likely that was the intent of the author. I'd rather stick to what is actually asserted.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure what you mean?

Copy link
Member Author

Choose a reason for hiding this comment

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

it looks like all its doing is testing for 0.5 opacity.

I'm saying that may have been their intention but it is actually asserting a color so it's safer to just keep it asserting the same thing that is written. If the goal is to avoid having to have a color matcher, we'll likely need one anyway to land flutter/engine#54981

Copy link
Contributor

Choose a reason for hiding this comment

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

kk

Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 5, 2024
@auto-submit auto-submit bot merged commit 9e99675 into flutter:master Sep 5, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 6, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Sep 6, 2024
flutter/flutter@45ef8f3...2e221e7

2024-09-06 [email protected] Fix DropdownMenu focused item styles (flutter/flutter#153159)
2024-09-06 [email protected] Support custom transition duration for `DialogRoute`, `CupertinoDialogRoute` and show dialog methods. (flutter/flutter#154048)
2024-09-06 [email protected] [tool] Add `dartFileName` setting for platform plugins  (flutter/flutter#153099)
2024-09-06 [email protected] [Conductor] Add ability to override mirror, add tests for default arg parsing and custom arg parsing (flutter/flutter#154363)
2024-09-06 [email protected] Improve CupertinoPopupSurface appearance (flutter/flutter#151430)
2024-09-06 [email protected] Roll Packages from 71e827e to 56df73e (1 revision) (flutter/flutter#154725)
2024-09-06 [email protected] Quick access to style guide (flutter/flutter#154689)
2024-09-06 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from c50eb8a65097 to 015f3b1dec53 (2 revisions) (#154691)" (flutter/flutter#154726)
2024-09-05 [email protected] Improve iOS unpack target's error messages (flutter/flutter#154649)
2024-09-05 [email protected] Made some pixel tests fuzzy (flutter/flutter#154680)
2024-09-05 [email protected] Roll Flutter Engine from c50eb8a65097 to 015f3b1dec53 (2 revisions) (flutter/flutter#154691)
2024-09-05 49699333+dependabot[bot]@users.noreply.github.com Bump peter-evans/create-pull-request from 7.0.0 to 7.0.1 (flutter/flutter#154690)
2024-09-05 [email protected] Normalize Dialog theme (flutter/flutter#153982)
2024-09-05 [email protected] iOS,macOS: Do not copy unsigned_binaries.txt to build outputs (flutter/flutter#154684)
2024-09-05 [email protected] Roll Flutter Engine from e042ff5df7af to c50eb8a65097 (1 revision) (flutter/flutter#154679)
2024-09-05 [email protected] Add proguard rule to keep the class for all implementations of FlutterPlugin (flutter/flutter#154677)
2024-09-05 [email protected] Fix DropdownMenu menu does not follow the text field (flutter/flutter#154667)
2024-09-05 [email protected] Roll Flutter Engine from a156e713f4dc to e042ff5df7af (1 revision) (flutter/flutter#154678)
2024-09-05 [email protected] Fix ZoomPageTransitionsBuilder hardcoded fill color (flutter/flutter#154057)
2024-09-05 [email protected] Roll Flutter Engine from 34b61eb53b99 to a156e713f4dc (1 revision) (flutter/flutter#154672)

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],[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
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants