842616 - Running make test-perf sometimes report measures multiple times

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Anthony Ricaud (:rik)]

This happens when we quit the tests before the end. We don't call unregisterLoadTimeListener which means we have multiple listeners when we run it again.

We could solve this by rebooting before running performance tests:
+ Clean state for our measurements
- Slows down the process

Or we could also try to remove the listeners when we setup the tests. I'm not sure it will solve every case but we should try.

Comment 1

[Anthony Ricaud (:rik)]

Attachment: Proposed patch

Finally, that was easy. :)

Comment 2

[Julien Wajsberg [:julienw]]

Comment on attachment 716516 [details] [diff] [review]
Proposed patch

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

r=me with the nit

please file a bug about doing this measurement in a friendlier way, regarding the insertion into the global object.

::: tests/performance/performance_helper.js
@@ +18,5 @@
>        var registerListener =
>          'var w = global.wrappedJSObject;' +
>          'w.loadTimes = [];' +
> +        'if (w.onapplicationloaded) {' +
> +        '  /* We\'ve been here before, let\'s clean ! */' +

nit: You can put the comments outside of the string you know :)

Comment 3

[Anthony Ricaud (:rik)]

https://github.com/mozilla-b2g/gaia/commit/889a20d22a972932298c62b7aab4932dd577e245

Opened bug 844032 for the global object pollution.

Comment 4

[Julien Wajsberg [:julienw]]

Comment on attachment 716516 [details] [diff] [review]
Proposed patch

only changes are in the performance framework

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 716516 [details] [diff] [review]
Proposed patch

a=tests

Comment 6

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x  889a20d22a972932298c62b7aab4932dd577e245
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 7

[Julien Wajsberg [:julienw]]

v1-train: 3429fd39e3f556b2ecea5ed72df49e8e4c6808ba