717685 - Talos config files should permit a generic "tests" section that the other tests inherit from

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Jeff Hammel]

Currently, each test suite in talos has a bunch of information in
e.g. sample.config that is repeated for each set of tests:

http://hg.mozilla.org/build/talos/file/4887e3aebe37/talos/sample.config#l99

Instead, there should be a generic section that *all* tests inherit
from, but that individual test suites can override

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: create a basetest which has default values (1.0)

as discussed briefly in IRC, we should have a much simpler manifest solution and the first step is to create a base class.  I went with a new section vs another test inside of tests.  

I am not sure if we want this in run_tests.py, but my looking at PerfConfigurator.py seemed as though it would be a lot of work.  This is a first stab, if we like it great, if not, back to the drawing board.

Comment 2

[Jeff Hammel]

Comment on attachment 589961 [details] [diff] [review]
create a basetest which has default values (1.0)

+# Prototype function, this should be in PerfConfigurator, so our resulting .yml file has a single bit  

Not really sure what this means, nor am I convinced in the long term that it should be in PerfConfigurator, as I would ideally like to combine the configuration stuff from PerfConfigurator and run_tests (https://bugzilla.mozilla.org/show_bug.cgi?id=704654)

The function itself looks fine.

The rest of the patch looks fine.  I'm happy of any patch that deletes more code than it adds and provides the same functionality. We will need a follow-up for being able to modify parameters of basetest directly, but I'm happy to take that as a subsequent step.

Also, testbase vs. basetest?  I don't really care, tbh, but will go with popular opinion.

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 589961 [details] [diff] [review]
create a basetest which has default values (1.0)

This looks great to me. One thought - I know we're planning to clean up/consolidate many of the .config files, but in the mean time, will this be problematic if a config file other than sample.config is specified to PerfConfigurator?

In terms of putting this in PrefConfigurator vs. run_tests, I'm with Jeff that these should be combined in the not too distant future.

For naming, it hardly matters to me, but basetest seems a little clearer ("base" as a modifier for "test").

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: create a basetest which has default values (2.0)

tested with other config files as well as commandline options.  This seems to work well, doing a staging run overnight;

Comment 5

[Jeff Hammel]

Comment on attachment 590089 [details] [diff] [review]
create a basetest which has default values (2.0)

+  tests = useBaseTestDefaults(yaml_config.get('basetest', []), tests)

I probably should have caught this before, but shouldn't this be yaml_config.get('basetest', {}) ?

+# Prototype function, this should be in PerfConfigurator, so our resulting .yml file has a single bit
Again, not sure if this comment is necessary

Other than those, looks good!

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

http://hg.mozilla.org/build/talos/rev/0ee2d31e29c8