Skip to content

[flutter_tool] Prevent accidental calls to io.exit in unit tests#46639

Merged
jonahwilliams merged 13 commits intoflutter:masterfrom
jonahwilliams:never_again_
Dec 10, 2019
Merged

[flutter_tool] Prevent accidental calls to io.exit in unit tests#46639
jonahwilliams merged 13 commits intoflutter:masterfrom
jonahwilliams:never_again_

Conversation

@jonahwilliams
Copy link
Copy Markdown
Contributor

Description

To prevent a scenario such as #46142 for occurring again in the future, prevent calls to dart:io's exit when asserts are enabled within a unit test. This is detected by invoking expect and checking if a StateError is thrown

@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Dec 9, 2019
/// Throws [StateError] if assertions are enabled and the dart:io exit
/// is still active when called. This may indicate exit was called in
/// a test without being configured correctly. This behavior can be
/// removed by setting the `FLUTTER_TEST` environment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like the comment needs to be updated.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

ExitFunction get exit {
assert(() {
if (_exitFunction == io.exit) {
try {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider pulling the expect() check into a helper method, and simplifying here as in the last PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Copy Markdown
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

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

lgtm

ExitFunction get exit {
assert(
_exitFunction != io.exit || !_inUnitTest(),
'io.exit was called with assertions active in a unit test'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: Does assert allow a comma after the message string?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Lets find out...

@jonahwilliams jonahwilliams merged commit d0526d3 into flutter:master Dec 10, 2019
@jonahwilliams jonahwilliams deleted the never_again_ branch December 10, 2019 02:58
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants