Closed
Bug 1045717
Opened 10 years ago
Closed 10 years ago
Failures in current memory test for make test-perf leak into launch timings
Categories
(Firefox OS Graveyard :: Gaia::PerformanceTest, defect, P1)
Tracking
(b2g-v2.0 fixed, b2g-v2.1 fixed)
RESOLVED
FIXED
2.1 S1 (1aug)
People
(Reporter: Eli, Assigned: Eli)
References
Details
(Keywords: perf, Whiteboard: [c=automation p=1 s= u=])
Attachments
(2 files)
31 bytes,
text/plain
|
Details | |
46 bytes,
text/x-github-pull-request
|
hub
:
review+
bajaj
:
approval-gaia-v2.0+
|
Details | Review |
As seen in the attached screenshot [1], the launch timings for SMS, cost control, settings, and video were displaying erratic values. Inspecting the replicate values for each app, only the initial launch time was affected; each of the other replicates launched with normal times. Upon inspection of the output from make test-perf [2], each of the erratic values was preceded by a failing startup memory test with output displaying the following assertion error: couldn't collect mem usage. This matches directly with the assertion generated at [3]. Something was causing these tests to fail, and for some reason also affected the separate tests for launch timings. We should probably be closing the app using the provided `app.close()` method before we assert the data is good so other tests can continue normally. [1] https://cloudup.com/c_NnuurKjZh [2] http://jenkins1.qa.scl3.mozilla.com/job/flame.b2g-inbound.perf.gaia/884/artifact/perf.json [3] https://github.com/mozilla-b2g/gaia/blob/master/tests/performance/startup_test.js#L55
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → eperelman
Status: NEW → ASSIGNED
Whiteboard: [c=automation p= s= u=] → [c=automation p=1 s= u=]
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8464147 -
Flags: review?(hub)
Assignee | ||
Comment 2•10 years ago
|
||
The PR from comment 1 should resolve the issue of leaking problems into subsequent tests when the memory test asserts bad, but does not attempt to resolve what could have caused the original memory test to assert bad in the first place.
Comment 3•10 years ago
|
||
Comment on attachment 8464147 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/22273 r=me See the nit on the PR. Also maybe we want this on 2.0?
Attachment #8464147 -
Flags: review?(hub) → review+
Assignee | ||
Comment 4•10 years ago
|
||
I've commented on the nit in the PR, please let me know if you would still like me to make the change and I will. I don't think having this on 2.0 would be a bad thing, once this lands could you please mark this for gaia-approval-v2.0? Thanks Hub!
Comment 5•10 years ago
|
||
Merged in master https://github.com/mozilla-b2g/gaia/commit/694fee705ae128e20adea97ed8e0b3fea8d57c41
Comment 6•10 years ago
|
||
Comment on attachment 8464147 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/22273 NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] [Bug caused by] (feature/regressing bug #): [User impact] if declined: testing will be less reliable. [Testing completed]: make test-perf [Risk to taking this patch] (and alternatives if risky): NPOTB. testing only [String changes made]: none Thank you kindly.
Attachment #8464147 -
Flags: approval-gaia-v2.0?
Updated•10 years ago
|
Attachment #8464147 -
Flags: approval-gaia-v2.0? → approval-gaia-v2.0+
Assignee | ||
Updated•10 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 7•10 years ago
|
||
v2.0: https://github.com/mozilla-b2g/gaia/commit/bb3d2cf36c10f7186899431d4a21a2489864d9c6
You need to log in
before you can comment on or make changes to this bug.
Description
•