Closed Bug 648307 Opened 13 years ago Closed 13 years ago

Create a mobile Tp pageset with real mobile pages

Categories

(Release Engineering :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mfinkle, Assigned: jmaher)

References

Details

(Whiteboard: [mobile_unittests])

Attachments

(4 files, 7 obsolete files)

3.68 KB, patch
mozilla
: review+
mozilla
: checked-in+
Details | Diff | Splinter Review
2.59 KB, patch
anodelman
: review+
Details | Diff | Splinter Review
25.70 KB, patch
bear
: review+
mozilla
: checked-in+
Details | Diff | Splinter Review
959 bytes, patch
anodelman
: review+
Details | Diff | Splinter Review
I thought I filed this bug already, but can't find it.

We reduced the size of the Tp pageset for mobile so we can stop OOM'ing during the tests. However, the pageset is 100% desktop webpages. We need a pageset with mostly mobile pages for testing the mobile browser.

I have a pageset I have been using for Zippity that seems to work well. I'll post it.
bug 629503 reduced the Tp4 manifest size, but we need a whole new pageset
using the mobile only pages provided by mfinkle from the zippity tests and tools, I am able to run 22 mobile pages to completion in a tp4 style test a few times with no problems.  On the same machine I could not run tp4 with >4 pages.

I have a mobile_tp4.zip at http://people.mozilla.org/~jmaher/mobile_tp4.zip.  This needs to be downloaded to the {talos}/page_load_test/ directory and unzipped there so you have:
{talos}/page_load_test/mobile_tp4/{m.domain.com/...}
Assignee: nobody → jmaher
Attachment #524621 - Flags: review?(aki)
Attachment #524621 - Flags: feedback?(mark.finkle)
Status: NEW → ASSIGNED
Whiteboard: [mobile_unittests]
Comment on attachment 524621 [details] [diff] [review]
update mobile talos tp4 to use mobile only pages (1.0)

Yep, these are the pages I thought we were using. Sorry I dropped the ball getting us switched over to use the mobile-ish pageset.

Thanks Joel
Attachment #524621 - Flags: feedback?(mark.finkle) → feedback+
Comment on attachment 524621 [details] [diff] [review]
update mobile talos tp4 to use mobile only pages (1.0)

This cannot land until we have the appropriate factory changes, or all mobile tp4 runs will burn.
Attachment #524621 - Flags: review?(aki) → review+
is that something I can help write?  Otherwise, let me know when I can check this in.
I've pushed the mobile_tp4 pageset to bm-remote-talos-webhost-01 through 03, so the tegras should be good.  I still need to write a patch for the n900s.
Attached patch mobile_tp4 n900 configs (obsolete) — Splinter Review
Attachment #525529 - Flags: review?(jhford)
Comment on attachment 525529 [details] [diff] [review]
mobile_tp4 n900 configs

looks good.  At some point we should delete the n810 stuff altogether as there is 0 chance of using it again.
Attachment #525529 - Flags: review?(jhford) → review+
Attached patch mobile_tp4 n900 buildbotcustom (obsolete) — Splinter Review
Tested on smm.
Verified that the n900 downloaded+extracted+softlinked the tarball correctly.
Verified that I could get to various pages in the mobile_tp4 manifest in my desktop browser if I s,localhost,n900-070,

(The actual test run hung b/c it was using the un-patched mobile_tp4.manifest, so was looking for the tp4/ pageset; I think this level of testing should be acceptable.)
Attachment #525531 - Flags: review?(jhford)
Comment on attachment 525531 [details] [diff] [review]
mobile_tp4 n900 buildbotcustom

sounds good and should be easy to backout if needed.
Attachment #525531 - Flags: review?(jhford) → review+
Sweet.

Joel: we're now ready to land; we just need to coordinate landing the patches at approximately the same time.
Comment on attachment 524621 [details] [diff] [review]
update mobile talos tp4 to use mobile only pages (1.0)

http://hg.mozilla.org/build/talos/rev/be849ccbd6f3

Checked in with a horked -u (oops) but without the trailing newline, which may or may not have caused issues.
Attachment #524621 - Flags: checked-in+
http://hg.mozilla.org/build/talos/pushloghtml?changeset=be849ccbd6f3
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 524621 [details] [diff] [review]
update mobile talos tp4 to use mobile only pages (1.0)

backed out: http://hg.mozilla.org/build/talos/rev/683754ee01cc
Attachment #524621 - Flags: checked-in+
add all the pages from tp4 mobile pageset to the graph server.

as a side note, I noticed a duplicate entry in data.sql while testing this:
insert into machines values (NULL,1,0,"1.83","qm-pxp-try01",1,unix_timestamp());
Attachment #525760 - Flags: review?(anodelman)
Comment on attachment 525760 [details] [diff] [review]
graph server sql update for tp4m (1.0)

+insert into tests values (NULL,"tp4m_nochrome", "Tp4 Mobile",0,1,14);

