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.

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

[rfw, ci] Regenerate goldens, manually roll #4835 #4862

Merged
merged 8 commits into from
Sep 7, 2023

Conversation

ditman
Copy link
Member

@ditman ditman commented Sep 6, 2023

This PR updates the goldens for package:rfw.

This PR also:

  • Modifies run_tests.sh so it forwards all its command-line parameters to the inner test_coverage.dart bin. (This enables passing --update-goldens without much fuss.)
  • Updates the documentation on how to update goldens in the package.

Issues

This PR fixes #4835, and should enable the auto-roller to resume.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

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

@ditman ditman requested a review from stuartmorgan September 6, 2023 21:52
@ditman ditman requested a review from Hixie as a code owner September 6, 2023 21:52
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie or stuartmorgan on the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

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.

@github-actions github-actions bot added the p: rfw Remote Flutter Widgets label Sep 6, 2023
@ditman
Copy link
Member Author

ditman commented Sep 6, 2023

This PR should be test-exempt. It actually fixes tests so flutter/flutter may resume rolling into flutter/packages.

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM

packages/rfw/README.md Outdated Show resolved Hide resolved
@stuartmorgan
Copy link
Contributor

test-exempt: is a test

@ditman ditman added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 6, 2023
@ditman
Copy link
Member Author

ditman commented Sep 6, 2023

(PS, I've just noticed the png diffing in github, it's pretty handy!)

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 6, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Sep 6, 2023

auto label is removed for flutter/packages/4862, due to - The status or check suite Linux repo_checks has failed. Please fix the issues identified (or deflake) before re-applying this label.

@ditman

This comment was marked as resolved.

@ditman ditman added override: no versioning needed Override the check requiring version bumps for most changes and removed override: no versioning needed Override the check requiring version bumps for most changes labels Sep 6, 2023
@ditman

This comment was marked as resolved.

@stuartmorgan
Copy link
Contributor

@stuartmorgan do I still need an override: no versioning needed after adding a NEXT entry in the changelog?

For future reference, yes. If we didn't have the explicit check, people would use NEXT all the time via cargo-culting and we'd forget to check that it was actually right because CI wouldn't warn us. With the label, it's impossible to not think about it.

@ditman
Copy link
Member Author

ditman commented Sep 7, 2023

The last check has passed here: https://ci.chromium.org/ui/p/flutter/builders/try/Linux_android%20android_platform_tests_shard_2%20master/1663/overview

But the GH status hasn't updated. I'm tempted to manually merge this one (otherwise I'll push an empty commit and let CI try again)

@stuartmorgan
Copy link
Contributor

It's fine to merge manually in cases like this where we can see that the tests did in fact pass and it's just a report-back-to-GitHub problem.

@ditman
Copy link
Member Author

ditman commented Sep 7, 2023

Will land manually to unblock the roll, and keep an eye on the CI on main.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Sep 8, 2023
flutter/packages@22d4754...aaae5ef

2023-09-08 [email protected] [tool] Add Android dependency (gradle) option to update dependencies command (flutter/packages#4757)
2023-09-08 [email protected] [camerax] Implement resolution configuration (flutter/packages#3799)
2023-09-07 [email protected] Manual roll Flutter from 685ce14 to aea4552 (64 revisions) (flutter/packages#4870)
2023-09-07 [email protected] [rfw, ci] Regenerate goldens, manually roll #4835 (flutter/packages#4862)
2023-09-07 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 3.6.0 to 4.0.0 (flutter/packages#4845)
2023-09-07 [email protected] [video_player] Add optional web options [web] (flutter/packages#4551)
2023-09-07 [email protected] [flutter_markdown] Remove `ignore: avoid_init_to_null` since the package uses Dart 3 (flutter/packages#4852)

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-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
sybrands-place pushed a commit to sybrands-place/packages that referenced this pull request Sep 14, 2023
* main: (83 commits)
  go_router_builder: support the latest pkg:analyzer (flutter#4921)
  Replace collection type lints with more general lint (flutter#4912)
  Roll Flutter from 219efce to 4e7a07a (30 revisions) (flutter#4910)
  [camerax] Implement `startVideoCapturing` and `onVideoRecordedEvent` (flutter#4815)
  [tool] Add a package inclusion filter (flutter#4904)
  [flutter_markdown] Fix changelog regarding minimum supported SDK version (flutter#4851)
  [ios_platform_images] Add integration tests (flutter#4899)
  [image_picker] Copy exif tags in categories II and III (flutter#4738)
  [google_maps_flutter_android] Fix for testToggleInfoWindow persistently flaky  (flutter#4768)
  Roll Flutter from 7c28e8e to 219efce (16 revisions) (flutter#4901)
  [url_launcher] migrating iOS tests from objc to swift (flutter#4758)
  Roll Flutter from da676f7 to 7c28e8e (20 revisions) (flutter#4879)
  Bump actions/upload-artifact from 3.1.2 to 3.1.3 (flutter#4863)
  Roll Flutter from aea4552 to da676f7 (28 revisions) (flutter#4874)
  [webview_flutter_android] Added the functionality to fullscreen html5 video (flutter#3879)
  [tool] Add Android dependency (gradle) option to update dependencies command (flutter#4757)
  [camerax] Implement resolution configuration (flutter#3799)
  Manual roll Flutter from 685ce14 to aea4552 (64 revisions) (flutter#4870)
  [rfw, ci] Regenerate goldens, manually roll flutter#4835 (flutter#4862)
  Bump actions/checkout from 3.6.0 to 4.0.0 (flutter#4845)
  ...
@reidbaker reidbaker mentioned this pull request Sep 15, 2023
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p: rfw Remote Flutter Widgets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants