-
Notifications
You must be signed in to change notification settings - Fork 6k
[web] running ios unit tests from felt … #18856
Conversation
…reate simulator with felt
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
yjbanov
left a comment
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.
Do we need all the command-line options? Can we support using a custom simulator instance by picking whatever is currently running? Then, all you need to do to choose a different simulator is Cmd + Space, type "simulator" and select the one you want.
lib/web_ui/dev/utils.dart
Outdated
| cleanupCallbacks.forEach((element) { | ||
| element.call(); | ||
| cleanupCallbacks.forEach((element) async { | ||
| await element.call(); |
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.
What's the effect of this change?
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.
When I didn't run in like this, I wasn't able to see the INFO, I print out:
"print('INFO: Simulator ${iosSimulator.id} shutdown.');"
The previous implementation was missing the await. Let me know if I'm missing something. I'm open to suggestions on change.
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.
Hmm, strange. This shouldn't have any effect because cleanupCallbacks.forEach is synchronous anyway; it ignores async and await. However, the following would have an effect:
for (AsyncCallback cleanupCallback in cleanupCallbacks) {
await cleanupCallback();
}
This is because the for loop is aware of await and it would not pick up the next element until the previous callback's future is resolved.
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.
This worked when I also combined it with adding a await to the call inside test_runner.dart
thanks for the suggestions.
I think, I should have initially used Future.forEach instead: https://api.dart.dev/stable/2.8.4/dart-async/Future/forEach.html
…nstead of foreach
|
Merging the PR since it is related to Flutter Web tooling. No relation to the failing code paths. |
running ios unit tests from felt by booting/shutingdown simulators. create simulator with felt.
updating the readme.
Enabled felt commands:
Contributes to flutter/flutter#53690