Document and clean up mobile testing infrastructure

RESOLVED FIXED

Status

Release Engineering
General
RESOLVED FIXED
9 years ago
5 years ago

People

(Reporter: aki, Assigned: aki)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

9 years ago
To facilitate a shared environment.

1) users/asasaki_mozilla.com/mobile-configs merge -> buildbot-configs repo
2) buildbotcustom patch for unittest downloads
3) all wikis are up-to-date and grok-able by team
4) send production maemkit patch to qa/maemkit repo
5) make sure tip in talos-maemo repo is good

Not necessarily in that order.
(Assignee)

Updated

9 years ago
Depends on: 502761, 499334
(Assignee)

Comment 1

9 years ago
Created attachment 389739 [details] [diff] [review]
allow for wildcard wget in DownloadFile

For Maemo unittests, I download both the fennec tarball and xulrunner packaged tests tarball.  There currently isn't a logical way to determine the xulrunner test tarball filename from the fennec tarball filename.  Rather than hardcode or try to figure out how to poll for both files, I'm wgetting a wildcard.

To make this workable, I'm renaming the resulting test tarball tests.tar.bz2 so I can untar without a similar wildcard.  This patch enables this behavior.

This has been running for quite a few weeks (months?) on mobile-master and I'd like to check it in.  If, however, there is a more elegant solution, I'm open to trying that.
Attachment #389739 - Flags: review?(catlee)

Updated

9 years ago
Attachment #389739 - Flags: review?(catlee) → review+
(Assignee)

Comment 3

9 years ago
Status:

1) still working in my user repo.  I've split out a config.py for branch work though, which should make it easier for other people to work on.

2) patch is attached and r+'ed; waiting to land post-releases

3) wikis written. need to double check up-to-dateness and have them reviewed by others on team.

4) checked in my maemkit patch

5) talos-maemo tip is good as of now.

So: need to merge mobile-configs, need to land buildbotcustom patch, need to have wikis reviewed.
(Assignee)

Comment 4

9 years ago
Comment on attachment 389739 [details] [diff] [review]
allow for wildcard wget in DownloadFile

No longer needed now that bug 516466 is fixed.
Attachment #389739 - Attachment is obsolete: true
(Assignee)

Comment 5

9 years ago
Created attachment 402202 [details] [diff] [review]
merge mobile, mobile-staging back to buildbot-configs
Attachment #402202 - Flags: review?(bhearsum)
(Assignee)

Comment 6

9 years ago
Comment on attachment 402202 [details] [diff] [review]
merge mobile, mobile-staging back to buildbot-configs

Basically removing mobile and mobile-staging from buildbot-configs, and adding in the contents from http://hg.mozilla.org/users/asasaki_mozilla.com/mobile-configs

I know it's not pretty.  Welcoming comments.  I imagine there will be some followup bugs (e.g. DownloadFile).
Attachment #402202 - Flags: review?(anodelman)
(Assignee)

Updated

9 years ago
Attachment #402202 - Flags: review?(catlee)
Comment on attachment 402202 [details] [diff] [review]
merge mobile, mobile-staging back to buildbot-configs

I'm not going to give this a nit picky review since it's already running in production. This stuff really needs to be fixed though:
* factories.py stuff needs to be integrated into buildbotcustom
* please use the existing FtpPoller, buildbotcustom.changes.ftppoller, duplication--
Attachment #402202 - Flags: review?(bhearsum) → review-
(Assignee)

Comment 8

9 years ago
The two ftppoller.py's are fairly divergent.  Should I port the diffs into buildbotcustom.changes.ftppoller?  I'd be mainly concerned with breaking the release ftppoller, though hopefully we'd catch that with testing and reviews.

