Closed Bug 664831 Opened 11 years ago Closed 11 years ago

disable tp4


(Release Engineering :: General, defect, P2)



(Not tracked)



(Reporter: armenzg, Assigned: armenzg)



(Whiteboard: [disable by June 30th])


(2 files)

We are aiming to disable tp4 by June 30th, 2011 on every branch except older release branches. Right now, tp4 & tp5 are running side by side.

Let's use this bug to track the work and make it clear for others.

I will bring this item on the Tuesday meetings until then and ask for anode's OK before doing so.
The date is just 2 weeks from now but it can be adjusted if there are blockers or we feel we don't have to wait the 2 weeks.
Whiteboard: [disable by June 30th]
per email with alice, we do *not* need a downtime/treeclosure for disabling tp4.
Priority: -- → P5
Assignee: nobody → armenzg
Priority: P5 → P3
Announced yesterday on developer's meeting and on dev.planning and dev.tree-management.

We are still on schedule for June 30th.
Depends on: 666711
Depends on: 666952
FTR we had to fix several small infra changes outside of releng:
* add tp5 to [1]
* add tp5 to tbpl's compare feature [2]
* change tp4 to tp on Try Chooser website's [3]

Remaining work:
* disable tp4 (this bug)
* remove tp4 from tbpl's compare feature once disabled on buildbot [4]

[2] bug 666711
[4] bug 666952
Attached patch disable tp4Splinter Review
Getting ready for this Thursday.
Attachment #542175 - Flags: review?(coop)
Attachment #542175 - Flags: review?(anodelman)
Comment on attachment 542175 [details] [diff] [review]
disable tp4

We may want to keep around the combined tp4/tp5 test, as we still have to get that rolled out to all branches (we still have branches that have not done the side-by-side running to create a viable baseline).  Might be better to create a TALOS_TP5_OPTS and a tp5_tests.
One thing you might have not noticed is that we have been running tp4/tp5 side by side on all branches! Not just mozilla-central & tracemonkey :)
Priority: P3 → P2
Attachment #542175 - Flags: review?(coop) → review+
This was again announced at the platform meeting without any objections.

anode, gentle poke wrt to review.
Comment on attachment 542175 [details] [diff] [review]
disable tp4

Looks like tp4 would still be running on:
BRANCHES['mozilla-2.0']['tp4_tests'] = (1, True, TALOS_TP4_OPTS, ALL_PLATFORMS)
BRANCHES['mozilla-2.1']['tp4_tests'] = (1, True, TALOS_TP4_OPTS, ALL_PLATFORMS)
BRANCHES['mozilla-1.9.1']['tp4_tests'] = (1, True, TALOS_TP4_OPTS, OLD_BRANCH_ALL_PLATFORMS)
BRANCHES['mozilla-1.9.2']['tp4_tests'] = (1, True, TALOS_TP4_OPTS, OLD_BRANCH_ALL_PLATFORMS)

I thought that this was to totally be done with tp4?

If we are truly killing tp4 then we should get rid of it, including 

TALOS_TP4_OPTS = {'plugins': 'zips/', 'pagesets': ['zips/',]}

    'tp4': GRAPH_CONFIG + ['--activeTests', 'tp4'],

and lines like:

BRANCHES['mozilla-central']['tp4_tests'] = (0, True, TALOS_TP4_OPTS, ALL_PLATFORMS)

Also, I'd prefer
+TALOS_TP_OPTS = {'plugins': 'zips/', 'pagesets': ['zips/']}
to be
+TALOS_TP_OPTS = {'plugins': 'zips/', 'pagesets': ['zips/',]}

Python can deal with lists in interesting ways, including interpreting a list of a single string entry into a list of individual characters in the string.  The comma forces the interpretation that we want.
Attachment #542175 - Flags: review?(anodelman) → review-
Comment on attachment 542175 [details] [diff] [review]
disable tp4

The plan is to leave tp4 running for older releases branches (1.9.1/1.9.2/2.0/2.1)

I believe the comma problem only happens when dealing with tuples but sure we can take it.

In the light of what I replied can I please have the r+?
Attachment #542175 - Flags: review- → review?(anodelman)
> Python can deal with lists in interesting ways, including interpreting a
> list of a single string entry into a list of individual characters in the
> string.  The comma forces the interpretation that we want.

Armen is correct, this only applies to tuples.

("foo") is the same as "foo"
["foo"] is ["foo"]
Attachment #542175 - Flags: review?(anodelman) → review+
Comment on attachment 542175 [details] [diff] [review]
disable tp4

Checked-in on default:
It will be picked up in today's reconfig.

I have left the try server enabled since roc is figuring out a regression on tp4.
Attachment #542175 - Flags: checked-in+
I think we need this to actually stop running tp4.
Attachment #543329 - Flags: review?
Attachment #543329 - Flags: review? → review+
Attachment #543329 - Flags: checked-in?
Comment on attachment 543329 [details] [diff] [review]
actually disable tp4

Landed and reconfiguring:
Attachment #543329 - Flags: checked-in? → checked-in+
Closed: 11 years ago
Resolution: --- → FIXED
Product: → Release Engineering
You need to log in before you can comment on or make changes to this bug.