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

[flutter_tool] Timeout the Azure bot detector http request #53217

Merged
merged 1 commit into from Mar 25, 2020
Merged

[flutter_tool] Timeout the Azure bot detector http request #53217

merged 1 commit into from Mar 25, 2020

Conversation

zanderso
Copy link
Member

Description

When not on Azure, depending on the environment, the connection setup might not timeout, but the http request will. This PR adds a timeout for this case.

Related Issues

#53098

Tests

I added the following tests:

Added test to bot_detector_test.dart

Breaking Change

Did any tests fail when you ran them? Please read [Handling breaking changes].

  • No, no existing tests failed, so this is not a breaking change.

@zanderso zanderso added tool Affects the "flutter" command-line tool. See also t: labels. work in progress; do not review labels Mar 25, 2020
return completer.future; // Never completed to test timeout behavior.
});

unawaited(azureDetector.isRunningOnAzure.then((bool onBot) {
Copy link
Member

Choose a reason for hiding this comment

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

The structure of this test is a little odd, because it seems like this expectation isn't guaranteed to be hit before the test body finishes. I would rearrange it a bit, maybe something like this?

FakeAsync().run((FakeAsync time) async {
  ...
  final Future<bool> isRunningOnAzure = azureDetector.isRunningOnAzure;
  time.elapse(const Duration(seconds: 2));

  expect(await isRunningOnAzure, true);
});

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

@fluttergithubbot fluttergithubbot merged commit 52e4011 into flutter:master Mar 25, 2020
@zanderso zanderso deleted the azure-detector-timeout branch March 30, 2020 16:37
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 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.

None yet

4 participants