Also, for factories.py (or at least the bulk of it), any objections to creating a new mobiletestfactory.py under process/?
(In reply to comment #8)
> The two ftppoller.py's are fairly divergent.  Should I port the diffs into
> buildbotcustom.changes.ftppoller?  I'd be mainly concerned with breaking the
> release ftppoller, though hopefully we'd catch that with testing and reviews.

I hadn't realized just how different they are until now. At the very least, this FtpPoller needs to go in buildbotcustom.

> Also, for factories.py (or at least the bulk of it), any objections to creating
> a new mobiletestfactory.py under process/?

WFM. Be careful of import/reload issues, though.
Comment on attachment 402202 [details] [diff] [review]
merge mobile, mobile-staging back to buildbot-configs

Mostly the same objections as Ben.  All the factory type stuff should be in buildbotcustom where possible.

A few style nits:

- In general we shouldn't have commented out code checked in.  e.g.

    #class MobileParseTestLog(ShellCommandReportTimeout):

- Using arrays for commands where possible can be cleaner, e.g.

    command='rm -rf %s fennec* /home/user/.mozilla /root/.mozilla /tools/.mozilla release' % self.binaryDir,

    vs.

    command=['rm', '-rf', self.binaryDir, 'fennec*', '/home/user/.mozilla',
    '/root/.mozilla', '/tools/.mozilla', 'release']

- We don't normally have import/reloads in the middle of a file

- There's quite a bit of code duplication, or things that look very similar to existing code, like the ftp poller and all the perfrunner code.  If it's too much work to unify these now, please file bugs to make sure we try to at some point.

- Hacking kwargs in contructors is a bit confusing.  Instead of

    if command:
        kwargs['command'] = command
    else:
        kwargs['command'] = 'foo'

    ShellCommand.__init__(self, **kwargs)

  you can do

    if not command:
        command = 'foo'

    ShellCommand.__init__(self, command=command, **kwargs)


I really like the vim folding tags!  We could probably use more of those.
Attachment #402202 - Flags: review?(catlee) → review-
(Assignee)

Comment 11

9 years ago
Created attachment 403531 [details] [diff] [review]
configs patch with factories moved to buildbotcustom

patch 1 of 2.
Attachment #402202 - Attachment is obsolete: true
Attachment #403531 - Flags: review?(bhearsum)
Attachment #402202 - Flags: review?(anodelman)
(Assignee)

Comment 12

9 years ago
Created attachment 403532 [details] [diff] [review]
mobiletestfactory/mobileftpparser

patch 2 of 2.
Attachment #403532 - Flags: review?(bhearsum)
(Assignee)

Updated

9 years ago
Attachment #403532 - Flags: review?(catlee)
(Assignee)

Updated

9 years ago
Attachment #403531 - Flags: review?(catlee)
(Assignee)

Comment 13

9 years ago
So I've got the factories split to process/mobiletestfactory.py, and the "duplicate but different" ftppoller moved to changes/mobileftppoller.py.

I've gotten rid of the perfrunner.py by using the methods (or similar) that the new TalosFactory is using for MozillaWget and InstallTarBz2.

(In reply to comment #10)
> - In general we shouldn't have commented out code checked in.

Removed, other than the commented out tpan/tzoom tests.
I can remove those too, but I think those are just temporarily removed (depending on when dev/qa can get those tests working again).

> - Using arrays for commands where possible can be cleaner, e.g.
> 
>     command='rm -rf %s fennec* /home/user/.mozilla /root/.mozilla
> /tools/.mozilla release' % self.binaryDir,
> 
>     vs.
> 
>     command=['rm', '-rf', self.binaryDir, 'fennec*', '/home/user/.mozilla',
>     '/root/.mozilla', '/tools/.mozilla', 'release']

Right.
As mentioned in person, the arrays interpret their contents literally, and don't expand backticks or wildcards.  It would be good to get rid of the wildcards entirely, but we're not there yet.

> - We don't normally have import/reloads in the middle of a file

Removed once perfrunner was removed.  Also, I think it's apparent that I concatenated two files together to create factories.py =)

> - There's quite a bit of code duplication, or things that look very similar to
> existing code, like the ftp poller and all the perfrunner code.  If it's too
> much work to unify these now, please file bugs to make sure we try to at some
> point.

Yeah.  As mentioned in comment #8, ftppoller and mobileftppoller are fairly divergent.  Perhaps the solution is to open up sendchanges from MPT to MV and add mobile-master to the list of sendchange masters, and I wouldn't need to use mobileftppoller anymore.

> - Hacking kwargs in contructors is a bit confusing.

Fixed.

> I really like the vim folding tags!  We could probably use more of those.

Cool.  Hopefully the non-folding vim users (and emacs/other editor users) don't find 'em distracting, but I think they're fairly low profile.

I have run into the issue where people who don't understand them put code in the wrong block, or where they get moved around to the wrong places in refactoring/copy-pastes, but for the most part I've found them more useful than not.

Updated

9 years ago
Attachment #403532 - Flags: review?(catlee) → review+

Updated

9 years ago
Attachment #403531 - Flags: review?(catlee) → review+
Attachment #403531 - Flags: review?(bhearsum) → review?
Comment on attachment 403531 [details] [diff] [review]
configs patch with factories moved to buildbotcustom

Again, no line-by-line review, but I'm happy with where everything is living, and I don't see any reason you would hit reload() issues with this.
Comment on attachment 403532 [details] [diff] [review]
mobiletestfactory/mobileftpparser

Again, no line-by-line review, but I'm happy with where everything is living, and I don't see any reason you would hit reload() issues with this.
Attachment #403532 - Flags: review?(bhearsum) → review?
Attachment #403531 - Flags: review?
Attachment #403532 - Flags: review?
(Assignee)

Comment 16

9 years ago
Comment on attachment 403531 [details] [diff] [review]
configs patch with factories moved to buildbotcustom

http://hg.mozilla.org/build/buildbot-configs/rev/d73ddfc825dc
(Assignee)

Updated

9 years ago
Attachment #403531 - Flags: checked-in+
(Assignee)

Comment 17

9 years ago
Comment on attachment 403532 [details] [diff] [review]
mobiletestfactory/mobileftpparser

http://hg.mozilla.org/build/buildbotcustom/rev/63a621acf7d1
Attachment #403532 - Flags: checked-in+
(Assignee)

Updated

9 years ago
Status: NEW → RESOLVED
Last Resolved: 9 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.