Closed Bug 1065381 Opened 9 years ago Closed 9 years ago

Add result_callbacks to include test debug in HTML reports for failures and errors


(Remote Protocol :: Marionette, defect)

Not set


(Not tracked)



(Reporter: davehunt, Assigned: davehunt)




(2 files, 2 obsolete files)

The HTML formatter in mozlog supports extra data such as screenshots to be included. As I understand it we would need to add result callbacks in the test harness for this to start appearing in the HTML.

To be able to replace the HTML report generated by Marionette's test runner, we would need the following extra data:

* Screenshot
* Settings
* Source

See for examples of gathering this data.
James: Would you be able to provide additional information of these results callbacks, perhaps with an example?
Flags: needinfo?(james)
Blocks: 1065410
So you basically need to pass a result_callbacks argument when the StructuredTestRunner class is created. These must return a dictionary of extra data to be included in the result message. So in this case it would presumably return something like {"screenshot": "base64 encoded screenshot as string", "source": "source"}, although note that "source" is not currently present in the HTML report.
Flags: needinfo?(james)
So, looking at this a bit more, you probably need to arrange for BaseMarionetteTestRunner to take a results_callback argument that gets passed to textrunnerclass. This would default to the empty list (via None ofc). Then, when you initialise the BaseMarionetteTestRunner, you would have something like:

def gather_debug(test, status):
    rv = {}
    marionette = test._marionette_weakref()
    rv['screenshot'] = marionette.screenshot()
    rv['source'] = marionette.page_source

    rv['settings'] = json.dumps(self.marionette.execute_async_script("""
SpecialPowers.addPermission('settings-read', true, document);
SpecialPowers.addPermission('settings-api-read', true, document);
var req = window.navigator.mozSettings.createLock().get('*');
req.onsuccess = function() {
}""", special_powers=True), sort_keys=True, indent=4, separators=(',', ': '))
    return rv

result_callbacks = [gather_debug]
Assignee: nobody → dave.hunt
Attachment #8509513 - Flags: review?(james)
Comment on attachment 8509513 [details] [diff] [review]
Gather basic debug for HTML log formatter. r=jgraham

Review of attachment 8509513 [details] [diff] [review]:

Just a couple of small fixups.

::: testing/marionette/client/marionette/runner/
@@ +483,5 @@
>          self.gecko_log = gecko_log
>          self.mixin_run_tests = []
>          self.manifest_skipped_tests = []
>          self.tests = []
> +        self.result_callbacks = result_callbacks or []

I tend to prefer the more explicit 'result_callbacks if result_callbacks is not None else []'

@@ +491,5 @@
> +            marionette = test._marionette_weakref()
> +            try:
> +                marionette.set_context(marionette.CONTEXT_CHROME)
> +                rv['screenshot'] = marionette.screenshot()
> +                marionette.set_context(marionette.CONTEXT_CONTENT)

I guess this is safe if we are confident that we're always in CONTENT to begin with. I don't think you can do better at least.

@@ +796,5 @@
>                  break
>          if suite.countTestCases():
> +            runner = self.textrunnerclass(
> +                logger=self.logger,

Revert to the old alignment, please.
Attachment #8509513 - Flags: review?(james) → review+
Applied suggested changes. Carrying r+
Attachment #8509513 - Attachment is obsolete: true
Attachment #8510440 - Flags: review+
Hi Dave,

seems this patch didn't apply cleanly:

applying bug1065381.patch
patching file testing/marionette/client/marionette/runner/
Hunk #2 FAILED at 214
Hunk #3 FAILED at 437
Hunk #5 FAILED at 790
3 out of 5 hunks FAILED -- saving rejects to file testing/marionette/client/marionette/runner/
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh bug1065381.patch

could you take a look. Thanks!
Flags: needinfo?(dave.hunt)
Keywords: checkin-needed
This was caused by bug 1084565 landing first. I've updated the patch and carried the r+ after testing due to the minimal changes.
Attachment #8510440 - Attachment is obsolete: true
Flags: needinfo?(dave.hunt)
Attachment #8511945 - Flags: review+
Comment on attachment 8509516 [details] [review]
Gather Gaia debug for HTML log formatter. v1.0

Thanks for the review, James. I've updated based on your comments - could you take another look?
Comment on attachment 8509516 [details] [review]
Gather Gaia debug for HTML log formatter. v1.0

Applying r+ from github.
Attachment #8509516 - Flags: review?(james) → review+
(In reply to Dave Hunt (:davehunt) from comment #12)
> Comment on attachment 8509516 [details] [review]
> Gather Gaia debug for HTML log formatter. v1.0
> Applying r+ from github.

Landed in:
Keywords: leave-open
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.