1170278 - cleanup talos test.py to remove tests we don't run anymore!

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8) (PTO back normal Nov 17)]

We can start removing tests we haven't run for a log time.

ts
ts_paint_cold
trobopan
tcheckerboard
tprovider
tp
tdhtml
tsvg
tsvg_opacity
tscroll
a11y
tdhtmlr
tsvgr
tscrollr

likewise we can remove the related files.

some other things to remove as well:
talos/places_generated_med
talos/places_generated_max

Comment 1

[Julien Pagès (:parkouss)]

Attachment: 1170278.patch

This is related to this bug, but not yet what is asked.

I wanted to clean up the way Talos tests are registered and used. The idea is that we register tests using a decorator, so the class definition and the registration are done at the same place.

Doing this, I noted some classes that were not registered (not present in the previous global "tests" variable) so I removed them.

I think it is interesting to do that because it is safer to create and register class at the same place - this way it is harder to forget one or the other when we make changes in this file.

Joel, if you are interested in this change, please carefuly review the patch - to be sure I did not removed too many classes or changed something in a wrong way. :)

Comment 2

[Joel Maher ( :jmaher ) (UTC -8) (PTO back normal Nov 17)]

Comment on attachment 8614014 [details] [diff] [review]
1170278.patch

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

I think this is good.  It also removed the nagging list of tests at the end!

::: talos/test.py
@@ +119,1 @@
>  class ts(TsBase):

we can remove ts, although other classes depend on it, so those need to be fixed.

@@ +154,1 @@
>  class ts_paint_cold(ts_paint):

we cna remove ts_paint-cold

@@ +197,1 @@
>  class trobopan(TsBase):

we can remove trobopan

@@ +214,1 @@
>  class tcheckerboard(TsBase):

we can remove tcheckerboard

@@ +230,1 @@
>  class tprovider(TsBase):

we can remove tprovider

@@ +590,1 @@
>  class tdhtml(PageloaderTest):

we can remove tdhtml

@@ -719,5 @@
> -    """ASAP mode"""
> -    preferences = {'layout.frame_rate': 0,
> -                   'docshell.event_starvation_delay_hint': 1,
> -                   'dom.send_after_paint_to_content': False}
> -    filters = [["ignore_first", [2]], ['median', []]]

we want to keep svgm, but remove svgr

Comment 3

[Julien Pagès (:parkouss)]

Attachment: 1170278-final.patch

There is some big changes here (a lot of removed hopefully obsolete stuf).

You can look at my git branch here, that might be easier to review since there are multiple commits:

https://github.com/mozilla/build-talos/compare/master...parkouss:remove-talos-tests

Note this should be the exact same changes.

I highly suggest trying this before merging. :)

Comment 4

[Joel Maher ( :jmaher ) (UTC -8) (PTO back normal Nov 17)]

Comment on attachment 8614603 [details] [diff] [review]
1170278-final.patch

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

you accidentally are removing tsvgm from the test definition as well as tsvgm.manifest from the svg/ directory.  Other than that, I like this patch!

::: talos/page_load_test/svgx/svgm.manifest
@@ -1,2 @@
> -# gearflowers image
> -http://localhost/page_load_test/svgx/gearflowers.svg

we still need svgm!

::: talos/test.py
@@ -703,5 @@
> -    tpcycles = 1
> -    tppagecycles = 25
> -
> -
> -class tsvgm(tsvg):

we need tsvgm

Comment 5

[Julien Pagès (:parkouss)]

Hey Joel,

In fact it was intentionnal since it looked like this test was not used. :)

It was not in the original global tests variables that was the list of available tests (from what I understood):

https://github.com/mozilla/build-talos/compare/master...parkouss:remove-talos-tests#diff-b6c648d279b20e3d39f4280194a56650L802

So it looks like it was not used to me. Tell me if I must bring it back.

Comment 6

[Joel Maher ( :jmaher ) (UTC -8) (PTO back normal Nov 17)]

oh, it should be used- I see it isn't.  lets leave it in and make sure it is usable- I can work on the mozharness and buildbot scripts to get it working.

thanks for pointing that out!

Comment 7

[Julien Pagès (:parkouss)]

Attachment: 1170278-final.patch

Ok, I made the changes.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8) (PTO back normal Nov 17)]

https://hg.mozilla.org/build/talos/rev/711f037bd269

Comment 9

[Joel Maher ( :jmaher ) (UTC -8) (PTO back normal Nov 17)]

I think we forgot the talos/page_load_test/svg/ directory- testing with that removed on my try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bd70ad0f9ae6

Comment 10

[Julien Pagès (:parkouss)]

Pushed fix: https://hg.mozilla.org/build/talos/rev/b55193016019