-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Description
There's a problem with this code:
flutter/packages/flutter_tools/bin/xcode_backend.dart
Lines 40 to 44 in 5483fef
| if (arguments.isEmpty) { | |
| // Named entry points were introduced in Flutter v0.0.7. | |
| echoXcodeError(incompatibleErrorMessage); | |
| exit(-1); | |
| } |
Rather than calling exit(-1) directly, it should instead call the wrapper exitApp(-1):
flutter/packages/flutter_tools/bin/xcode_backend.dart
Lines 171 to 173 in 5483fef
| Never exitApp(int code) { | |
| exit(code); | |
| } |
The subclass TestContext overwrites exitApp to throw an exception:
flutter/packages/flutter_tools/test/general.shard/xcode_backend_test.dart
Lines 1092 to 1096 in 5483fef
| Never exitApp(int code) { | |
| // This is an exception for the benefit of unit tests. | |
| // The real implementation calls `exit(code)`. | |
| throw Exception('App exited with code $code'); | |
| } |
So calling exit(-1) directly would terminate the test process directly, rather than throwing an exception.
Solution
short term
Replace exit with exitApp
Long term
This is another drawback of the current design using inheritance (see discussion #173133). It's too easy to introduce bugs like this if we keep this design.
Instead, we should inject an abstraction layer, say "Host":
// excuse my poor dart syntax, but you get the idea
abstract class Host {
Never exit(int code);
// other members like stdout/stderr
}
class RealHost: Host {
Never exit(int code) {
dart.io.exit(code);
}
}
class FakeHost: Host {
Never exit(int code) {
throw Exception...
}
}
Then xcode_backend.dart should just depend on the Host abstraction layer, without importing dart.io package.