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

FlutterWindowsEngineTest.TestExit is very flaky #124162

Closed
jonahwilliams opened this issue Apr 4, 2023 · 4 comments
Closed

FlutterWindowsEngineTest.TestExit is very flaky #124162

jonahwilliams opened this issue Apr 4, 2023 · 4 comments
Assignees
@yaakovschectman
Labels
a: desktop Running on desktop c: flake Tests that sometimes, but not always, incorrectly pass engine flutter/engine repository. See also e: labels. P1 High-priority issues at the top of the work list

Comments

@jonahwilliams
Copy link
Member

It appears that most of the host unopt windows builders are having to retry to rerun this single test. I'm not sure if we have any automatic flakiness detection for these, but filling so we don't lose track of it.

For example: https://luci-milo.appspot.com/ui/p/flutter/builders/try/Windows%20Engine%20Drone/64316/overview

When these tests flake we have to rerun the entire shard, which may be constributing to windows capacity issues.

@yaakovschectman @cbracken
@jonahwilliams jonahwilliams added engine flutter/engine repository. See also e: labels. team: flakes a: desktop Running on desktop P1 High-priority issues at the top of the work list labels Apr 4, 2023
@loic-sharma
Copy link
Member

loic-sharma commented Apr 4, 2023 •
edited
Loading

This test was added yesterday by flutter/engine#40802. @yaakovschectman could you investigate? My guess is the test shuts down the engine while the engine's initialization is still happening in the background:

[ERROR:flutter/shell/platform/embedder/embedder_surface_gl.cc(121)] Could not create a resource context for async texture uploads. Expect degraded performance. Set a valid make_resource_current callback on FlutterOpenGLRendererConfig.
[ERROR:flutter/shell/gpu/gpu_surface_gl_skia.cc(43)] Could not make the context current to set up the Gr context.
[ERROR:flutter/shell/gpu/gpu_surface_gl_skia.cc(81)] Could not make the context current to set up the Gr context.
[ERROR:flutter/shell/common/platform_view.cc(75)] Failed to create platform view rendering surface

GMOCK WARNING:
Uninteresting mock function call - taking default action specified at:
../../flutter/shell/platform/windows/flutter_windows_engine_unittests.cc(654):
    Function call: IsLastWindowOfProcess()
          Returns: true
NOTE: You can safely ignore the above warning unless this call should not happen.  Do not suppress it by blindly adding an EXPECT_CALL() if you don't mean to enforce the call.  See https://github.com/google/googletest/blob/master/docs/gmock_cook_book.md#knowing-when-to-expect for details.
../../flutter/shell/platform/windows/flutter_windows_engine_unittests.cc(679): error: Value of: did_call
  Actual: true
Expected: false

@yaakovschectman
Copy link
Contributor

Is TestExit the only test flaking? This PR added several tests, and if all other tests are okay, their functionality will be able to make up for this one test in the meantime.

@jonahwilliams
Copy link
Member Author

Yeah

@yaakovschectman yaakovschectman mentioned this issue Apr 5, 2023
Merged
8 tasks
yaakovschectman added a commit to flutter/engine that referenced this issue Apr 7, 2023
@yaakovschectman
Fix flaky Windows exit unit test, remove error messages (#40945)
1f5433d 
Shutdown is async, so we can not guarantee the order of execution and,
by extension, ignorance of execution used as an auxiliary part of a unit
test. Also, mock the windows view/binding handler for the engine in
order to avoid error messages.

Addresses flutter/flutter#124162

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or Hixie said the PR is test-exempt. See [testing the engine]
for instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [ ] I signed the [CLA].
- [x] All existing and new tests are passing.

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
zhongwuzw pushed a commit to zhongwuzw/engine that referenced this issue Apr 14, 2023
@yaakovschectman @zhongwuzw
Fix flaky Windows exit unit test, remove error messages (flutter#40945)
cbcd79f 
Shutdown is async, so we can not guarantee the order of execution and,
by extension, ignorance of execution used as an auxiliary part of a unit
test. Also, mock the windows view/binding handler for the engine in
order to avoid error messages.

Addresses flutter/flutter#124162

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or Hixie said the PR is test-exempt. See [testing the engine]
for instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [ ] I signed the [CLA].
- [x] All existing and new tests are passing.

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
@flutter-triage-bot flutter-triage-bot bot added the c: flake Tests that sometimes, but not always, incorrectly pass label Jul 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@yaakovschectman yaakovschectman

Labels
a: desktop Running on desktop c: flake Tests that sometimes, but not always, incorrectly pass engine flutter/engine repository. See also e: labels. P1 High-priority issues at the top of the work list
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@loic-sharma @jonahwilliams @yaakovschectman