1633890 - [perfdocs] Make the raptor test descriptions less redundant

Status: NEW

(Testing :: Performance, task, P3)

Description

[Greg Mierzwinski [:sparky]]

This bug is for making the raptor test descriptions in the perfdocs less redundant.

Comment 1

[Greg Mierzwinski [:sparky]]

Most of the test descriptions are quite redundant, to remove a lot of the redundancy we can provide a suite-level description (like for benchmarks/scenario) instead of per-test descriptions: https://firefox-source-docs.mozilla.org/testing/perfdocs/raptor.html

That said, I think we should rethink how we display this information. I don't have a solution for that yet.

Comment 2

[Greg Mierzwinski [:sparky]]

You can find existing test descriptions for Raptor in this folder: https://searchfox.org/mozilla-central/source/testing/raptor/raptor/perfdocs

Comment 3

[Alexandru Ionescu (needinfo me) [:alexandrui]]

They are already broken by the website tested which is the least redundant form. We can break by framework (webextension/browsertime). Make a universal description for the tests and then just enumerate the websites.

[pageload-type] desktop/android page-load performance tests on [browsers] using [framework] for the following websites:
- list

But we intent to add more information for every test, don't we? Which will make it less redundant. I am not keen on making this change and then get back to the initial form to add more information on every test. This means updates to the code.

You have a point though, and 'think we can revisit this after we add the extra information we plan to.

Comment 4

[Myeongjun Go]

Hello sparky :)
I am considering how to revise it by referring to the contents.
If I make universal description about desktop, there are already four cases(description) as below [0] [1].
Sure, An error occurs in the example below. There can be only one description [2]. It will have a widen range modified code.
I need to confirm my thoughts.
If the intention is different, It may be hassle, but could you show me an example?
As always, thank you.

[0]

suites:
    desktop:
        description: "Desktop page-load performance tests on firefox, chrome, and chromium using WebExtension for the following websites"
        tests:
            raptor-tp6-amazon: " "
            raptor-tp6-google: " "
            raptor-tp6-youtube: " "
            ...
        description: "Desktop page-load performance tests on chrome, and chromium using WebExtension for the following website"
        tests:
            raptor-tp6-facebook: " "
        description: "Desktop page-load performance test on firefox with binast recordings and flag enabled using WebExtension ..."
        tests:
            raptor-tp6-binast-instagram-firefox: " "
        description: "Desktop page-load performance test using Browsertime for the following websites"
        tests:
            amazon: " "
            bing-search: " "
            ...
    mobile:
        ...

[1] (origin)
https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/testing/raptor/raptor/perfdocs/config.yml

[2]
https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/tools/lint/perfdocs/generator.py#62

Comment 5

[Greg Mierzwinski [:sparky]]

Hi Myeongjun :)

Thanks for making progress on this issue! So here's a method that you might want to take a look at: https://bugzilla.mozilla.org/show_bug.cgi?id=1633909#c0

We will have to change how the descriptions are setup and built in Perfdocs. Here's what I'm thinking will need to be done:

1. Add a new method (called something like build_test_description) to the framework gatherer (RaptorGatherer and FrameworkGatherer), that will be responsible for providing a documentation chunk for that test.
   1. The build_test_description would take the test name, and a given description if it exists as arguments, and return either (1) a documentation chunk (like we have now, - a block of text), or (2) a new file that contains the test description information.
2. Use this method here to make the framework gatherers responsible for providing the documentation chunk (for a given test): https://searchfox.org/mozilla-central/source/tools/lint/perfdocs/generator.py#100-112
   1. To get access to the framework gatherer from the generator you'll need to make a method in the verifier and gatherer that would return the correct framework gatherer.

It may be best to implement (1) and (2) and keep the same documentation format for now. We can change the structure afterwards using this new feature.

What I'm thinking we can do with it is return nothing when asked to build a test description, and when nothing is returned, just add the test to a list of tests that will be shown under the suite description (like in the bug description I linked above). Otherwise, if a description is returned and it isn't a new page, add it underneath the test name in that list.

Let me know if anything here isn't clear or you have any questions! :)

Comment 6

[Alexandru Ionescu (needinfo me) [:alexandrui]]

Sparky, I'm thinking we could standardize the way the config is built as this framework based approach seems fairly simple to make structure app (perfdocs) wide. Doing this will be simpler to add descriptions without missing something (because the structure of the config will be the same as the one in html) and will save us extra processing in the code (pretty error prone).

Myeongjun, this is a pretty nasty task. It is a great idea to work on it, but you can expect difficulties or change of plans as we discover better solutions on the go. But it is ok if it happens that, we also had some hard times developing this. Good luck!

Comment 7

[Greg Mierzwinski [:sparky]]

(In reply to Alexandru Ionescu (needinfo me) :alexandrui from comment #6)

Sparky, I'm thinking we could standardize the way the config is built as this framework based approach seems fairly simple to make structure app (perfdocs) wide. Doing this will be simpler to add descriptions without missing something (because the structure of the config will be the same as the one in html) and will save us extra processing in the code (pretty error prone).

The config is already standardized but I see what you mean. Although, you should provide a more concrete example of what you are proposing because it's not very clear.

I agree that we could change the config structure to better reflect the HTML page, but it is already very close to mirroring it. Regardless of this point, we should still implement the two changes in comment #5 (1, and 2) so that we can let the framework classes handle their own descriptions - it'll get rid of the "browsertime" from the main code which shouldn't be there since it's framework-specific.

We can build off this in any config changes that come afterwards.

Comment 8

[Myeongjun Go]

Thanks for the review :)
I'll analyze it based on the content and write the code.
Please let me know anytime if there are any additional changes or details.

Comment 9

[Myeongjun Go]

Hello sparky :)
I'm looking at the existing code whenever I have time. There's something I need to confirm.
Should I make a new file in the form that you told me through above link[0]?
And, Should I bring the contents of this file through a new function and attach it under the current page?
Then can you tell me about the new file format(e.g xml)?

1. The `build_test_description` would take the test name, and a given description if it exists as arguments, and return either (1) a documentation chunk (like we have now, - a block of text), or (2) a new file that contains the test description information.

I have one more question. Can you explain a little bit more about the arguments in the new function?

If this direction is right, I'll start developing it.

[0]
https://bugzilla.mozilla.org/show_bug.cgi?id=1633909#c0

Comment 10

[Greg Mierzwinski [:sparky]]

Hi Myeongjun, I hope all is well for you! :)

No need to worry about changing the actual content of the descriptions file or creating a new one. We'll do that afterwards. I like your idea of XML though, that could be something interesting to look into after!

I've filed another bug for you specifically for this task (bug 1637058). What you need to do here should be clearer based on that bug - let me know if it's clearer now and/or if you need more information!