Skip to content

Flutter integration test timeouts with https://dart-review.googlesource.com/c/sdk/+/149613 #42350

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
Closed
gw280 opened this issue Jun 15, 2020 · 6 comments
Closed

Flutter integration test timeouts with https://dart-review.googlesource.com/c/sdk/+/149613 #42350

gw280 opened this issue Jun 15, 2020 · 6 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. customer-flutter type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@gw280
Copy link

gw280 commented Jun 15, 2020

The Flutter Engine roll into Flutter was failing due to timeouts on our integration tests, and this patch was found to be the root cause:

https://dart-review.googlesource.com/c/sdk/+/149613
b0484ec

Failures: https://github.com/flutter/flutter/pull/59380/checks?check_run_id=767354642

cc @rmacnak-google
@zanderso zanderso added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. customer-flutter type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jun 15, 2020
@zanderso
Copy link
Member

/cc @a-siva as well.

@zanderso
Copy link
Member

The dart -> engine roller is currently blocked on this.

@alexmarkov
Copy link
Contributor

Revert: https://dart-review.googlesource.com/c/sdk/+/151169

dart-bot pushed a commit that referenced this issue Jun 15, 2020
@alexmarkov
Revert "[vm] Check prefix.loadLibrary is called and returns before pr…
9a87cf9 
…efix members are used."

This reverts commit b0484ec.

Reason for revert: timeouts on Flutter integration tests
(#42350).

Original change's description:
> [vm] Check prefix.loadLibrary is called and returns before prefix members are used.
> 
> Restore checks against reloading a library with deferred prefixes.
> 
> No loading is actually deferred.
> 
> Bug: #26878
> Bug: #41974
> Change-Id: Iec2662de117453d596cca28dd9481a9751091ce9
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149613
> Commit-Queue: Ryan Macnak <[email protected]>
> Reviewed-by: Alexander Markov <[email protected]>
> Reviewed-by: Siva Annamalai <[email protected]>

[email protected],[email protected],[email protected]

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: #26878, #41974
Change-Id: I78709650e91d206b84a8ddd9171ef66d6cf1b008
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151169
Reviewed-by: Alexander Markov <[email protected]>
Commit-Queue: Alexander Markov <[email protected]>
@jonahwilliams
Copy link
Contributor

jonahwilliams commented Jun 15, 2020 •
edited
Loading

We should probably just fix the test right? This is intended to test the code generation for the l10n tool, which includes the ability to generate deferred for web applications.

@jonahwilliams jonahwilliams mentioned this issue Jun 15, 2020
Closed
@rmacnak-google
Copy link
Contributor

Yeah, after some digging I found some generated code includes

  if (kIsWeb) {                                                                                                                                                                          
    return loadLibrary().then((dynamic _) => localizationClosure());                                                                                                                     
  } else {                                                                                                                                                                               
    return SynchronousFuture<AppLocalizations>(localizationClosure());                                                                                                                   
  }

which is bogus for the non-web case.

@jonahwilliams
Copy link
Contributor

Fixed upstream in flutter with flutter/flutter#59539

dart-bot pushed a commit that referenced this issue Jun 16, 2020
@rmacnak-google
Reland "[vm] Check prefix.loadLibrary is called and returns before pr…
62893f9 
…efix members are used."

This reverts commit 9a87cf9.

Reason for revert: Broken test disabled

Original change's description:
> Revert "[vm] Check prefix.loadLibrary is called and returns before prefix members are used."
> 
> This reverts commit b0484ec.
> 
> Reason for revert: timeouts on Flutter integration tests
> (#42350).
> 
> Original change's description:
> > [vm] Check prefix.loadLibrary is called and returns before prefix members are used.
> > 
> > Restore checks against reloading a library with deferred prefixes.
> > 
> > No loading is actually deferred.
> > 
> > Bug: #26878
> > Bug: #41974
> > Change-Id: Iec2662de117453d596cca28dd9481a9751091ce9
> > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149613
> > Commit-Queue: Ryan Macnak <[email protected]>
> > Reviewed-by: Alexander Markov <[email protected]>
> > Reviewed-by: Siva Annamalai <[email protected]>
> 
> [email protected],[email protected],[email protected]
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug: #26878, #41974
> Change-Id: I78709650e91d206b84a8ddd9171ef66d6cf1b008
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151169
> Reviewed-by: Alexander Markov <[email protected]>
> Commit-Queue: Alexander Markov <[email protected]>

[email protected],[email protected],[email protected]

# Not skipping CQ checks because this is a reland.

Bug: #26878, #41974
Change-Id: Ife76bd51db65ca58e08655a9b8406c8ca483447f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151326
Reviewed-by: Ryan Macnak <[email protected]>
Reviewed-by: Alexander Markov <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. customer-flutter type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@gw280 @zanderso @rmacnak-google @jonahwilliams @alexmarkov