Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Factor out use of "print" in flutter_goldens #144846

Merged
merged 2 commits into from
Mar 17, 2024
Merged

Factor out use of "print" in flutter_goldens #144846

merged 2 commits into from
Mar 17, 2024
+96 −35

Conversation

Hixie
Copy link
Contributor

@Hixie Hixie commented Mar 8, 2024

This is part 5 of a broken down version of the #140101 refactor.

This PR removes direct use of print and replaces it with a callback.

@github-actions github-actions bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Mar 8, 2024
@Hixie Hixie force-pushed the golden_refactor_journey_5 branch from d50a4ab to 5a4c704 Compare March 12, 2024 20:48
@goderbauer goderbauer requested a review from Piinks March 12, 2024 22:04
Piinks
Piinks reviewed Mar 13, 2024
View reviewed changes
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

Love this improvement 💯

dev/automated_tests/flutter_test/flutter_gold_expectation.txt
@@ -1,7 +1 @@
[0-9]+:[0-9]+ [+]0: Local passes non-existent baseline for new test, null expectation *
Copy link
Contributor

Choose a reason for hiding this comment

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

How will folks be notified of this case now?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh wait, I see now. 🙂
This is really neat.

dev/automated_tests/flutter_test/flutter_gold_test.dart Outdated
expect(
await comparator.compare(
Uint8List.fromList(_kFailPngBytes),
Uri.parse('flutter.new_golden_test.1.png'),
),
isTrue,
);
expect(log, <String>[
// ignore: no_adjacent_strings_in_list
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this just be written to avoid needing an ignore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pulled the string into a constant and made the list just contain the constant.

dev/automated_tests/flutter_test/flutter_gold_test.dart Outdated
expect(
await comparator.compare(
Uint8List.fromList(_kFailPngBytes),
Uri.parse('flutter.new_golden_test.2.png'),
),
isTrue,
);
expect(log, <String>[
// ignore: no_adjacent_strings_in_list
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@Hixie
Factor out use of "print" in flutter_goldens
dee657a 
This is part 5 of a broken down version of the flutter#140101 refactor.

This PR removes direct use of `print` and replaces it with a callback.
@Hixie Hixie force-pushed the golden_refactor_journey_5 branch from 5a4c704 to 4a29dc4 Compare March 13, 2024 22:00
@Hixie Hixie force-pushed the golden_refactor_journey_5 branch from 4a29dc4 to 3c12004 Compare March 13, 2024 22:00
Piinks
Piinks approved these changes Mar 14, 2024
View reviewed changes
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

LGTM!

@Hixie Hixie added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 17, 2024
@auto-submit auto-submit bot merged commit c7123c9 into flutter:master Mar 17, 2024
133 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 17, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 17, 2024
@engine-flutter-autoroll
c7123c96c Factor out use of "print" in flutter_goldens (flutter/flutt…
c195fd2 
…er#144846)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Mar 17, 2024
@engine-flutter-autoroll
Roll Flutter from 71606af8d82e to 7c860ddf9705 (21 revisions) (#6345)
a757073 
flutter/flutter@71606af...7c860dd

2024-03-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from cfbf11d757d1 to 4c420dd2dfd9 (2 revisions) (flutter/flutter#145280)
2024-03-17 ian@hixie.ch Factor out use of "print" in flutter_goldens (flutter/flutter#144846)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from dd4050334ff3 to cfbf11d757d1 (1 revision) (flutter/flutter#145275)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 618ee6c8ba34 to dd4050334ff3 (1 revision) (flutter/flutter#145270)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from e4602aa149d8 to 618ee6c8ba34 (1 revision) (flutter/flutter#145266)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 91b9cf24d328 to e4602aa149d8 (1 revision) (flutter/flutter#145259)
2024-03-16 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland #128236 "Improve build output for all platforms" (#143166)" (flutter/flutter#145261)
2024-03-16 6655696+guidezpl@users.noreply.github.com Reland #128236 "Improve build output for all platforms" (flutter/flutter#143166)
2024-03-16 6655696+guidezpl@users.noreply.github.com Introduce `Split` curve (flutter/flutter#143130)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 120e52b8a1c2 to 91b9cf24d328 (1 revision) (flutter/flutter#145256)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3abc25139959 to 120e52b8a1c2 (1 revision) (flutter/flutter#145254)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6f180615e6ac to 3abc25139959 (1 revision) (flutter/flutter#145253)
2024-03-16 magder@google.com Platform view devicelab ad banner scroll list real ads (flutter/flutter#145239)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6ddaa40bf1a6 to 6f180615e6ac (1 revision) (flutter/flutter#145251)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from d46c1fa17dda to 6ddaa40bf1a6 (1 revision) (flutter/flutter#145250)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7f15dc1200bc to d46c1fa17dda (2 revisions) (flutter/flutter#145248)
2024-03-16 31859944+LongCatIsLooong@users.noreply.github.com Use a separate `TextPainter` for intrinsics calculation in `RenderEditable` and `RenderParagraph` (flutter/flutter#144577)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2db915c64e6e to 7f15dc1200bc (1 revision) (flutter/flutter#145245)
2024-03-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from ba1115c30381 to 2db915c64e6e (2 revisions) (flutter/flutter#145240)
2024-03-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from c2fd5333a5d2 to ba1115c30381 (3 revisions) (flutter/flutter#145237)
2024-03-15 91688203+yusuf-goog@users.noreply.github.com Renaming the virtual device Nexus5 name to Nexus5.gce_x86 (flutter/flutter#145225)

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 rmistry@google.com,stuartmorgan@google.com,tarrinneal@google.com 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 May 14, 2024
@engine-flutter-autoroll
c7123c96c Factor out use of "print" in flutter_goldens (flutter/flutt…
47ce931 
…er#144846)
arc-yong pushed a commit to Arctuition/packages-arc that referenced this pull request Jun 14, 2024
@engine-flutter-autoroll @arc-yong
Roll Flutter from 71606af8d82e to 7c860ddf9705 (21 revisions) (flutte…
7eb57dd 
…r#6345)

flutter/flutter@71606af...7c860dd

2024-03-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from cfbf11d757d1 to 4c420dd2dfd9 (2 revisions) (flutter/flutter#145280)
2024-03-17 ian@hixie.ch Factor out use of "print" in flutter_goldens (flutter/flutter#144846)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from dd4050334ff3 to cfbf11d757d1 (1 revision) (flutter/flutter#145275)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 618ee6c8ba34 to dd4050334ff3 (1 revision) (flutter/flutter#145270)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from e4602aa149d8 to 618ee6c8ba34 (1 revision) (flutter/flutter#145266)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 91b9cf24d328 to e4602aa149d8 (1 revision) (flutter/flutter#145259)
2024-03-16 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland #128236 "Improve build output for all platforms" (#143166)" (flutter/flutter#145261)
2024-03-16 6655696+guidezpl@users.noreply.github.com Reland #128236 "Improve build output for all platforms" (flutter/flutter#143166)
2024-03-16 6655696+guidezpl@users.noreply.github.com Introduce `Split` curve (flutter/flutter#143130)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 120e52b8a1c2 to 91b9cf24d328 (1 revision) (flutter/flutter#145256)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3abc25139959 to 120e52b8a1c2 (1 revision) (flutter/flutter#145254)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6f180615e6ac to 3abc25139959 (1 revision) (flutter/flutter#145253)
2024-03-16 magder@google.com Platform view devicelab ad banner scroll list real ads (flutter/flutter#145239)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6ddaa40bf1a6 to 6f180615e6ac (1 revision) (flutter/flutter#145251)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from d46c1fa17dda to 6ddaa40bf1a6 (1 revision) (flutter/flutter#145250)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7f15dc1200bc to d46c1fa17dda (2 revisions) (flutter/flutter#145248)
2024-03-16 31859944+LongCatIsLooong@users.noreply.github.com Use a separate `TextPainter` for intrinsics calculation in `RenderEditable` and `RenderParagraph` (flutter/flutter#144577)
2024-03-16 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2db915c64e6e to 7f15dc1200bc (1 revision) (flutter/flutter#145245)
2024-03-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from ba1115c30381 to 2db915c64e6e (2 revisions) (flutter/flutter#145240)
2024-03-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from c2fd5333a5d2 to ba1115c30381 (3 revisions) (flutter/flutter#145237)
2024-03-15 91688203+yusuf-goog@users.noreply.github.com Renaming the virtual device Nexus5 name to Nexus5.gce_x86 (flutter/flutter#145225)

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 rmistry@google.com,stuartmorgan@google.com,tarrinneal@google.com 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Piinks Piinks Piinks approved these changes

Assignees
No one assigned
Labels
a: tests "flutter test", flutter_test, or one of our tests autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Hixie @Piinks