Open Bug 960436 Opened 6 years ago Updated 2 years ago

add a register_exception_handler function to base.py

Categories

(Testing :: Marionette, defect, P3)

x86
All
defect

Tracking

(Not tracked)

People

(Reporter: mdas, Assigned: mdas)

References

Details

(Keywords: pi-marionette-runner, Whiteboard: [runner])

Attachments

(1 file)

In this try block: http://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/runner/base.py#260, we want to be able to handle certain exceptions. This is useful for test harnesses that want to let some exceptions occur, or do special handling if a particular exception occurs. This is immediately useful for MTBF

I'd like to add an exception handler registry, that will add this flow, and would be called like: register_exception_handler(IOError, my_callback)
Duplicate of this bug: 940916
I know I'm assigned to this, but I'm working on another high priority bug right now, so if you or wachen are interested, you're more than welcome to submit a patch :) If not, I'll get back on this bug as soon as possible.
sure, I'll have a look for this.
Thanks! If you're not done by the new year holiday, I can work on it during your break.
Give an initial patch and see if anything concern.
Attachment #8372044 - Flags: review?(mdas)
Comment on attachment 8372044 [details] [diff] [review]
initial patch for exception handler

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

This patch doesn't allow you to register a handler via add_exception_handler. We want to be able to add multiple handlers if needed...

Also, I'm no longer sure if this is the right spot for exception handling. Can you give me an example of what kind of exceptions you'd like to catch here? I was testing out some socket.error type exceptions, but they don't propagate up to the this point. Depending on your use case, we may want to add exception handling somewhere else.

::: testing/marionette/client/marionette/runner/base.py
@@ +21,5 @@
>  from marionette import Marionette
>  from moztest.results import TestResultCollection, TestResult, relevant_line
>  
>  
> +class MarionetteException(Exception):

MarionetteException is already a defined class in the client. Can this be renamed to TestRunnerException?

@@ +268,5 @@
>              startTestRun()
>          try:
>              test(result)
> +        except self.exceptionclass as e:
> +            e.handler(this)

what is 'this'?
Attachment #8372044 - Flags: review?(mdas) → review-
Test runner needs to handle exception and decide it should continue, exit, or skip.
For example, stakeholder needs to stop when 4 consequent timeout occur, and not to catch any other error.
(In reply to Paul Yang [: pyang] from comment #7)
> Test runner needs to handle exception and decide it should continue, exit,
> or skip.
> For example, stakeholder needs to stop when 4 consequent timeout occur, and
> not to catch any other error.

okay, then this is not the right area for this handler since the timeout exceptions don't propagate here. We'll have to investigate to find the right area for this, I don't know at this moment. You can suggest one if you can find a good place, since I have to work on yet another high priority project until MWC, so about 2 weeks. After that, I'll be able to focus on this again.
Sure, thanks for help.
I wanted to give you an update: Sorry for the delays, I and others who work on marionette are still working on a "drop everything and work on this" high priority project (the certification tests) so this bug is *still* not being worked on yet, and we don't have a clear timeline of when I can focus back to maraionette. I want to get on this as soon as I can dedicate time to marionette.

Is this a blocker for MTBF? If not, are there any bugs that are blockers for MTBF?
Currently MTBF works smoothly and this won't be urgent as we expected.
I'm stuck with other tasks as well so we can come back for this later.
Whiteboard: [runner]
[mass update] Setting Harness bugs to all P3
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.