1048559 - Always dump log messages with an ERROR or CRITICAL level

Status: RESOLVED WONTFIX

(Testing :: Mochitest, defect)

Description

[Ahmed Kachkach [:akachkach]]

The log buffering in mochitest hides all log message unless there's a failure (a test failing, or an unstructured log starting with TEST-UNEXPECTED since we still have some of those in a couple tests).

We should also consider log messages with an ERROR or CRITICAL log level.

Comment 1

[Ahmed Kachkach [:akachkach]]

Attachment: 0001-Bug-1048559-Consider-ERROR-and-CRITICAL-mochitest-lo.patch

proposed patch;

try push: https://tbpl.mozilla.org/?tree=Try&rev=f5a63d3ff35d

Comment 2

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8467354 [details] [diff] [review]
0001-Bug-1048559-Consider-ERROR-and-CRITICAL-mochitest-lo.patch

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

It looks like this isn't in the old log buffering logic, so I'm not sure why this is necessary now. It will give some context whenever these levels are logged, but wont change the job status. Can you give some context for the issue this is trying to solve?

Comment 3

[Ahmed Kachkach [:akachkach]]

We were trying to solve bug 1032878, and mwargers created a patch that logged an error in case a test_status is sent after a test_end, but with the current buffering logic we're missing those errors.

But maybe we should try using ok(false, some_message) in that case instead?

Comment 4

[Martijn Wargers (dead)]

The ok function doesn't exist in the TestRunner.js window, which is the parent window of the test iframe (where SimpleTest.ok exists).
SimpleTest.ok is essentially doing this logging: http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js#339
Should I be using something similar for bug 1032878?

Comment 5

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8467354 [details] [diff] [review]
0001-Bug-1048559-Consider-ERROR-and-CRITICAL-mochitest-lo.patch

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

After chatting with akachkach, it sounds like there are other ways to get the information we need from the runner.

Comment 6

[Ahmed Kachkach [:akachkach]]

Changing this to WONTFIX for now as we're not planning to go this way.