Closed Bug 1284502 Opened 9 years ago Closed 9 years ago

Marionette harness logs exceptions twice in cli() and run()

Categories

(Remote Protocol :: Marionette, defect)

49 Branch
defect
Not set
normal

Tracking

(firefox50 fixed)

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

Details

Attachments

(1 file)

As noticed today we do an extra logging for exceptions happening inside of `run()` within the cli. This doesn't seem to be necessary: https://dxr.mozilla.org/mozilla-central/source/testing/marionette/harness/marionette/runtests.py#72 https://dxr.mozilla.org/mozilla-central/source/testing/marionette/harness/marionette/runtests.py#95 As the `cli()` logging message says it should only log exceptions for harness setup but not the execution of tests. Maja, maybe this came in with your refactoring a couple of months ago?
Flags: needinfo?(mjzffr)
Actually the extra logging came in via bug 1227918.
Blocks: 1227918
Keywords: regression
The logging added via bug 1227918 is necessary, as explained in that bug's summary -- in particular, if there is any issue with the `harness_class` constructor. If you want to change how logging is done with respect to test execution versus harness setup, I suppose you could remove the logging message at line 72, since the exception is re-raised anyway, and generalize the message at line 95.
Flags: needinfo?(mjzffr)
Keywords: regression
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Whenever an exception is raised while tests are executed, the log error message should only be printed once. As best this should happen in `cli()`, so that subclasses can better set their own behavior, and we safe us from re-raising the exception. Review commit: https://reviewboard.mozilla.org/r/66882/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/66882/
Attachment #8774404 - Flags: review?(mjzffr)
https://reviewboard.mozilla.org/r/66882/#review63632 ::: testing/marionette/harness/marionette/runtests.py:86 (Diff revision 1) > """ > logger = mozlog.commandline.setup_logging('Marionette test runner', {}) > try: > - failed = harness_class(runner_class, parser_class, testcase_class, args=args).run() > + harness_instance = harness_class(runner_class, parser_class, testcase_class, > + args=args) > + failed = harness_instance.run() I split this line up to make it easier to spot if the exception actually happened during setup or test execution.
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5c6a56bb4a6c Marionette harness should log exceptions from run() only once. r=maja_zf
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: