Closed
Bug 660124
Opened 13 years ago
Closed 13 years ago
disable ts and txul for branches that have ts_paint and tpaint
Categories
(Release Engineering :: General, defect, P2)
Release Engineering
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jmaher, Assigned: armenzg)
References
Details
(Whiteboard: [buildfaster:p2])
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
jmaher
:
review+
armenzg
:
checked-in+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jmaher
:
review+
armenzg
:
checked-in+
|
Details | Diff | Splinter Review |
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•13 years ago
|
||
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•13 years ago
|
||
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.
Assignee | ||
Comment 3•13 years ago
|
||
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•13 years ago
|
||
Txul is also called Twinopen.
Reporter | ||
Comment 5•13 years ago
|
||
yes, from 'chrome' and 'nochrome' (twinopen only).
Comment 6•13 years ago
|
||
(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•13 years ago
|
||
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
Comment 8•13 years ago
|
||
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]]
Updated•13 years ago
|
Whiteboard: [buildfaster:p2]
Assignee | ||
Comment 10•13 years ago
|
||
I will drive this to completion.
Assignee: jmaher → armenzg
Status: NEW → ASSIGNED
Priority: -- → P2
Assignee | ||
Comment 11•13 years ago
|
||
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
Assignee | ||
Comment 12•13 years ago
|
||
The patch didn't handle well project_branches.py so I started doing some refactoring in bug 633054 before getting to this bug.
Assignee | ||
Comment 13•13 years ago
|
||
I am moving the work of this bug to bug 659328.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → DUPLICATE
Assignee | ||
Comment 14•13 years ago
|
||
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 → ---
Reporter | ||
Comment 15•13 years ago
|
||
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.
Assignee | ||
Comment 16•13 years ago
|
||
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)
Reporter | ||
Comment 17•13 years ago
|
||
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-
Assignee | ||
Comment 18•13 years ago
|
||
"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)
Reporter | ||
Comment 19•13 years ago
|
||
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+
Assignee | ||
Comment 20•13 years ago
|
||
This went live at 8:45 AM PDT.
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•13 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 21•13 years ago
|
||
Attachment #572498 -
Flags: review?(jmaher)
Reporter | ||
Comment 22•13 years ago
|
||
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+
Assignee | ||
Comment 23•13 years ago
|
||
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+
Assignee | ||
Comment 24•13 years ago
|
||
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+
Assignee | ||
Comment 25•13 years ago
|
||
(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: 13 years ago → 13 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
You need to log in
before you can comment on or make changes to this bug.
Description
•