660124 - disable ts and txul for branches that have ts_paint and tpaint

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

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

we should disable ts and txul for branches where we are testing with tpaint and ts_paint.  This will free up machine resources and let us focus on the more realistic numbers for end users.

Comment 1

[Jim Mathies [:jimm]]

There are differences in the results between the paint and the txul tests. I'd suggest we mark this as won't fix. That data could come in handy some day.

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

We will happily disable old and unloved tests, but some developer consensus would be good first. The newsgroups seems like the right place to do that.

Comment 3

[Armen [:armenzg]]

paint_tests (ts_paint and tpaint) are enabled everywhere except for 1.9.1 and 1.9.2 (there is also addontester and addonbaselinetester but these are different).

I can't find txul at all.

By removing ts are you referring to remove ts from the 'chrome' set? or more than that?
http://hg.mozilla.org/build/buildbot-configs/file/tip/mozilla-tests/config.py#l45

Comment 4

[Timothy Nikkel (:tnikkel)]

Txul is also called Twinopen.

Comment 5

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

yes, from 'chrome' and 'nochrome' (twinopen only).

Comment 6

[Shawn Wilsher :sdwilsh]

(In reply to comment #1)
> There are differences in the results between the paint and the txul tests. I'd
> suggest we mark this as won't fix. That data could come in handy some day.
How?  Those tests were designed to see when the browser would be usable for a user when it was started, or when a new window was opened.  They were not doing a good job of that, and the paint tests are much better.

Keeping those tests running is not free.  They provide little use at best in their current form (which is why we wanted the paint tests in the first place).

Comment 7

[Chris AtLee [:catlee]]

Un-assign yourself once you get developer consensus on the newsgroups. Apologies if I missed it, I'm really behind on my newsgroup reading!

Comment 8

[Dão Gottwald [:dao]]

Given bug 661918 comment 12 and below, I think ts_paint and tpaint need to be modified to wait for the first paint event after the load event, rather than just some paint event.

Comment 9

[cmtalbert]

(In reply to comment #1)
> There are differences in the results between the paint and the txul tests.
> I'd suggest we mark this as won't fix. That data could come in handy some
> day.

I second Shawn's note in comment 6.  It's been my understanding that the txul/ts and txul-paint/ts-paint results have tracked each other quite nicely since we started measuring txul-paint/ts-paint. One example is here [1]. 

What is it that the old style txul/ts gives you that the other doesn't?  What does the old style measure that is not measured by the new style?

We need a better justification here because we're talking about turn around times for every checkin for every developer.  I'm happy to keep running these tests if they are useful, don't misunderstand me.  But, I want to be very certain we know why they are useful and why we should continue to expend the resources now that we have the new txul-paint and ts-paint tests.

[1]: http://graphs.mozilla.org/#tests=[[16,1,351],[83,1,354],[83,1,355],[83,1,356],[16,1,353],[16,1,354],[83,1,359]]

Comment 10

[Armen [:armenzg]]

I will drive this to completion.

Comment 11

[Armen [:armenzg]]

Attachment: [not tested] remove twinopen from 'chrome' for every branch except 1.9.2

We removed txul/twinopen from nochrome some time ago in bug 661897.

This patch removes twinopen from 'chrome' for every branch except 1.9.2

Comment 12

[Armen [:armenzg]]

The patch didn't handle well project_branches.py so I started doing some refactoring in bug 633054 before getting to this bug.

Comment 13

[Armen [:armenzg]]

I am moving the work of this bug to bug 659328.

Comment 14

[Armen [:armenzg]]

Let's get this done on this bug without mixing with the other one.

jmaher you say we're ready for this. Can you please restate what we want to enable/disable? which branches?

Thanks.

Comment 15

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

we need to remove ts and twinopen from the configs for all branches.  We have been running both in parallel for months now.

Here are the suites that have ts and twinopen:

    'chrome': {
        'enable_by_default': True,
        'suites': GRAPH_CONFIG + ['--activeTests', 'tscroll:a11y:ts:tdhtml:tsspider', '--mozAfterPaint'],
        'options': ({}, NO_MAC),
    },
    'chrome_mac': {
        'enable_by_default': True,
        'suites': GRAPH_CONFIG + ['--activeTests', 'tscroll:ts:tdhtml:twinopen:tsspider', '--mozAfterPaint'],
        'options': ({}, MAC_ONLY),
    },
    'chrome_twinopen': {
        'enable_by_default': False,
        'suites': GRAPH_CONFIG + ['--activeTests', 'tscroll:a11y:ts:tdhtml:twinopen:tsspider', '--mozAfterPaint'],
        'options': ({}, NO_MAC),
    },


we should make them look like:

    'chrome': {
        'enable_by_default': True,
        'suites': GRAPH_CONFIG + ['--activeTests', 'tscroll:a11y:ts_paint:tdhtml:tsspider', '--mozAfterPaint'],
        'options': ({}, NO_MAC),
    },
    'chrome_mac': {
        'enable_by_default': True,
        'suites': GRAPH_CONFIG + ['--activeTests', 'tscroll:ts_paint:tdhtml:tpaint:tsspider', '--mozAfterPaint'],
        'options': ({}, MAC_ONLY),
    },
    'chrome_twinopen': {
        'enable_by_default': False,
        'suites': GRAPH_CONFIG + ['--activeTests', 'tscroll:a11y:ts_paint:tdhtml:tpaint:tsspider', '--mozAfterPaint'],
        'options': ({}, NO_MAC),
    },

and then remove the paint suite:
    'paint': {
        'enable_by_default': True,
        'suites': GRAPH_CONFIG + ['--activeTests', 'ts_paint:tpaint', '--setPref', 'dom.send_after_paint_to_content=true'],
        'options': ({}, ALL_PLATFORMS),
    },


this will be the case for all branches except 1.9.2.  On 1.9.2 we already are running the old_chrome vs chrome, so we shouldn't have to change anything.

Comment 16

[Armen [:armenzg]]

Attachment: (passed test-masters.sh) raplce ts, twinopen for ts_paint and some clean up

I assume the following patch after this lands would be to disable paint everywhere, right?

Let me know if you want to separate the clean up code from this patch.

Comment 17

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

Comment on attachment 571735 [details] [diff] [review]
(passed test-masters.sh) raplce ts, twinopen for ts_paint and some clean up

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

::: mozilla-tests/config.py
@@ +152,5 @@
>          'options': ({}, NO_MAC),
>      },
>      'chrome_mac': {
>          'enable_by_default': True,
> +        'suites': GRAPH_CONFIG + ['--activeTests', 'tscroll:ts_painttdhtml:tsspider', '--mozAfterPaint'],

twinopen -> tpaint, so this should be:
tscroll:ts_paint:tpaint:tdhtml:tsspider

@@ -159,5 @@
> -    'chrome_twinopen': {
> -        'enable_by_default': False,
> -        'suites': GRAPH_CONFIG + ['--activeTests', 'tscroll:a11y:ts:tdhtml:twinopen:tsspider', '--mozAfterPaint'],
> -        'options': ({}, NO_MAC),
> -    },

not sure why this is removed unless we don't run it anymore

@@ -270,5 @@
> -    'old_chrome': {
> -        'enable_by_default': False,
> -        'suites': GRAPH_CONFIG + ['--activeTests', 'tscroll:a11y:ts:tdhtml:tsspider'],
> -        'options': ({}, NO_MAC),
> -    },

are we ready to remove old_chrome?

Comment 18

[Armen [:armenzg]]

Attachment: (passed test-masters.sh) replace ts/twinopen for ts_paint/tpaint and some clean up

"chrome_twinopen" existed from when we removed twinopen from "chrome" because we started running ts_paint:tpaint in the "paint" set. We don't need to run it anymore. I also removed old_chrome_twinopen as old_chrome will now runs twinopen again.

I would like to keep the "paint" set for now (I don't want chrome to be increasing the end-to-end times by adding ts and tpaint to it) and run ts_paint:tpaint in there only (that is why I remove it from "chrome").
BTW are we supposed to run ts_paint and tpaint with --mozAfterPaint?
Can ts_paint and tpaint be run in ALL_PLATFORMS? including mac?

What is the output of this patch?
- remove old_chrome_twinopen and chrome_twinopen which were not being used (no-op)
- remove ts from "chrome" and "chrome_mac" because they run inside of "paint"
- for 1.9.2, move "twinopen" back into "old_chrome"
- for 1.9.2, replace old_chrome_twinopen for old_chrome

I am glad we are doing this clean up.

Comment 19

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

Comment on attachment 571769 [details] [diff] [review]
(passed test-masters.sh) replace ts/twinopen for ts_paint/tpaint and some clean up

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

thanks, this looks great and the comments are very helpful!

Comment 20

[Armen [:armenzg]]

This went live at 8:45 AM PDT.

Comment 21

[Armen [:armenzg]]

Attachment: we don't need to run the "paint" set since "chrome/chrome_mac" takes care of it

Comment 22

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

Comment on attachment 572498 [details] [diff] [review]
we don't need to run the "paint" set since "chrome/chrome_mac" takes care of it

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

nice and simple

Comment 23

[Armen [:armenzg]]

Comment on attachment 571769 [details] [diff] [review]
(passed test-masters.sh) replace ts/twinopen for ts_paint/tpaint and some clean up

http://hg.mozilla.org/build/buildbot-configs/rev/00cc766972ca

Comment 24

[Armen [:armenzg]]

Comment on attachment 572498 [details] [diff] [review]
we don't need to run the "paint" set since "chrome/chrome_mac" takes care of it

http://hg.mozilla.org/build/buildbot-configs/rev/d69f802a227d

Comment 25

[Armen [:armenzg]]

(In reply to Armen Zambrano G. [:armenzg] - (off Friday 4th) from comment #24)
> Comment on attachment 572498 [details] [diff] [review] [diff] [details] [review]
> we don't need to run the "paint" set since "chrome/chrome_mac" takes care of
> it
> 
> http://hg.mozilla.org/build/buildbot-configs/rev/d69f802a227d

This second change went live at 9:35 AM PDT.