Closed Bug 1034267 Opened 6 years ago Closed 6 years ago

Structured Logging for robocop tests

Categories

(Testing :: Mozbase, defect)

x86
macOS
defect
Not set
normal

Tracking

(firefox33 fixed, firefox34 fixed)

RESOLVED FIXED
mozilla34
Tracking Status
firefox33 --- fixed
firefox34 --- fixed

People

(Reporter: chmanchester, Assigned: akachkach)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 5 obsolete files)

Bug 886570 gets us most much of the way there, but we need to emit structured messages from java (bug 1034236).
Assignee: nobody → akachkach
Attached 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: 6 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.