1253618 - Add support for e10s & asan annotations in Marionette manifests

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

mochitests have valid e10s and asan values for use in creating rules (i.e. |skip-if = e10s| or |skip-if = asan|). They do not appear to work for Marionette. Fixing that would allow us to turn on Mn on ASAN builds (one test that depends on the crash reporter needs skipping) and would allow for a less heavy-handed test skipping approach in bug 1239552 until the underlying problem can be investigated and fixed.

Comment 1

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Re e10s - as per [1], Marionette harness has a skip_if_e10s decorator; which I just tested locally with [2] and it works. You should use that to skip individual tests. 

As for skipping entire files with |skip-if = e10s| or |skip-if = asan|: right now the Mn runner calls manifestparser [3], but only provides values from |mozinfo.info|, so nothing regarding e10s or asan. I'll look into adding the missing data.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1239552#c3
[2] https://dxr.mozilla.org/mozilla-central/source/testing/marionette/harness/marionette/tests/unit/test_about_pages.py#58
[3] https://dxr.mozilla.org/mozilla-central/source/testing/marionette/harness/marionette/runner/base.py#985-990

Comment 2

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Here's a try run that shows |skip-if = e10s| working: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4dcad126a2d3&filter-tier=1&filter-tier=2&filter-tier=3&exclusion_profile=false

It seems Mn tests don't ever run against asan builds, but based on code in mozrunner, I understand that 'asan' is included in mozinfo.info, so |skip-if = asan| should already work with Marionette runner. I haven't had a chance to test this myself. Will do so later.

Comment 3

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Attachment: MozReview Request: Bug 1253618 - Add support for |skip-if = e10s| manifest annotations in Marionette runner; r=AutomatedTester

Review commit: https://reviewboard.mozilla.org/r/38825/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38825/

Comment 4

[Ryan VanderMeulen [:RyanVM]]

