Closed
Bug 1410355
Opened 7 years ago
Closed 7 years ago
Re-enable Marionette unit tests for ASAN builds except for crash tests
Categories
(Remote Protocol :: Marionette, enhancement)
Tracking
(firefox57 fixed, firefox58 fixed)
RESOLVED
FIXED
mozilla58
People
(Reporter: whimboo, Assigned: whimboo)
References
Details
Attachments
(1 file)
(In reply to Ted Mielczarek [:ted.mielczarek] from bug 1223277 comment #100) > Sorry for not getting back to this, but ASAN builds disable the > crashreporter, so tests for crash reporting should not be run on them. Right now we disabled all the Marionette unit tests for ASAN builds. With this information we should be able to unskip most of them, except the crash tests.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8920983 -
Flags: review?(ato)
Comment 2•7 years ago
|
||
Is running Marionette on ASAN builds, which I understand are quite
slow, good use of resources? What do we gain from running Mn on
ASAN builds?
A digression is that it seems to me that the way the Mn job is
currently disabled should really have been done at a TC level so
that TC would not have to allocate a test slave in the first place:
> [include:unit/unit-tests.ini]
> skip-if = asan
Flags: needinfo?(hskupin)
Assignee | ||
Comment 3•7 years ago
|
||
Each crash we can find with Marionette by using ASAN builds is helpful. In all those cases they are most likely security related. I would like to see that enabled because only our harness can do all the restart logic.
Flags: needinfo?(hskupin)
Comment 4•7 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #3) > Each crash we can find with Marionette by using ASAN builds > is helpful. In all those cases they are most likely security > related. I would like to see that enabled because only our harness > can do all the restart logic. That sounds reasonable, but I was wondering if there was a precedence for that.
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8920983 [details] Bug 1410355 - Re-enable Marionette unit tests for ASAN builds except crash tests. https://reviewboard.mozilla.org/r/191954/#review197248
Attachment #8920983 -
Flags: review?(ato) → review+
Assignee | ||
Comment 6•7 years ago
|
||
(In reply to Andreas Tolfsen ‹:ato› from comment #4) > That sounds reasonable, but I was wondering if there was a > precedence for that. We actually disabled the tests via bug 1223277 comment 90 and following to get the click + close window code passing. (In reply to Andreas Tolfsen ‹:ato› from comment #2) > A digression is that it seems to me that the way the Mn job is > currently disabled should really have been done at a TC level so > that TC would not have to allocate a test slave in the first place: > > > [include:unit/unit-tests.ini] > > skip-if = asan We still run the other existing tests of that manifest. So it's all fine. Lets observe the results in the next months to see how valuable it still is to run the ASAN builds for our unit tests. Since disabled I haven't seen any new bug filed from the other Marionette tests.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Assignee | ||
Updated•7 years ago
|
status-firefox57:
--- → affected
status-firefox58:
--- → affected
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/363bf0516c89 Re-enable Marionette unit tests for ASAN builds except crash tests. r=ato
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/363bf0516c89
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Updated•7 years ago
|
Assignee | ||
Comment 9•7 years ago
|
||
test-only change which might be good to have on beta if it applies cleanly. Otherwise lets wontfix.
Keywords: checkin-needed
Comment 10•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-release/rev/0951afb05f1c
Keywords: checkin-needed
Updated•2 years ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•