1036565 - marionette_test calling addError results in multiple test_end messages

Status: RESOLVED INVALID

(Testing :: Mozbase, defect)

Description

[Chris Manchester (limited bugmail, email directly)]

Browsing marionette logs today I noticed exceptions thrown in the tbplformatter since landing bug 956739 when an error is encountered in a test. It looks like marionette_test calling addError directly is causing multiple test_end messages to be emmited by the strucutured runner, causing the formatter to throw.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

An example:
https://tbpl.mozilla.org/php/getParsedLog.php?id=43430759&tree=Mozilla-Central&full=1#error3

Comment 2

[Chris Manchester (limited bugmail, email directly)]

Attachment: Prevent the tbplformatter from throwing when multiple test_end messages are issued.

We need to arrange to have marionette use the structured api correctly, but here's a stopgap to keep the formatter from throwing in the meantime, in case the errors missing from the logs due to this are useful.

Comment 3

[Chris Manchester (limited bugmail, email directly)]

As far as I can see, this can only happen if both this:
http://hg.mozilla.org/mozilla-central/annotate/0f8a2996abc9/testing/marionette/client/marionette/marionette_test.py#l189

and this:

http://hg.mozilla.org/mozilla-central/annotate/0f8a2996abc9/testing/marionette/client/marionette/marionette_test.py#l205

execute, implying an exception thrown in tearDown(). I guess I don't know why this would happen, or if the second error might be significant.

Comment 4

[Jonathan Griffin (:jgriffin)]

We do a few things in tearDown:

http://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/marionette_test.py#352

So if something bad happened during the test (like B2G hung or crashed), we could end up timing out during the test (error #1) and then dying during tearDown as well (error #2).  In this case, the second error isn't really significant, since it's a side-effect of the first.

Comment 5

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8453243 [details] [diff] [review]
Prevent the tbplformatter from throwing when multiple test_end messages are issued.

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

Ok, let's not do this as a stopgap. I still don't think the formatter should throw, but let's work that out in bug 1036427.

Comment 6

[James Graham [:jgraham]]

It seems like an error in teardown should just be logged as an error, and we should enforce the invariant that we have always logged test_end before teardown (or we always do after teardown).

Comment 7

[Chris Manchester (limited bugmail, email directly)]

I'm assuming this patch isn't relevant anymore.