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

sliverGridDelegate mainAxisExtent add assert #148470

Merged
merged 3 commits into from
May 29, 2024
Merged

sliverGridDelegate mainAxisExtent add assert #148470

merged 3 commits into from
May 29, 2024
+53 −2

Conversation

hello-coder-xu
Copy link
Contributor

@hello-coder-xu hello-coder-xu commented May 16, 2024 •
edited by Piinks

  • sliverGridDelegate mainAxisExtent add assert,Avoid incorrect use *

Fixes #138871

Pre-launch Checklist

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

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels May 16, 2024
@goderbauer goderbauer requested a review from Piinks May 17, 2024 21:05
Piinks
Piinks reviewed May 21, 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.

Hi @hello-coder-xu welcome! Is there an open issue this PR is resolving?

@hello-coder-xu
Copy link
Contributor Author

Hi @hello-coder-xu welcome! Is there an open issue this PR is resolving?

Fixed #138871, I forgot to deal with my previous PR

Piinks
Piinks reviewed May 22, 2024
View reviewed changes
packages/flutter/test/widgets/grid_view_test.dart Outdated
mainAxisExtent: -100,
),
)),
throwsA(isA<AssertionError>()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you verify these throw the assertion error that you are expecting, rather than the one from the issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the assertion content to compare

@hello-coder-xu
Merge branch 'master' into feature/grid_delegate_add_assert
c644da0 
* master: (92 commits)
  Add tests for actions.0.dart API example. (flutter#148678)
  Introduce `WidgetStateBorderSide.lerp` (flutter#148122)
  add `default-flavor` field to flutter pubspec, which will be used as the flavor in `flutter build/run` if `--flavor` is not provided (flutter#147968)
  [wiki migration] Pages under docs/postmortems/ (flutter#148798)
  Roll Flutter Engine from e5a73e520e89 to c89defa55801 (2 revisions) (flutter#148812)
  Make hover tests functional and cleanup mouse pointers in Material toggleables (flutter#148808)
  Fix two dimensional viewport unexpected null exception when no child is laid out (flutter#148256)
  Roll Flutter Engine from bc1345b6b50a to e5a73e520e89 (3 revisions) (flutter#148807)
  Add test for undo_history_controller.0.dart (flutter#148205)
  Roll Flutter Engine from a8872c8915a2 to bc1345b6b50a (6 revisions) (flutter#148802)
  Fix test that leaks images. (flutter#148494)
  Fix warnings in `dependency_version_checker.gradle.kts` (flutter#148699)
  [wiki migration] Android team pages (flutter#148585)
  Fix leaky test. (flutter#148788)
  Add DropdownButton.menuWidth (flutter#148125)
  Add test for focus example 2 (flutter#147624)
  Add a migrator to remove `FlutterMultiDexApplication.java` (flutter#148515)
  [wiki migration] Infra team pages (flutter#148718)
  Roll Flutter Engine from 8a352f01e503 to a8872c8915a2 (1 revision) (flutter#148776)
  Fix the output of the CDN test. (flutter#148730)
  ...
@hello-coder-xu
Update assertion content comparison
147a384
Piinks
Piinks approved these changes May 28, 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.

This LGTM, thanks for the update!
I will need to find another reviewer, please hold. :)

QuncCccccc
QuncCccccc approved these changes May 28, 2024
View reviewed changes
Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for your contribution:)

@Piinks Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label May 28, 2024
@auto-submit auto-submit bot merged commit af5a1a9 into flutter:master May 29, 2024
73 checks passed
@eyebrowsoffire
Copy link
Contributor

The added unit test is failing in postsubmit in the web tests and closing the tree. I'm not sure why it didn't fail on presubmit in this PR, but this should be reverted until the issue is resolved. (See https://ci.chromium.org/ui/p/flutter/builders/prod/Linux%20web_canvaskit_tests_7_last/14617/overview for an example failure)

@eyebrowsoffire
Copy link
Contributor

Reason for revert: Unit test failures closing the tree.

@eyebrowsoffire eyebrowsoffire added the revert Autorevert PR (with "Reason for revert:" comment) label May 29, 2024
auto-submit bot pushed a commit that referenced this pull request May 29, 2024
Revert "sliverGridDelegate mainAxisExtent add assert (#148470)"
f9c542e 
This reverts commit af5a1a9.
@auto-submit auto-submit bot mentioned this pull request May 29, 2024
Merged
9 tasks
@auto-submit auto-submit bot removed the revert Autorevert PR (with "Reason for revert:" comment) label May 29, 2024
auto-submit bot added a commit that referenced this pull request May 29, 2024
@auto-submit
Reverts "sliverGridDelegate mainAxisExtent add assert (#148470)" (#14…
886de22 
…9224)

Reverts: #148470
Initiated by: eyebrowsoffire
Reason for reverting: Unit test failures closing the tree.
Original PR Author: hello-coder-xu

Reviewed By: {QuncCccccc, Piinks}

This change reverts the following previous change:
* sliverGridDelegate mainAxisExtent add assert,Avoid incorrect use *

Fixes #138871
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 29, 2024
@engine-flutter-autoroll
af5a1a9e1 sliverGridDelegate mainAxisExtent add assert (flutter/flutt…
3c45120 
…er#148470)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 29, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 29, 2024
@engine-flutter-autoroll
af5a1a9e1 sliverGridDelegate mainAxisExtent add assert (flutter/flutt…
cf4e193 
…er#148470)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request May 29, 2024
@engine-flutter-autoroll
Roll Flutter from a1a33e63b911 to c85fa6abf4ff (20 revisions) (#6829)
82a811f 
flutter/flutter@a1a33e6...c85fa6a

2024-05-29 polinach@google.com Clean leak in editable_text_test.dart. (flutter/flutter#149223)
2024-05-29 sokolovskyi.konstantin@gmail.com Add tests for animated_switcher.0.dart API example. (flutter/flutter#149180)
2024-05-29 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from d0323905fc2f to b26e1b023cdb (16 revisions) (flutter/flutter#149220)
2024-05-29 hany212mohamed@gmail.com Change snack bar default hitTestBehavior to deferToChild when SnackBarThemeData.insetPadding is not null (flutter/flutter#148568)
2024-05-29 98614782+auto-submit[bot]@users.noreply.github.com Reverts "sliverGridDelegate mainAxisExtent add assert (#148470)" (flutter/flutter#149224)
2024-05-29 yinxulolol@gmail.com sliverGridDelegate mainAxisExtent add assert (flutter/flutter#148470)
2024-05-29 luis901101@gmail.com Fix `SearchAnchor` suggestions not refreshing after long API call (flutter/flutter#148767)
2024-05-28 737941+loic-sharma@users.noreply.github.com Add link to golden file test docs in the framework gardener guide (flutter/flutter#149207)
2024-05-28 polinach@google.com Remove opt out for CurvedAnimation. (flutter/flutter#147594)
2024-05-28 31859944+LongCatIsLooong@users.noreply.github.com Fix the RenderFlex.computeDryBaseline implementation to match computeDistanceToActualBaseline (flutter/flutter#149062)
2024-05-28 polinach@google.com Clean leaky test. (flutter/flutter#149199)
2024-05-28 34871572+gmackall@users.noreply.github.com Change `android_plugin_new_output_dir_test.dart` test description (flutter/flutter#149198)
2024-05-28 leroux_bruno@yahoo.fr fix M2 InputDecorator suffix icon doesn't turn red on error (flutter/flutter#149161)
2024-05-28 77919688+varunkamanibosc@users.noreply.github.com Add selectionOverlayBuilder in CupertinoDatePicker and CupertinoTimer� (flutter/flutter#143079)
2024-05-28 jmccandless@google.com Mouse onEnter and onExit now support hovering stylus (flutter/flutter#149006)
2024-05-28 31859944+LongCatIsLooong@users.noreply.github.com Remove `TextEditingController` private member access (flutter/flutter#149042)
2024-05-28 engine-flutter-autoroll@skia.org Roll Packages from b7bcb4b to a933c30 (1 revision) (flutter/flutter#149184)
2024-05-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from b1751088c7e9 to d0323905fc2f (2 revisions) (flutter/flutter#149169)
2024-05-28 kevmoo@users.noreply.github.com [tool] Use kebabCase directly (flutter/flutter#149150)
2024-05-28 katelovett@google.com [wiki migration] Leftover wiki pages and links (flutter/flutter#148989)

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 camillesimon@google.com,rmistry@google.com,stuartmorgan@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
@hello-coder-xu hello-coder-xu mentioned this pull request May 30, 2024
Open
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request May 31, 2024
@hello-coder-xu @victorsanni
sliverGridDelegate mainAxisExtent add assert (flutter#148470)
49079c3 
* sliverGridDelegate mainAxisExtent add assert,Avoid incorrect use *

Fixes flutter#138871
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request May 31, 2024
@auto-submit @victorsanni
Reverts "sliverGridDelegate mainAxisExtent add assert (flutter#148470)…
bb480a9 
…" (flutter#149224)

Reverts: flutter#148470
Initiated by: eyebrowsoffire
Reason for reverting: Unit test failures closing the tree.
Original PR Author: hello-coder-xu

Reviewed By: {QuncCccccc, Piinks}

This change reverts the following previous change:
* sliverGridDelegate mainAxisExtent add assert,Avoid incorrect use *

Fixes flutter#138871
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

@QuncCccccc QuncCccccc QuncCccccc approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Milestone
No milestone
4 participants
@hello-coder-xu @eyebrowsoffire @Piinks @QuncCccccc