Skip to content

[CP] Fix crash from invalid ListWheelViewport assertion (#126539)#126804

Merged
XilaiZhang merged 2 commits into
flutter:flutter-3.10-candidate.1from
nt4f04uNd:cp/126491/flutter-3.10-candidate.1
May 15, 2023
Merged

[CP] Fix crash from invalid ListWheelViewport assertion (#126539)#126804
XilaiZhang merged 2 commits into
flutter:flutter-3.10-candidate.1from
nt4f04uNd:cp/126491/flutter-3.10-candidate.1

Conversation

@nt4f04uNd

@nt4f04uNd nt4f04uNd commented May 15, 2023

Copy link
Copy Markdown
Member

Fixes #126782

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 Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • 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.

@flutter-dashboard

Copy link
Copy Markdown

This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter.

Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed.

@flutter-dashboard flutter-dashboard Bot added f: cupertino flutter/packages/flutter/cupertino repository f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. labels May 15, 2023
@nt4f04uNd

Copy link
Copy Markdown
Member Author

Failed test output

04:11 +5630 ~1: /b/s/w/ir/x/w/flutter/packages/flutter/test/cupertino/picker_test.dart: Registers taps and does not crash with certain diameterRatio                                                   
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: [0, 1, 0, 1]
  Actual: [0, 0, 1, 1]
   Which: at location [1] is <0> instead of <1>

When the exception was thrown, this was the stack:
#4      main.<anonymous closure> (file:///b/s/w/ir/x/w/flutter/packages/flutter/test/cupertino/picker_test.dart:575:5)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
  file:///b/s/w/ir/x/w/flutter/packages/flutter/test/cupertino/picker_test.dart line 575
The test description was:
  Registers taps and does not crash with certain diameterRatio
════════════════════════════════════════════════════════════════════════════════════════════════════

Apparently, there's something different about how CupertinoPicker renders its children between this stable branch and master branch.

I corrected the test to match this behaviour here.

@nt4f04uNd nt4f04uNd requested a review from Piinks May 15, 2023 07:30

@Piinks Piinks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CP LGTM

@XilaiZhang XilaiZhang merged commit 9fa53ae into flutter:flutter-3.10-candidate.1 May 15, 2023
@nt4f04uNd nt4f04uNd deleted the cp/126491/flutter-3.10-candidate.1 branch May 15, 2023 23:15
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

f: cupertino flutter/packages/flutter/cupertino repository f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants