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

Fix flakiness in FlutterVSyncWaiterTest.VSyncWorks and FlutterDisplayLinkTest.WorkaroundForFB13482573 #51405

Merged
merged 2 commits into from
Mar 14, 2024

Conversation

knopp
Copy link
Member

@knopp knopp commented Mar 14, 2024

  • Removes potential source of flakiness where the tests assumes that scheduled block will be performed within certain time period, which, despite the tolerances may not be the case on test runner.

  • Ensures that viewDidChangeWindow notification is not received while invalidating the displayLink and removing the display link view, which could deadlock at the end of WorkaroundForFB13482573 test because the notification would come while FlutterDisplayLink is in @synchronized block.

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

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 and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

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

@knopp knopp changed the title Fix FlutterVSyncWaiterTest.VSyncWorks flakyness Fix FlutterVSyncWaiterTest.VSyncWorks flakiness Mar 14, 2024
@knopp knopp requested a review from cbracken March 14, 2024 05:53
@knopp knopp changed the title Fix FlutterVSyncWaiterTest.VSyncWorks flakiness Fix FlutterVSyncWaiterTest.VSyncWorks and FlutterDisplayLinkTest.WorkaroundForFB13482573 flakiness Mar 14, 2024
@knopp knopp requested a review from loic-sharma March 14, 2024 06:03
@knopp knopp changed the title Fix FlutterVSyncWaiterTest.VSyncWorks and FlutterDisplayLinkTest.WorkaroundForFB13482573 flakiness Fix flakiness in FlutterVSyncWaiterTest.VSyncWorks and FlutterDisplayLinkTest.WorkaroundForFB13482573 Mar 14, 2024
Copy link
Member

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

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

Thanks for following-up so quickly!

@knopp knopp merged commit 8f49b4a into flutter:main Mar 14, 2024
22 checks passed
@knopp knopp deleted the fix_flaky_test branch March 14, 2024 21:00
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 15, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Mar 15, 2024
…145191)

flutter/engine@622b372...995d890

2024-03-14 matanlurey@users.noreply.github.com Add some header-goodies for et. (flutter/engine#51434)
2024-03-14 jonahwilliams@google.com [Impeller] revert glyph atlas texture recycling. (flutter/engine#51428)
2024-03-14 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add DisplayList Region and Transform benchmarks to CI (#51429)" (flutter/engine#51432)
2024-03-14 flar@google.com Add DisplayList Region and Transform benchmarks to CI (flutter/engine#51429)
2024-03-14 skia-flutter-autoroll@skia.org Roll Dart SDK from 2bc8b222d01f to 70ca2323a702 (1 revision) (flutter/engine#51430)
2024-03-14 matanlurey@users.noreply.github.com Reduce code-duplication a bit and add more error context across `SkiaGoldClient`. (flutter/engine#51426)
2024-03-14 zanderso@users.noreply.github.com [et] build and run commands disable RBE with a flag or when not available (flutter/engine#51404)
2024-03-14 matej.knopp@gmail.com Fix flakiness in FlutterVSyncWaiterTest.VSyncWorks and FlutterDisplayLinkTest.WorkaroundForFB13482573 (flutter/engine#51405)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC bdero@google.com,rmistry@google.com,zra@google.com 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://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
Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

Thanks for landing this -- apologies, out on holiday at the moment!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants