Closed Bug 660124 Opened 9 years ago Closed 9 years ago

disable ts and txul for branches that have ts_paint and tpaint

Categories

(Release Engineering :: General, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jmaher, Assigned: armenzg)

References

Details

(Whiteboard: [buildfaster:p2])

Attachments

(2 files, 2 obsolete files)

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.
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.
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.
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
Txul is also called Twinopen.
yes, from 'chrome' and 'nochrome' (twinopen only).
(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).
Un-assign yourself once you get developer consensus on the newsgroups. Apologies if I missed it, I'm really behind on my newsgroup reading!
Assignee: nobody → jmaher
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.
(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]]
Whiteboard: [buildfaster:p2]
I will drive this to completion.
Assignee: jmaher → armenzg
Status: NEW → ASSIGNED
Priority: -- → P2
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
The patch didn't handle well project_branches.py so I started doing some refactoring in bug 633054 before getting to this bug.
I am moving the work of this bug to bug 659328.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 659328
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.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
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.
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.
Attachment #546005 - Attachment is obsolete: true
Attachment #571735 - Flags: review?(jmaher)
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?
Attachment #571735 - Flags: review?(jmaher) → review-
"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.
Attachment #571735 - Attachment is obsolete: true
Attachment #571769 - Flags: review?(jmaher)
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!
Attachment #571769 - Flags: review?(jmaher) → review+
This went live at 8:45 AM PDT.
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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
Attachment #572498 - Flags: review?(jmaher) → review+
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
Attachment #571769 - Attachment description: (passed test-masters.sh) repalce ts/twinopen for ts_paint/tpaint and some clean up → (passed test-masters.sh) replace ts/twinopen for ts_paint/tpaint and some clean up
Attachment #571769 - Flags: checked-in+
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
Attachment #572498 - Flags: checked-in+
(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.
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.