(In reply to Maja Frydrychowicz (:maja_zf) from comment #2)
> It seems Mn tests don't ever run against asan builds, but based on code in
> mozrunner, I understand that 'asan' is included in mozinfo.info, so |skip-if
> = asan| should already work with Marionette runner. I haven't had a chance
> to test this myself. Will do so later.

I filed this because my earlier results say it isn't :). Here's a link to a recent Cedar run showing it:
https://treeherder.mozilla.org/#/jobs?repo=cedar&revision=84b2eeea8164&filter-searchStr=asan%20mn

Comment 5

[David Burns :automatedtester]

Comment on attachment 8728167 [details]
MozReview Request: Bug 1253618 - Add support for |skip-if = e10s| manifest annotations in Marionette runner; r=AutomatedTester

https://reviewboard.mozilla.org/r/38825/#review35547

Comment 6

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

I've learned that mochitest updates mozinfo with the content of mozinfo.json, which is found in mochitest.tests.zip; then mozinfo.info will contain an 'asan' key. So we need to do the same in Marionette runner, which means I need to get mozinfo.json into common.tests.zip.

Patch forthcoming + I'll file a bug about getting mozinfo.json into common.tests.zip

Comment 7

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8728167 [details]
MozReview Request: Bug 1253618 - Add support for |skip-if = e10s| manifest annotations in Marionette runner; r=AutomatedTester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38825/diff/1-2/

Comment 8

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Attachment: MozReview Request: Bug 1253618 - Support |skip-if = asan| manifest annotations in Marionette runner; r?AutomatedTester

Tentative patch up for feedback; |skip-if = asan| works locally in the presence
of a mozinfo.json file. (Skip is ignored if mozinfo.json file is absent)

`update_mozinfo` is copied from mochitest, which I'm not too happy with.
https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#553-565

I'm looking into moving this function into some common area, like mozinfo itself.
Any thoughts?

Review commit: https://reviewboard.mozilla.org/r/38999/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/38999/

Comment 9

[David Burns :automatedtester]

Comment on attachment 8728640 [details]
MozReview Request: Bug 1253618 - Support |skip-if = asan| manifest annotations in Marionette runner; r?AutomatedTester

https://reviewboard.mozilla.org/r/38999/#review35793

::: testing/marionette/harness/marionette/runner/base.py:35
(Diff revision 1)
> +def update_mozinfo(path=None):

Yea, this looks like it would benefit from being in a more central place. We can do that in a separate bug if you want so that you can update the other runners at the same time.

Comment 10

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

https://reviewboard.mozilla.org/r/38999/#review35793

> Yea, this looks like it would benefit from being in a more central place. We can do that in a separate bug if you want so that you can update the other runners at the same time.

I filed Bug 1255815 to take care of the moving later.

Comment 11

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Here's a try run with extra log messages showing that an "asan" key is getting added to |mozinfo.info| - https://treeherder.mozilla.org/#/jobs?repo=try&revision=1fb55935bf27&selectedJob=18012034

Comment 12

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8728167 [details]
MozReview Request: Bug 1253618 - Add support for |skip-if = e10s| manifest annotations in Marionette runner; r=AutomatedTester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38825/diff/2-3/

Comment 13

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8728640 [details]
MozReview Request: Bug 1253618 - Support |skip-if = asan| manifest annotations in Marionette runner; r?AutomatedTester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38999/diff/1-2/

Comment 14

[David Burns :automatedtester]

Comment on attachment 8728640 [details]
MozReview Request: Bug 1253618 - Support |skip-if = asan| manifest annotations in Marionette runner; r?AutomatedTester

https://reviewboard.mozilla.org/r/38999/#review36559

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/936703b9e4cd
https://hg.mozilla.org/integration/mozilla-inbound/rev/595815f1f12a

Comment 16

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Seeing failures on some platforms.
gecko.log shows:
>>1458055951320	Marionette	DEBUG	Got request: executeScript, data: {"name":"executeScript","parameters":{"scriptTimeout":null,"newSandbox":true,"args":[],"filename":"base.py","script":"\n            try {\n              return Services.appinfo;\n            } catch (e) {\n              return null;\n            }","sandbox":"default","line":677}}, id: null
>>1458055951331	Marionette	INFO	sendToClient: {"from":"0","error":{"message":"NS_ERROR_FILE_INVALID_PATH: Component returned failure code: 0x80520009 (NS_ERROR_FILE_INVALID_PATH) [nsIFile.target]","status":17,"stacktrace":"execute_script @base.py, line 677\ninline javascript, line 21\nsrc: \"undefined\""}}, {643000ef-03f7-4371-aeda-d8209fb6890e}, {643000ef-03f7-4371-aeda-d8209fb6890e}

Comment 17

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

And by "some platforms" I really mean Linux 64 opt (Ld only) - https://treeherder.mozilla.org/logviewer.html#?job_id=23797053&repo=mozilla-inbound

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for the Ld failure on Linux platforms.

Backouts:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a15ce89d5a1e
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ffaddc56d24

A descendant push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=6b3093fbaa8a
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=23797053&repo=mozilla-inbound

Comment 19

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

My guess is that this b2g binary from 2015 could be the problem:
https://dxr.mozilla.org/mozilla-central/rev/7773387a9a2f1fd10e4424ea923c6185063f620b/testing/mozharness/configs/luciddream/linux_config.py#15

Comment 20

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

I'm inclined to simply work around this error in Marionette runner.

A few questions
- Given that the luciddream tests use b2g, shouldn't they be tier-3 on Treeherder?
- If not, can you provide or prepare a b2g binary that will work with Marionette's call to Services.appinfo?

Comment 21

[Alexandre Poirot [:ochameau]]

luciddream might just be disabled. Let me ask around.

Comment 22

[Alexandre Poirot [:ochameau]]

I do not see the direct relation between "NS_ERROR_FILE_INVALID_PATH [nsIFile.target]" and Services.appinfo but yes, you can disable luciddream. I thought it was disable as other b2g test suites.

Comment 23

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/38b6ef96c7d0
https://hg.mozilla.org/integration/mozilla-inbound/rev/b3fa8850897f

Comment 24

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Landed these again, since Ld is Tier-3

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/38b6ef96c7d0
https://hg.mozilla.org/mozilla-central/rev/b3fa8850897f

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/92b64cbafa53
https://hg.mozilla.org/releases/mozilla-aurora/rev/cb16eda2fb43

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5e2fc8a6942f
https://hg.mozilla.org/releases/mozilla-beta/rev/6ef3c032d038