949209 - make test-perf fails

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect, P1)

Description

[Hubert Figuiere [:hub]]

Apparently make test-perf fails on Hamachi.

I don't seem to get the source of the "apploadtime" event (e.target.src is null).

Comment 1

[Hubert Figuiere [:hub]]

Actually it is not specific to hamachi.

It is likely caused by bug 907013.

Comment 2

[Hubert Figuiere [:hub]]

Not so sure of the cause actually. But it is not specific to inari, that's for sure.

Comment 3

[Hubert Figuiere [:hub]]

I mean to hamachi as I reproduce on inari.

Comment 4

[Hubert Figuiere [:hub]]

Git bisect show the first bad revision being a6c2af8d32274113e81d2c57e23d0621af118e53 which is bug 907013

Comment 5

[Hubert Figuiere [:hub]]

See bug 936676

Comment 6

[Hubert Figuiere [:hub]]

But, e.detail.manifestURL return null. Any idea?

Comment 7

[James Lal [:lightsofapollo]]

Can we rig |make test-perf| to run a subset of tests on travis just to ensure we have not broken it... We can migrate that into TBPL soon enough.

Comment 8

[Jason Smith [:jsmith]]

This sounds like an automation blocker, right? If so, please nominate this to block for the earliest release this bug applies to.

Comment 9

[Hubert Figuiere [:hub]]

We have bug 936416 for datazilla. Yes that would be nice, but we might get lot of intermittent that we should fix. We can think of the subset, for sure.

Comment 10

[Hubert Figuiere [:hub]]

