-
Notifications
You must be signed in to change notification settings - Fork 28
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
Fixes #2361. Fix flaky tests on web-platforms #2362
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sgrekhov for the quick fixes! One small comment below.
asyncEnd(); | ||
if (!fired) { | ||
asyncEnd(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe you also need to include fired = true
here?
// timers fire even 15ms early. We add a safety margin to prevent flakiness | ||
// when running this test on affected platforms. | ||
Duration safetyMargin = const bool.fromEnvironment('dart.library.js') | ||
? Duration(milliseconds: 40) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that you may want to also adjust the intervals below for additional coverage. When periodMs
is 0, 1, 10, and 30, we are no longer validating much about the duration, only that the event was fired.
The case with 100 does validate that at least 60ms have passed. I'm personally OK with that, but if you want additional coverage, we could include another check at 200.
@sigmundch thak you for good advices! They are implemented in #2364 |
2023-11-10 sgrekhov22@gmail.com dart-lang/co19#2361. Improve some flaky tests on Web (dart-lang/co19#2364) 2023-11-10 sgrekhov22@gmail.com dart-lang/co19#2350. Add more factory constructor tests. Part 3 (dart-lang/co19#2363) 2023-11-09 sgrekhov22@gmail.com dart-lang/co19#2349. Add more tests for constant constructors of the extension types (dart-lang/co19#2352) 2023-11-09 sgrekhov22@gmail.com Fixes dart-lang/co19#2361. Fix flaky tests on web-platforms (dart-lang/co19#2362) 2023-11-08 sgrekhov22@gmail.com dart-lang/co19#2359. Update tests according to the new nullability rules. Part 1 (dart-lang/co19#2360) 2023-11-07 sgrekhov22@gmail.com Fixes dart-lang/co19#2356. Fix return type of `this` (dart-lang/co19#2358) 2023-11-07 sgrekhov22@gmail.com dart-lang/co19#2350. Update existing factory constructor tests. Part 2 (dart-lang/co19#2357) 2023-11-03 sgrekhov22@gmail.com Fixes dart-lang/co19#2339. Add tests that extension can't have name `type` (dart-lang/co19#2340) 2023-11-03 sgrekhov22@gmail.com dart-lang/co19#2346. Add expected error in analyzer (dart-lang/co19#2347) Change-Id: I6c0e1e9612dc4a0e21c4d506fa682ae1e9b98ea9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/335461 Reviewed-by: Erik Ernst <eernst@google.com> Reviewed-by: Alexander Thomas <athom@google.com> Commit-Queue: Alexander Thomas <athom@google.com>
No description provided.