503242 - Document and clean up mobile testing infrastructure

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Aki Sasaki (not active)]

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.

Comment 1

[Aki Sasaki (not active)]

Attachment: 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.

Comment 2

[Aki Sasaki (not active)]

https://wiki.mozilla.org/User:Asasaki:MobileMaster

Comment 3

[Aki Sasaki (not active)]

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.

Comment 4

[Aki Sasaki (not active)]

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

No longer needed now that bug 516466 is fixed.

Comment 5

[Aki Sasaki (not active)]

Attachment: merge mobile, mobile-staging back to buildbot-configs

Comment 6

[Aki Sasaki (not active)]

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).

Comment 7

[[e-mail] (:bhearsum)]

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--

Comment 8

[Aki Sasaki (not active)]

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/?

Comment 9

[[e-mail] (:bhearsum)]

(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 10

[Chris AtLee [:catlee]]

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.

Comment 11

[Aki Sasaki (not active)]

Attachment: configs patch with factories moved to buildbotcustom

patch 1 of 2.

Comment 12

[Aki Sasaki (not active)]

Attachment: mobiletestfactory/mobileftpparser

patch 2 of 2.

Comment 13

[Aki Sasaki (not active)]

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.

Comment 14

[[e-mail] (:bhearsum)]

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 15

[[e-mail] (:bhearsum)]

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.

Comment 16

[Aki Sasaki (not active)]

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

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

Comment 17

[Aki Sasaki (not active)]

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

http://hg.mozilla.org/build/buildbotcustom/rev/63a621acf7d1