(In reply to Jason Smith [:jsmith] from comment #8)
> This sounds like an automation blocker, right? If so, please nominate this
> to block for the earliest release this bug applies to.

I think it only impact master as the window manager refactor is only on master. 1.4? but then it is already blocking me on everything else.

Comment 11

[Hubert Figuiere [:hub]]

The error is 
      "stack": "{ name: 'TypeError',\n  stack: 'TypeError: Cannot call method \\'indexOf\\' of null\\n    at /home/hub/source/mozilla/B2G-inari/gaia/tests/performance/startup_test.js:53:23\\n    at Array.filter (native)\\n    at Context.<anonymous> (/home/hub/source/mozilla/B2G-inari/gaia/tests/performance/startup_test.js:51:23)\\n    at Test.Runnable.run (/home/hub/source/mozilla/B2G-inari/gaia/node_modules/mocha/lib/runnable.js:211:32)\\n    at Runner.runTest (/home/hub/source/mozilla/B2G-inari/gaia/node_modules/mocha/lib/runner.js:372:10)\\n    at /home/hub/source/mozilla/B2G-inari/gaia/node_modules/mocha/lib/runner.js:448:12\\n    at next (/home/hub/source/mozilla/B2G-inari/gaia/node_modules/mocha/lib/runner.js:297:14)\\n    at /home/hub/source/mozilla/B2G-inari/gaia/node_modules/mocha/lib/runner.js:307:7\\n    at next (/home/hub/source/mozilla/B2G-inari/gaia/node_modules/mocha/lib/runner.js:245:23)\\n    at /home/hub/source/mozilla/B2G-inari/gaia/node_modules/mocha/lib/runner.js:269:7\\n    at Hook.Runnable.run (/home/hub/source/mozilla/B2G-inari/gaia/node_modules/mocha/lib/runnable.js:213:5)\\n    at next (/home/hub/source/mozilla/B2G-inari/gaia/node_modules/mocha/lib/runner.js:257:10)\\n    at /home/hub/source/mozilla/B2G-inari/gaia/node_modules/mocha/lib/runner.js:269:7\\n    at done (/home/hub/source/mozilla/B2G-inari/gaia/node_modules/mocha/lib/runnable.js:185:5)\\n    at /home/hub/source/mozilla/B2G-inari/gaia/node_modules/mocha/lib/runnable.js:197:9\\n    at Object.executeHook (/home/hub/source/mozilla/B2G-inari/gaia/node_modules/marionette-client/lib/marionette/client.js:370:18)\\n    at process._tickCallback (node.js:415:13)',\n  type: 'non_object_property_call',\n  constructorName: 'Error',\n  expected: null,\n  actual: null }",


Which is caused by e.target.src to be null.
e.detail.manifestURL is as well - that's the "recommended way" according to bug 936676 comment 8

And yes bug 907013 has landed since I'm hitting this now.

Comment 12

[(inactive after 6/18) Alive Kuo [:alive]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/14671

Append window in the detail if it's custom.

Please try event.detail.window.manifestURL instead. Thanks!

Comment 13

[(inactive after 6/18) Alive Kuo [:alive]]

And get src by |event.detail.window.browser.element.src|

Comment 14

[Hubert Figuiere [:hub]]

If you apply your patch and run |MARIONETTE_RUNNER_HOST=marionette-device-host RUNS=1 APP=sms make test-perf| with a device connected, b2g WILL freeze and be restarted...

Comment 15

[Hubert Figuiere [:hub]]

Comment on attachment 8347599 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/14671

see my comment. - I believe it might be related to trying to serialize window back to the marionette client. ie it hits a bug in Gecko.

Note that if you do |detail.src = this.browser.element.src;| in your patch instead, I'm good with it.

Comment 16

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8347599 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/14671

On demand

Comment 17

[Hubert Figuiere [:hub]]

Comment on attachment 8347599 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/14671

yup, works great. I still a local change in the test script. Maybe we could combine it. I'll attach the patch.

Comment 18

[Hubert Figuiere [:hub]]

Please get that patch too for this PR.

https://github.com/hfiguiere/gaia/commit/ffaff1e649fc74665dd75d8917099fc6fe07a722

Thanks

Comment 19

[Hubert Figuiere [:hub]]

Just to clarify this change in the test script is needed to deal with the change above. Together they fix the issue.

Comment 20

[(inactive after 6/18) Alive Kuo [:alive]]

https://github.com/mozilla-b2g/gaia/commit/ecb768890aafbb16d2a2eb1e24c3d662cd2a133a

Comment 21

[Hubert Figuiere [:hub]]

Thanks !

Comment 22

[Julien Wajsberg [:julienw]]

Hey guys,

does that mean that the perf framework on master does not work with the apps on 1.3/1.2 etc now?
I think it's a pity but that's probably unavoidable. Maybe a notice about this on MDN could be useful though.

Comment 23

[Hubert Figuiere [:hub]]

We could make it possible to have them work if need. Otherwise, yes I will add a note on MDN.

Comment 24

[Julien Wajsberg [:julienw]]

I got bit once already, but I don't think it's worth making it work on older phone. The tests will be specific to a Gaia version as well (except the most generic ones) so...

Comment 25

[Hubert Figuiere [:hub]]

in fact the fix should be easy.

see attached patch.

Comment 26

[Hubert Figuiere [:hub]]

Attachment: Propose backward compatibility fix.

Comment 27

[Julien Wajsberg [:julienw]]

Comment on attachment 8348854 [details] [diff] [review]
Propose backward compatibility fix.

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

looks good but I haven't tried it yet

::: tests/performance/performance_helper.js
@@ +59,4 @@
>          '}' +
>          'w.onapplicationloaded = function(e) {' +
>          '  var data = e.detail;' +
> +        '  if(data.src == null) data.src = e.target.src;' +

nit: `data.src = data.src || e.target.src;`

Comment 28

[Hubert Figuiere [:hub]]

ok, I'll do that an a PR.

Comment 29

[Hubert Figuiere [:hub]]

Attachment: Backward compat fix

Comment 30

[Julien Wajsberg [:julienw]]

Comment on attachment 8348864 [details] [review]
Backward compat fix

r=me
go go go !

Comment 31

[Hubert Figuiere [:hub]]

Merged.

https://github.com/mozilla-b2g/gaia/commit/0ab61f7281e3d2e23444796b5162d7a8dd64d315

Comment 32

[Hubert Figuiere [:hub]]

v1.3t uplift
https://github.com/hfiguiere/gaia/commit/ede66232af0fbd75d4287aa751196883ed8e6c0e
https://github.com/hfiguiere/gaia/commit/aab683c00a8980dacbe63c1eaf194779aad4040b