Julien, any update on this? Are you working on this and if not, do you know when you might? It's blocking several important performance automation efforts.

I'll take it. We need to go forward on that. Julien if you object to it, let me know.

Please do! I really wanted to do it quickly but the truth is that we all have more important things in our respective teams. I'd be very happy if the performance team would do this :)

Porting the testing framework from xpcshell to Node.js is underway in the following branch https://github.com/hfiguiere/gaia/tree/node-switch-over

I'm not sure this is the goal, but I may be wrong. Could be useful to have James peek this code :)

(In reply to Julien Wajsberg [:julienw] from comment #5) > I'm not sure this is the goal, but I may be wrong. Could be useful to have > James peek this code :) Yes it is. According to James, "anything that was designed to work with xpcshell needs to die fast"

The new integration tests _ONLY_ work on node... There never again will be an xpchshell marionette-js-client. I hope I was very clear about this during our original chats.

Comment on attachment 829061 [details] Part 1. Giving a very high level r+ (mostly on the deletes that where my mode :)) I wrote the original marionette client and a bunch of horrible xpcshell extensions that made it glue. I am comfortable reviewing that stuff (deletes :D) and extensions needed for the new runner. Most of the other code :Rik, :etienne_s and :julienw wrote and I don't feel super comfortable as the final word there (the performance framework bits). I am happy to support you in whatever way makes sense but I think we need another reviewer for the performance framework bits who can also act as a future maintainer.

Comment on attachment 829061 [details] Part 1. Julien, can we get a review? I have more fixes pending directly for the test to actually produce the results I'd like to push in a later patch. Thanks

Will do tomorrow!

Comment on attachment 829061 [details] Part 1. I can't really r+ on an unfinished patch. You can squash your current commits, fix the whitespaces in a separate commit, and then continue your work on another commit, so that the new changes will be easy to review. Please ask review again once you're ready :)

Julien, I add to pick that piece of patch from bug 912541. Thanks.

s/add/had/g

Comment on attachment 833025 [details] Gaia fix for the perf event picking the right reviewer. Thanks Vivien !

Comment on attachment 833025 [details] Gaia fix for the perf event Can you not simply get the src attribute of the evt.target since the event is dispatched on the iframe itself? Please re-ask r? if there is any technical reasons why you can't.

This is actually a good idea! :)

Indeed event.target.src works.

Comment on attachment 833025 [details] Gaia fix for the perf event we have seen there is a better solution. obsolete.

Comment on attachment 829061 [details] Part 1. Julien, this is the up to date branch. All the known test run on my inari and the performance data is output correctly. There are a few commits to address previous comments, and then a big one for part 2.

Comment on attachment 829061 [details] Part 1. review finished, please ask me a review again once you're ready. (I put "r-" so that we see there is not only James' r+, but don't take offense ;) ) For next review, please have 3 commits: 1 for the first review, 1 for this review, and 1 for the next review. Thanks!

Comment on attachment 829061 [details] Part 1. 3rd review?

Comment on attachment 829061 [details] Part 1. Some more comments, and the event-based tests is not finishing correctly.

Comment on attachment 829061 [details] Part 1. Event based test works here. Also fixed most of the issue. Maybe we can merge this and follow up. It is still way better than last time.

Comment on attachment 829061 [details] Part 1. I agree this is way better than the previous state. I still want the startup event test to work properly before merging this though.

As I said on Github, I had all the startup event test work for contacts (which is the only app that has one) with a medium reference workload, as well as fm (see work on bug 819080) I pushed the nit fixes to the PR.

(In reply to Hubert Figuiere [:hub] from comment #27) > As I said on Github, I had all the startup event test work for contacts > (which is the only app that has one) with a medium reference workload, as > well as fm (see work on bug 819080) > > I pushed the nit fixes to the PR. see https://github.com/mozilla-b2g/gaia/pull/13497#issuecomment-29890013 for more explanation. I was probably not clear enough in my previous comment. The output is correct, which does not mean that it works as expected. We need the test to stop in a timely manner, instead of waiting for mocha's timeout.

Note: this is probably not a big thing to fix, I'm very confident that we'll be able to finish this this week.

Oh I see, I never realised it was waiting like that.

Indeed, marionetteScriptFinished() is called, but nothing happens.... until we timeout.

Comment on attachment 829061 [details] Part 1. Since this is deeper than expected, r=me, but please file a follow-up bug to investigate whether it's: * an issue in marionette * an issue in the marionette js client * an issue in our use of the marionette js client and fix it

Merged. https://github.com/mozilla-b2g/gaia/commit/259cc53ad62489ab582142487f13e9df0984b808

Follow up bug https://bugzilla.mozilla.org/show_bug.cgi?id=947259