Closed Bug 1488820 Opened 2 years ago Closed 2 years ago

Check for crashes if no connection can be established in raise_for_port()

Categories

(Testing :: Marionette, enhancement, P1)

Version 3
enhancement

Tracking

(firefox64 fixed)

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

Details

Attachments

(1 file)

When we wait for a connection but the process no longer exists we have to check for crashes. Otherwise we won't notice any startup crash of the application:

https://dxr.mozilla.org/mozilla-central/rev/c2e3be6a1dd352b969a45f0b85e87674e24ad284/testing/marionette/client/marionette_driver/marionette.py#711

This is most likely the reason why we see that many 120s timeout failures on Treeherder.
Comment on attachment 9006685 [details] [diff] [review]
[marionette] Check for crashes if no connection can be established in raise_for_port()

Review of attachment 9006685 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/marionette/client/marionette_driver/marionette.py
@@ +723,5 @@
>  
>          if not connected:
> +            # There might have been a startup crash of the application
> +            if runner is not None and self.check_for_crash() > 0:
> +                raise IOError('Process crashed (Exit code: {})'.format(runner.wait(0)))

This is probably fine, but since I’m completely unfamiliar with
this code, I have one concern.

It’s unclear to me from mozrunner’s documentation [1] whether passing
runner.wait(0) will cause the process to be killed immediately
because the timeout duration will always be elapsed?

  [1] https://searchfox.org/mozilla-central/rev/a41fd8cb947266ea2e3f463fc6e31c88bfab9d41/testing/mozbase/mozrunner/mozrunner/base/runner.py#144
Attachment #9006685 - Flags: review?(ato) → review+
(In reply to Andreas Tolfsen ❲:ato❳ from comment #3)
> It’s unclear to me from mozrunner’s documentation [1] whether passing
> runner.wait(0) will cause the process to be killed immediately
> because the timeout duration will always be elapsed?

A call to `wait()` never kills the process. In mozrunner we have the capability to specify a timeout, which means that if the application is still running once it elapsed, the call returns the current process exit code. Note that we cannot use `poll()` or `returncode` here due to bug 1433905. I hope that we can have this fixed soon.
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dce5fe50d17a
[marionette] Check for crashes if no connection can be established in raise_for_port().
https://hg.mozilla.org/mozilla-central/rev/dce5fe50d17a
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.