Closed Bug 1059381 Opened 7 years ago Closed 7 years ago

SimpleTest.requestCompleteLog does not work as expected for mochitest-browser

Categories

(Testing :: Mochitest, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla35

People

(Reporter: chmanchester, Assigned: chmanchester)

Details

Attachments

(1 file, 1 obsolete file)

Trying to debug bug 1041594, SimpleTest.requestCompleteLog() was found not to work as expected. I'm not sure if this is a regression from mochitest structured logging, or if this was ever supported from mochitest-browser, but it needs to work.
:akachkach, were you looking into this one? How's it going?
Flags: needinfo?(ahmed.kachkach)
I was looking at something related (passing the logger to SimpleTest to make all test logging from there), but that doesn't seem to be easily doable.

I wonder if requestCompleteLog worked before because some of the logging was done in Tester and other mochitest-browser classes, but in any case we have to fix this.

Did you have a temporary patch for this? (I remember jmaher mentionned that he started getting complete log results with something
Flags: needinfo?(ahmed.kachkach)
I think jmaher took --quiet out of the in tree config to work around this. I guess I'll look into this next week if you don't beat me to it :)
I only too --quiet out of the in tree stuff in my local patch queue
There was a warning in the code about name collisions, so here's an extensive try run:

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=542dd2df16e1
Assignee: nobody → cmanchester
Status: NEW → ASSIGNED
Comment on attachment 8483006 [details] [diff] [review]
Implement SimpleTest.requestCompleteLog for mochitest browser

Ok, try is green.
Attachment #8483006 - Flags: review?(jmaher)
Comment on attachment 8483006 [details] [diff] [review]
Implement SimpleTest.requestCompleteLog for mochitest browser

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

r=me with one nit addressed and one question answered.

::: testing/mochitest/browser-test.js
@@ +25,5 @@
>    "resource:///modules/ContentSearch.jsm");
>  
>  const SIMPLETEST_OVERRIDES =
> +  ["ok", "is", "isnot", "ise", "todo", "todo_is", "todo_isnot", "info", "expectAssertions",
> +   "requestCompleteLog"];

there is no need for a newline here, lets just keep it as two lines.

@@ +915,5 @@
> +    self.registerCleanupFunction(function() {
> +      self.__tester.dumper.structuredLogger.activateBuffering();
> +    })
> +  };
> +

could we not just call self.simpletest.requestCompleteLog() ?  I think this is more complete, but if we change one in the future we could overlook this.
Attachment #8483006 - Flags: review?(jmaher) → review+
(In reply to Joel Maher (:jmaher) from comment #7)
> Comment on attachment 8483006 [details] [diff] [review]
> Implement SimpleTest.requestCompleteLog for mochitest browser
> 
> Review of attachment 8483006 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with one nit addressed and one question answered.
> 
> ::: testing/mochitest/browser-test.js
> @@ +25,5 @@
> >    "resource:///modules/ContentSearch.jsm");
> >  
> >  const SIMPLETEST_OVERRIDES =
> > +  ["ok", "is", "isnot", "ise", "todo", "todo_is", "todo_isnot", "info", "expectAssertions",
> > +   "requestCompleteLog"];
> 
> there is no need for a newline here, lets just keep it as two lines.

Ok.

> 
> @@ +915,5 @@
> > +    self.registerCleanupFunction(function() {
> > +      self.__tester.dumper.structuredLogger.activateBuffering();
> > +    })
> > +  };
> > +
> 
> could we not just call self.simpletest.requestCompleteLog() ?  I think this
> is more complete, but if we change one in the future we could overlook this.

Structured logging (particularly the rewrite of buffering logic from JavaScript to Python) made SimpleTest's base requestCompleteLog incompatible with browser chrome tests, getting us into this mess in the first place.
Fixed nits. I'm intentionally including the change to browser_pass.js.
Attachment #8483006 - Attachment is obsolete: true
Comment on attachment 8483529 [details] [diff] [review]
Implement SimpleTest.requestCompleteLog for mochitest browser.

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

r=jmaher
Attachment #8483529 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/869d64f8ab6c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.