812514 - B2G emulator unittests should dump logcat to stdout on failure

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Andrew Halberstadt [:ahal]]

There have been several issues that could only be reproduced on the test slaves, not locally (e.g bug 809437). It would have been helpful for us to have the logcat dumped to the logs to help with debugging these kinds of errors.

The Mn tests already do this, but the others (mochitest/reftest) don't.

Comment 1

[Andrew Halberstadt [:ahal]]

This will require a patch to the b2g automation and the mozharness script.

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: Patch 1.0 (gecko) - Add logcat_dir option to b2g unittests

I also updated how we start the logcat process (we no longer need to call waitForFinish so there's no reason to start it in a new thread).

We'll need to land this patch before the mozharness change on all branches that we have b2g running on (which means m-c, aurora and probably beta since today is merge day).

Comment 3

[Andrew Halberstadt [:ahal]]

Some other notes:
The above patch worked locally, but I pushed to try anyway (due to mozprocess dependency changes): https://tbpl.mozilla.org/?tree=Try&noignore=1&rev=72b2e3c96783

I forgot to remove import multiprocessing.. won't bother with a new patch, but I'll do it before landing.

Comment 4

[Andrew Halberstadt [:ahal]]

Attachment: Patch 1.0 (mozharness) - Bits to enable and dump logcat

Aki, I'm wondering if the line:
> (parser.passed == 0 and parser.failed == 0) or parser.failed > 0

should catch all of the cases where we might want to dump the logcat. Would this include infra failures?

Comment 5

[Aki Sasaki (not active)]

(In reply to Andrew Halberstadt [:ahal] from comment #4)
> Created attachment 683235 [details] [diff] [review]
> Patch 1.0 (mozharness) - Bits to enable and dump logcat
> 
> Aki, I'm wondering if the line:
> > (parser.passed == 0 and parser.failed == 0) or parser.failed > 0
> 
> should catch all of the cases where we might want to dump the logcat. Would
> this include infra failures?

I'm not entirely sure, but it looks like it's parser.fail_count and parser.pass_count ?

I think -1 for either means we haven't reached the pass/fail/known output.
If you're looking for a logcat for any failure condition, parser.fail_count != 0 might do the trick.

Comment 6

[Aki Sasaki (not active)]

Comment on attachment 683235 [details] [diff] [review]
Patch 1.0 (mozharness) - Bits to enable and dump logcat

I think this is good other than the parser.passed line.
r=me with that fixed; feel free to either do that or send me another review if you want.

Comment 7

[Andrew Halberstadt [:ahal]]

(In reply to Aki Sasaki [:aki] from comment #6)
> Comment on attachment 683235 [details] [diff] [review]
> Patch 1.0 (mozharness) - Bits to enable and dump logcat
> 
> I think this is good other than the parser.passed line.
> r=me with that fixed; feel free to either do that or send me another review
> if you want.

Ok, I just copied what marionette.py did. I'll update that as well and do an inc review.

Comment 8

[Aki Sasaki (not active)]

I think they're different because they're using different parsers -- marionette has its own, b2g_emulator_unittest.py uses the DesktopUnittestOutputParser.

Comment 9

[Jonathan Griffin (:jgriffin)]

Comment on attachment 683190 [details] [diff] [review]
Patch 1.0 (gecko) - Add logcat_dir option to b2g unittests

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

Cool, thanks!

Comment 10

[Andrew Halberstadt [:ahal]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7a0bf63a5268

I missed the merge (aurora/beta are currently closed) so I'll have to land in both those branches before landing the mozharness change. I may need some help landing on beta.. not sure if the same deal we had with aurora will still hold or not.

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7a0bf63a5268

Comment 12

[Andrew Halberstadt [:ahal]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d0c7324bc500

I'm going to hold off on beta since:
a) we don't have tests running there yet
b) I don't know what the approval process is

But if we do plan on enabling tests there we'll need this patch before doing so.

Comment 13

[Andrew Halberstadt [:ahal]]

Attachment: Patch 2.0 (mozharness) - Address review comment, carry r+ forward

Comment 14

[Andrew Halberstadt [:ahal]]

The required gecko change has landed on inbound, central and aurora. Not yet on beta but we don't have automation there anyway.

If I land the mozharness change now it will break b2g emulator tests on all of the core project branches that haven't yet been merged with m-c (e.g fx-team, services-central, cedar, etc). Is this acceptable? How often do these get merged? I can merge cedar at the least, but not sure about the others.

Comment 15

[Andrew Halberstadt [:ahal]]

According to philor b2g tests should be enabled on beta after the next reconfig.. so I should land the gecko patch there before landing the mozharness one.

Comment 16

[Jonathan Griffin (:jgriffin)]

FYI, landing process on beta is same for aurora, just a=NPOTB for automation changes.

Comment 17

[Andrew Halberstadt [:ahal]]

https://hg.mozilla.org/releases/mozilla-beta/rev/80afbd9cf1e5

Comment 18

[Andrew Halberstadt [:ahal]]

https://hg.mozilla.org/build/mozharness/rev/595d0844f8ef

This is one of those times where it would be nice to have some sort of mozharness try. I'll keep a close watch on the tree and be ready to back this out if there are any problems.

Comment 19

[Andrew Halberstadt [:ahal]]

Woops, https://tbpl.mozilla.org/php/getParsedLog.php?id=17217191&tree=Mozilla-Inbound&full=1

Comment 20

[Andrew Halberstadt [:ahal]]

Attachment: Patch 2.1 (mozharness) - oops

Comment 21

[Andrew Halberstadt [:ahal]]

https://hg.mozilla.org/build/mozharness/rev/5f523a752d57