1052937 - Missing subtest (assertion) name in mochitest conflicts with structured logs

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Ahmed Kachkach [:akachkach]]

Testing a patch for bug 1050251, I added a seeded timeout error and hit this exception in logtypes: 

https://tbpl.mozilla.org/php/getParsedLog.php?id=45790129&tree=Try&full=1#error5

Comment 1

[James Graham [:jgraham]]

Do we know what the js output was when this message was hit? If not can we find out? I imagine you should see the same error locally.

Comment 2

[Ahmed Kachkach [:akachkach]]

I can't test mochitests locally (due to a change in Apple's headers, long story :)), but could this be caused by the fact that a test_status doesn't have a message field? (the patch slightly changed the structured logging to ignore empty strings when creating a test_status message)

Comment 3

[Ahmed Kachkach [:akachkach]]

Attachment: 0001-Bug-1052937-Default-value-for-mochitest-assertions-n.patch

This should do it (the issue was that some assertions had an undefined name, which causes the subtest field to be missing). Maybe we should have a check for missing fields in the future?

Comment 4

[Andrew Halberstadt [:ahal]]

Comment on attachment 8472501 [details] [diff] [review]
0001-Bug-1052937-Default-value-for-mochitest-assertions-n.patch

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

Lgtm!

Comment 5

[Ahmed Kachkach [:akachkach]]

Try run: https://tbpl.mozilla.org/?tree=Try&rev=c44d00cb5bfd

Comment 6

[Ahmed Kachkach [:akachkach]]

Try run looks good.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8a6bb8913ecb

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8a6bb8913ecb

Comment 9

[Ahmed Kachkach [:akachkach]]

Attachment: 0001-Bug-1052937-Default-value-for-missing-subtest-in-moc.patch

athena from #devtools just reported that she was encountering the same bug in devtools test. This is because we only fixed this issue in SimpleTest instead of fixing it at the root (in the structured logger).

With this patch, this problem is also fixed for tests that do not use SimpleTest

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b7aa62ee02a5

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b7aa62ee02a5

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5d4f07c87783