Is this a nochrome test?  If it is then the 'pretty name' should indicate it, if not then it shouldn't be called tp4m_nochrome.
Attachment #525760 - Flags: review?(anodelman) → review-
updated to use tp4m instead of tp4.
Attachment #524621 - Attachment is obsolete: true
Attachment #525769 - Flags: review?(aki)
There will be chrome and nochrome versions, so I guess the pretty name should
be fixed
updated with nochrome in the description as well.
Attachment #525760 - Attachment is obsolete: true
Attachment #525770 - Flags: review?(anodelman)
Attachment #525770 - Flags: review?(anodelman) → review+
Depends on: 649774
Comment on attachment 525769 [details] [diff] [review]
update mobile talos tp4 to use mobile only pages (2.0)

Could you remove the last empty line in the manifest?
I don't know if it'll cause issues, but it would make me feel better.
Attachment #525769 - Flags: review?(aki) → review+
Attached patch jhford is gonna hate me (obsolete) — Splinter Review
N900s. Need to test.
Attachment #525531 - Attachment is obsolete: true
Attached patch tp4m n900 configs + remove n810s (obsolete) — Splinter Review
Attachment #525529 - Attachment is obsolete: true
Comment on attachment 525883 [details] [diff] [review]
jhford is gonna hate me

This is broken on staging.  I'm leaning towards landing tp4m for the tegras, and still running tp4 on the n900s til we get time to look at it.
Attachment #525883 - Attachment is obsolete: true
Attachment #525885 - Attachment is obsolete: true
From staging:

RETURN:send failed, graph server says:
RETURN:No test_name called 'tp4m_shutdown' can be found
RETURN:  File "/var/www/html/graphs2/server/pyfomatic/collect.py", line 264, in handleRequest
RETURN:    metadata = MetaDataFromTalos(databaseCursor, databaseModule, inputStream)
RETURN:  File "/var/www/html/graphs2/server/pyfomatic/collect.py", line 59, in __init__
RETURN:    self.doDatabaseThings(databaseCursor)
RETURN:  File "/var/www/html/graphs2/server/pyfomatic/collect.py", line 102, in doDatabaseThings
RETURN:    raise DatabaseException("No test_name called '%s' can be found" % self.test_name)
RETURN:
RETURN:
Other than ^^ I think we're good.  Let's aim for landing and reconfiging Tues morning; should be easy to add tp4m_shutdown to graphs sql (I think).
Attachment #525843 - Attachment is obsolete: true
Attachment #526445 - Flags: review?(bear)
Attachment #526445 - Flags: review?(bear) → review+
tp4 is hidden on both mozilla-aurora and Mobile, so you'll need to use &noignore=1 on tbpl to see the effect of this landing.
This will create a new column for Tp4 Mobile (and remove the perma-orange tp4 for Android Tegra), so that will show up by default.  Also, I'm going to land the non-n900 portions of attachment 526445 [details] [diff] [review].
bear, we might want to update your patch to include:
tp4m_shutdown
tp4m_shutdown_nochrome
(In reply to comment #37)
> bear, we might want to update your patch to include:
> tp4m_shutdown
> tp4m_shutdown_nochrome

Not sure what patch this is referring to?
tp4m_shutdown[_nochrome] gets run automatically when tp4m[_nochrome] runs, so if you're referring to attachment 526445 [details] [diff] [review] (sans n900 bits), that should still be good.
thanks aki, I just want to make sure all of the buildbot related patches for tp4m will with with shutdown.
Comment on attachment 526716 [details] [diff] [review]
add in tp4m_shutdown sql to graph server (1.0)

The shutdown tests do not require a pageset id.  You can compare to the existing entries for tp4.
Attachment #526716 - Flags: review?(anodelman) → review-
Comment on attachment 526716 [details] [diff] [review]
add in tp4m_shutdown sql to graph server (1.0)

I was incorrect about the pageset_id not being identified with the standard tp4 test.  r+.  Sorry for the confusion.
Attachment #526716 - Flags: review- → review+
Depends on: 650879
Comment on attachment 526445 [details] [diff] [review]
android tp4m configs, tested+unbitrotted

Landed the Android Tegra portions:
http://hg.mozilla.org/build/buildbot-configs/rev/4e43bbaa4927
Attachment #526445 - Flags: checked-in+
Comment on attachment 525769 [details] [diff] [review]
update mobile talos tp4 to use mobile only pages (2.0)

Landed the Android Tegra portions (not mobile.config):
http://hg.mozilla.org/build/talos/rev/43d49cf3ce13

When John Ford reconfigs tomorrow, we (Bear or I) need to update foopy*:/builds/talos-data/talos/ to get this change.

There may be some burning if we time this poorly, but since we're currently hidden and perma-orange, the damage should be minimal.
Attachment #525769 - Flags: checked-in+
On the Mobile tinderbox page; both tp4m and tp4m_nochrome have gone green at least once.
Status: REOPENED → RESOLVED
Closed: 13 years ago13 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.