921029 - Cherry pick bug 915866 to m-c

Status: RESOLVED FIXED

Description

[Andrew Halberstadt [:ahal]]

Created attachment 810576 [details] [diff] [review]
Patch 1.0 - sync mozrunner_return_code

Bug 915866 is needed in m-c for debugging of several intermittent timeouts, and the sheriffs would like it asap. It looks like the broader mozbase->m-c mirror in bug 917750 has stalled, so I'd like to get this particular patch landed sooner if possible (if it is close to landing, then feel free to ignore this).

Comment 1

[Jeff Hammel]

Comment on attachment 810576 [details] [diff] [review]
Patch 1.0 - sync mozrunner_return_code

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

r+; i'm not 100% sure why this is needed in m-c, since, while inconvenient, the status code is available given effort.  That said, there's no harm in updating.

> It looks like the broader mozbase->m-c mirror in bug 917750 has
> stalled, so I'd like to get this particular patch landed sooner if
> possible (if it is close to landing, then feel free to ignore
> this).

stalled is one way of putting it; there are a number of issues that must be dealt with prior to that I did not anticipate (and add to that that it does not directly fulfill any immediate goals).

::: testing/mozbase/mozrunner/mozrunner/runner.py
@@ +68,5 @@
>          Use is_running() to determine whether or not a timeout occured.
>          Timeout is ignored if interactive was set to True.
>          """
>          if self.process_handler is None:
>              return

Do we want to return None here?  (just makin' sure)

Comment 2

[Andrew Halberstadt [:ahal]]

(In reply to Jeff Hammel [:jhammel] from comment #1)
> Comment on attachment 810576 [details] [diff] [review]
> Patch 1.0 - sync mozrunner_return_code
> 
> Review of attachment 810576 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+; i'm not 100% sure why this is needed in m-c, since, while inconvenient,
> the status code is available given effort.  That said, there's no harm in
> updating.

You're right. It's more just that I already made a patch that depends on it and got it reviewd. So between making a new patch with a new review and cherry picking this, the latter is the path of least resistance.

> ::: testing/mozbase/mozrunner/mozrunner/runner.py
> @@ +68,5 @@
> >          Use is_running() to determine whether or not a timeout occured.
> >          Timeout is ignored if interactive was set to True.
> >          """
> >          if self.process_handler is None:
> >              return
> 
> Do we want to return None here?  (just makin' sure)

That's a good point. It'll get confusing as None could also mean a timeout. Though this is something I should have caught before it even made it's way into m-c. I'll file a new bug to address it.

Comment 3

[Andrew Halberstadt [:ahal]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1262ce233fc8

Comment 4

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/1262ce233fc8