Centralize defaults for socket and startup timeouts

RESOLVED FIXED in Firefox 57

Status

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

57 Branch
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(1 attachment)

Currently we define the defaults for startup_timeout and socket_timeout at two different places. Which means that in this case startup_timeout has different values. While Marionette driver proposes 120s, the harness only uses 60s.

That means that for slow running builds like debug and ASAN we make use of a wrong and too short value. As result Marionette client itself kills the process because it cannot connect to the too late started server component of Marionette.

With this bug we will unify the defaults to the same value.

This might actually help us for bug 1261598 and bug 1362293.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment on attachment 8905495 [details]
Bug 1397734 - Centralize defaults for socket and startup timeouts

https://reviewboard.mozilla.org/r/177310/#review182576

r+wc

::: commit-message-3ecda:1
(Diff revision 2)
> +Bug 1397734 - Centralize defaults for socket and startup timeouts

I think you might also have to update the Mn harness tests (maybe just the mach_parsed_kwargs fixture)
Attachment #8905495 - Flags: review?(mjzffr) → review+
Assignee

Comment 4

2 years ago
mozreview-review-reply
Comment on attachment 8905495 [details]
Bug 1397734 - Centralize defaults for socket and startup timeouts

https://reviewboard.mozilla.org/r/177310/#review182576

> I think you might also have to update the Mn harness tests (maybe just the mach_parsed_kwargs fixture)

No, the code changes here do not affect the harness tests, so I didn't have to update those. Those still run fine. The only thing I wanted to change is the float/int combination of both, but defered that for a later time given that it might have larger implications for invoking the harness.

Comment 5

2 years ago
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bc05e6c94a17
Centralize defaults for socket and startup timeouts r=maja_zf
Backed out for mass mochitest failures (passed str to timedelta):

https://hg.mozilla.org/integration/autoland/rev/95430ac5c638f16cb20bca4abcffd02f9092b81c

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=bc05e6c94a1725d5561810e7a8d78b6f2484e60c&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=129445749&repo=autoland

[task 2017-09-08T06:27:21.198674Z] 06:27:21     INFO - Application command: /builds/worker/workspace/build/application/firefox/firefox -marionette -foreground -profile /tmp/tmp2Hbax9.mozrunner
[task 2017-09-08T06:27:21.339709Z] 06:27:21     INFO - runtests.py | Application pid: 1096
[task 2017-09-08T06:27:21.342206Z] 06:27:21     INFO - TEST-INFO | started process GECKO(1096)
[task 2017-09-08T06:27:21.349900Z] 06:27:21     INFO -  Traceback (most recent call last):
[task 2017-09-08T06:27:21.358855Z] 06:27:21     INFO -    File "/builds/worker/workspace/build/tests/mochitest/runtests.py", line 2635, in doTests
[task 2017-09-08T06:27:21.370135Z] 06:27:21     INFO -      marionette_args=marionette_args,
[task 2017-09-08T06:27:21.379457Z] 06:27:21     INFO -    File "/builds/worker/workspace/build/tests/mochitest/runtests.py", line 2165, in runApp
[task 2017-09-08T06:27:21.396029Z] 06:27:21     INFO -      self.marionette.start_session(timeout=port_timeout)
[task 2017-09-08T06:27:21.400619Z] 06:27:21     INFO -    File "/builds/worker/workspace/build/venv/local/lib/python2.7/site-packages/marionette_driver/decorators.py", line 23, in _
[task 2017-09-08T06:27:21.418200Z] 06:27:21     INFO -      return func(*args, **kwargs)
[task 2017-09-08T06:27:21.425515Z] 06:27:21     INFO -    File "/builds/worker/workspace/build/venv/local/lib/python2.7/site-packages/marionette_driver/marionette.py", line 1207, in start_session
[task 2017-09-08T06:27:21.436279Z] 06:27:21     INFO -      self.wait_for_port(timeout=timeout)
[task 2017-09-08T06:27:21.448270Z] 06:27:21     INFO -    File "/builds/worker/workspace/build/venv/local/lib/python2.7/site-packages/marionette_driver/marionette.py", line 674, in wait_for_port
[task 2017-09-08T06:27:21.460669Z] 06:27:21     INFO -      while datetime.datetime.now() - starttime < datetime.timedelta(seconds=timeout):
[task 2017-09-08T06:27:21.474742Z] 06:27:21     INFO -  TypeError: unsupported type for timedelta seconds component: str
Flags: needinfo?(hskupin)
Hurray for type checks in Python! As it looks like some test harnesses inappropriately use a string value instead of float/int to specify the timeout. As such we fail here.

Given that we don't know how many consumers this affects, I would simply put back the conversion to int for the arguments of the Marionette class as what we had before.
Flags: needinfo?(hskupin)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 11

2 years ago
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/629ecdd561c4
Centralize defaults for socket and startup timeouts r=maja_zf

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/629ecdd561c4
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.