Closed Bug 921029 Opened 8 years ago Closed 8 years ago

Cherry pick bug 915866 to m-c

Categories

(Testing :: Mozbase, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla27

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(1 file)

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 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+
(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
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Blocks: 917252
You need to log in before you can comment on or make changes to this bug.