Cherry pick bug 915866 to m-c

RESOLVED FIXED in mozilla27

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ahal, Assigned: ahal)

Tracking

unspecified
mozilla27
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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).
Attachment #810576 - Flags: review?(jhammel)

Comment 1

5 years ago
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)
Attachment #810576 - Flags: review?(jhammel) → review+
(Assignee)

Comment 2

5 years ago
(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.
https://hg.mozilla.org/mozilla-central/rev/1262ce233fc8
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
(Assignee)

Updated

5 years ago
Blocks: 917252
You need to log in before you can comment on or make changes to this bug.