Structured Logging for robocop tests

RESOLVED FIXED in Firefox 33

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cmanchester, Assigned: akachkach)

Tracking

(Blocks 1 bug)

unspecified
mozilla34
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox33 fixed, firefox34 fixed)

Details

Attachments

(1 attachment, 5 obsolete attachments)

Bug 886570 gets us most much of the way there, but we need to emit structured messages from java (bug 1034236).
Assignee: nobody → akachkach
Posted patch 0003-FennecMochitest.patch (obsolete) — Splinter Review
WIP.

(relies on bug 1034236)
Blocks: 1043485
Updated, seems to be working (https://tbpl.mozilla.org/?tree=Try&rev=097966857df3).

Wondering how I should handle dumpLog with a Throwable? I just concatenated the message with the String representation of the error for now.
Attachment #8453463 - Attachment is obsolete: true
Attachment #8462007 - Flags: review?(gbrown)
The logs in the try run result in multiple suite_end messages. This is ok with respect to the issue in bug 1043485 but will not be compatible with consumers of the structured log.
Deactivating buffering seem to have fixed this issue (see this try run: https://tbpl.mozilla.org/?rev=d74d7bb327fc&tree=Try)

I'm also trying to get rid of the multiple SUITE-END messages, waiting for a try run with the submitted patch (that should hopefully fix it).

If the logs are valid, we should try to land this before the workaround in bug 1043485.
Attachment #8462007 - Attachment is obsolete: true
Attachment #8462007 - Flags: review?(gbrown)
Attachment #8462859 - Flags: review?(gbrown)
Small change to the patch (to remove multiple suite_end messages).
Try run: https://tbpl.mozilla.org/?tree=Try&rev=18f40cf430b8
Attachment #8462859 - Attachment is obsolete: true
Attachment #8462859 - Flags: review?(gbrown)
Attachment #8462917 - Flags: review?(gbrown)
Comment on attachment 8462917 [details] [diff] [review]
0001-Bug-1034267-Structured-Logging-for-robocop-r-gbrown.patch

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

::: build/mobile/robocop/FennecMochitestAssert.java
@@ +89,5 @@
>          }
>  
>      }
>  
> +    private void _logMochitestResult(testInfo test, String passStatus, String passExpected, String failStatus, String failExpected) {

I would like to see a comment here explaining what the parameters mean.

@@ +117,5 @@
>              junit.framework.Assert.fail(message);
>          }
>      }
>  
> +

There is an extra line here - remove it.
Attachment #8462917 - Flags: review?(gbrown) → review+
fixed nits; carry r+ forward
Attachment #8462917 - Attachment is obsolete: true
Attachment #8463486 - Flags: review+
Keywords: checkin-needed
hi, do you know if the repeated mochitest 1 error on b2g are related to this push ?
Keywords: checkin-needed
I'll rebase and do another push to make sure!
Carry r+ forward

The B2G-desktop error was caused by a missing call to suite_end and buffer messages dumping. Fixed now!

Try run: https://tbpl.mozilla.org/?tree=Try&rev=c2b0a8fbed01
Attachment #8463486 - Attachment is obsolete: true
Attachment #8464268 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f3a5a084d90e
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Depends on: 1049085
You need to log in before you can comment on or make changes to this bug.