Closed
Bug 1059381
Opened 8 years ago
Closed 8 years ago
SimpleTest.requestCompleteLog does not work as expected for mochitest-browser
Categories
(Testing :: Mochitest, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla35
People
(Reporter: chmanchester, Assigned: chmanchester)
Details
Attachments
(1 file, 1 obsolete file)
2.49 KB,
patch
|
chmanchester
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•8 years ago
|
||
:akachkach, were you looking into this one? How's it going?
Flags: needinfo?(ahmed.kachkach)
Comment 2•8 years ago
|
||
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)
Assignee | ||
Comment 3•8 years ago
|
||
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 :)
Comment 4•8 years ago
|
||
I only too --quiet out of the in tree stuff in my local patch queue
Assignee | ||
Comment 5•8 years ago
|
||
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 | ||
Updated•8 years ago
|
Assignee: nobody → cmanchester
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•8 years ago
|
||
Comment on attachment 8483006 [details] [diff] [review] Implement SimpleTest.requestCompleteLog for mochitest browser Ok, try is green.
Attachment #8483006 -
Flags: review?(jmaher)
Comment 7•8 years ago
|
||
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+
Assignee | ||
Comment 8•8 years ago
|
||
(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.
Assignee | ||
Comment 9•8 years ago
|
||
Fixed nits. I'm intentionally including the change to browser_pass.js.
Assignee | ||
Updated•8 years ago
|
Attachment #8483006 -
Attachment is obsolete: true
Assignee | ||
Comment 10•8 years ago
|
||
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+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 11•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/869d64f8ab6c
Flags: in-testsuite+
Keywords: checkin-needed
Comment 12•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/869d64f8ab6c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in
before you can comment on or make changes to this bug.
Description
•