838657 - Implement a spec reporter for perf runs

Status: RESOLVED FIXED

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

Description

[Julien Wajsberg [:julienw]]

In Bug 837139 we implemented a JSON reporter that shows a synthetic view of all suites.

It's also useful to have a more "human" reporter that shows results as soon as they're run.

Comment 1

[Caio Lima(:caiolima)]

I would like to start work in this bug. Wich are the requirements for this implementation?

I guess the better ideia is reuse the JSON implementation and show it on reporter. My doubts is:

1. Where this more "human" report will be done? Console/APP or something like that?

Comment 2

[Caio Lima(:caiolima)]

Julien, I've runned the test-perf from gaia and I've seen how the current implementation of results are. 
Could you give some guide where I need to work/study, in terms of code, to create a friendly interface using this the JSON output?

In a fast looking, I'll need to work with Marionette, Is it right?

Another question is, this JSON output is being stored in another place? I guess that It is being used by Datazilla, and that is the motivation to report as JSON, all right?

Comment 3

[Anthony Ricaud (:rik)]

You can start taking a look at 'tests/reporters/jsonmozperf.js'. This is the current JSON reporter. And you can create a new reporter based on this.

Datazilla would still use REPORTER=JSONMozPerf but tests ran manually could choose the new reporter that you create.

Comment 4

[Julien Wajsberg [:julienw]]

You don't need to work with mationette, all this part is already done. You just need to create a new reporter and specify it when you run the perf test.

Comment 5

[Caio Lima(:caiolima)]

(In reply to Anthony Ricaud (:rik) from comment #3)
> You can start taking a look at 'tests/reporters/jsonmozperf.js'. This is the
> current JSON reporter. And you can create a new reporter based on this.
> 
> Datazilla would still use REPORTER=JSONMozPerf but tests ran manually could
> choose the new reporter that you create.

Thank you for the direction!

Comment 6

[Caio Lima(:caiolima)]

I've done a first proposal in this pull request:

https://github.com/mozilla-b2g/gaia/pull/11566

But I have some doubts about this kind of implementation. It has a lot of code cloned from "jsonmozperf.js". 

1. What do you think about a reporter that can be more reusable or more generic?

2. Who can I require a review?

Comment 7

[Julien Wajsberg [:julienw]]

1. it should be possible to use inheritance to factorize all this
2. either anthony (:rik) or me

Comment 8

[Caio Lima(:caiolima)]

(In reply to Julien Wajsberg [:julienw] from comment #7)
> 1. it should be possible to use inheritance to factorize all this
So, I'm thinking in create a supertype that stores the success and failures as the super class and 2 especialized class (ConsoleMozPerf and JSONMozPerf) for the report function, because they differ only in this way.

There is no problem in change the JSONMozPerf, all right? I'm worry about this.

Comment 9

[Caio Lima(:caiolima)]

I have another newbie javascript implementation problem. What is the right way to import a specific script on gaia. I'm having problems with this. I want to make a function "exportable"

Comment 10

[Caio Lima(:caiolima)]

julienw or rik, could one of you review my new pull? It's on 


https://github.com/mozilla-b2g/gaia/pull/11566

I've created a BaseMozPerf that does the base job of a PerfReporter.

Waiting for review.

Comment 11

[Julien Wajsberg [:julienw]]

Caio: I added comments on the pull request.

Note that you should add a file as attachment, you can put the PR url inside this file, this makes it easier to request reviews.

Comment 12

[Caio Lima(:caiolima)]

Attachment: pull request link

Comment 13

[Caio Lima(:caiolima)]

Julien, I'm with some doubts about implementation. I've commentend them.

Comment 14

[Caio Lima(:caiolima)]

Attachment: pull request link

Comment 15

[Caio Lima(:caiolima)]

Remember to assign this bug for me.

Comment 16

[Caio Lima(:caiolima)]

I've modified the code.

Ps.: When I push a new code, do I need to say it here in bugzilla?

Comment 17

[Julien Wajsberg [:julienw]]

(In reply to Caio Lima(:caiolima) from comment #16)
> 
> Ps.: When I push a new code, do I need to say it here in bugzilla?

It depends. Usually, when your code is ready to review, you need to flip the flag "review" to "?" on your attachment, and add the bugzilla handle for the reviewer (:julien for me, :rik for Anthony).

When we review it, we usually can do 2 different things:
* either we remove the "?" or we put a "-", and in that case, you just need to add the "?" again when you're ready;
* or we don't change anything and in that case you just need to comment here or on github to say you're ready to get reviewed again.

Comment 18

[Anthony Ricaud (:rik)]

Comment on attachment 792537 [details] [review]
pull request link

https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Platform/Gaia/Hacking#Submitting_a_patch has some information about submitting a patch.

You don't have to tell every time you push code. Just re-setting the review flag is enough.

Comment 19

[Julien Wajsberg [:julienw]]

Comment on attachment 792537 [details] [review]
pull request link

I'll review this tomorrow if that's ok for you, I'me quite full for today already :/

Comment 20

[Caio Lima(:caiolima)]

(In reply to Julien Wajsberg [:julienw] from comment #19)
> Comment on attachment 792537 [details] [review]
> pull request link
> 
> I'll review this tomorrow if that's ok for you, I'me quite full for today
> already :/

No problem for me. :)

Comment 21

[Caio Lima(:caiolima)]

julien, ping review

Comment 22

[Julien Wajsberg [:julienw]]

Attachment: new PR from Caio

We can resume work on this now.

Comment 23

[Hubert Figuiere [:hub]]

Maybe I should take over this one? I didn't realise this bug existed.

Comment 24

[Julien Wajsberg [:julienw]]

Yep, please do :)

Comment 25

[Hubert Figuiere [:hub]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19738

Comment 26

[Julien Wajsberg [:julienw]]

Comment on attachment 8430285 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19738

I wonder if we should remove the wrapping [] characters but this looks good enough for me, so r=me with a small nit.

Comment 27

[Hubert Figuiere [:hub]]

with nit addressed

merged
https://github.com/mozilla-b2g/gaia/commit/ca0390821ff924a3a8a9810be3a73dfbfa679d66