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.

Sign up for GitHub

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

Jump to bottom

Browserstack disconnection issue. #3038

Merged
merged 3 commits into from
Apr 6, 2020
Merged

Browserstack disconnection issue. #3038

merged 3 commits into from
Apr 6, 2020

Conversation

lina128
Copy link
Collaborator

@lina128 lina128 commented Apr 4, 2020 •
edited
Loading

BrowserStack support recommended the following configs. After upgrade to 1.5.0, we are seeing a lot of timeouts, see issue karma-runner/karma-browserstack-launcher#152. If changing config doesn't help, we will have to downgrade to 1.4.0.

captureTimeout: 3e5,
browserNoActivityTimeout: 3e5,
browserDisconnectTimeout: 3e5,
browserDisconnectTolerance: 3

This change is Reviewable

@lina128 lina128 requested a review from pyu10055 April 4, 2020 23:37
pyu10055
pyu10055 reviewed Apr 5, 2020
View reviewed changes
tfjs-core/karma.conf.js
@@ -108,9 +108,11 @@ module.exports = function(config) {
username: process.env.BROWSERSTACK_USERNAME,
accessKey: process.env.BROWSERSTACK_KEY
},
captureTimeout: 120000,
captureTimeout: 3e5,
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is this number translate to? will the large number cause the test to stuck and occupy the browserstack instance?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

300000 ms = 5 min

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can change back if this doesn't help, and it's not significantly larger than what we currently have, i.e. 2 min.

Copy link
Collaborator

@pyu10055 pyu10055 Apr 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

got it. maybe write it as 30000 for readability. the number seems to be arbitrary, any idea why this number would work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I put it in PR description, this set of number is recommended by Browserstack support.

pyu10055
pyu10055 approved these changes Apr 6, 2020
View reviewed changes
Copy link
Collaborator

@pyu10055 pyu10055 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r1.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @kangyizhang, @pyu10055, and @tafsiri)

@lina128 lina128 merged commit dc29a28 into tensorflow:master Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pyu10055 pyu10055 pyu10055 approved these changes

@tafsiri tafsiri Awaiting requested review from tafsiri

@kangyizhang kangyizhang Awaiting requested review from kangyizhang

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lina128 @pyu10055 @googlebot