494676 - Upload Packaged tests for SeaMonkey

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, enhancement)

Description

[Serge Gautherie (:sgautherie)]

Log example of what Firefox (3.5) does:
{
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1243158590.1243165479.10872.gz&fulltext=1
WINNT 5.2 mozilla-1.9.1 unit test on 2009/05/24 02:49:50

======== BuildStep started ========
set props: packageUrl
=== Output ===
make upload
 in dir e:\\builds\\moz2_slave\mozilla-1.9.1-win32-unittest\build/objdir (timeout 1200 secs)
[...]
make[1]: Entering directory `/e/builds/moz2_slave/mozilla-1.9.1-win32-unittest/build/objdir/browser/installer'
/d/mozilla-build/python25/python /e/builds/moz2_slave/mozilla-1.9.1-win32-unittest/build/build/upload.py --base-path ../../dist \
		"../../dist/firefox-3.5pre.en-US.win32.zip" \
		 \
		 \
		"../../dist/firefox-3.5pre.en-US.win32.tests.tar.bz2" \
		"../../dist/firefox-3.5pre.en-US.win32.crashreporter-symbols.zip" \
		 \
		  ../../dist/firefox-3.5pre.en-US.langpack.xpi
http://stage.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-1.9.1-win32-unittest/1243158795/firefox-3.5pre.en-US.win32.zip
http://stage.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-1.9.1-win32-unittest/1243158795/firefox-3.5pre.en-US.win32.tests.tar.bz2
http://stage.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-1.9.1-win32-unittest/1243158795/firefox-3.5pre.en-US.win32.crashreporter-symbols.zip
http://stage.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-1.9.1-win32-unittest/1243158795/firefox-3.5pre.en-US.langpack.xpi
Uploading e:\builds\moz2_slave\mozilla-1.9.1-win32-unittest\build\objdir\dist\firefox-3.5pre.en-US.win32.zip
Uploading e:\builds\moz2_slave\mozilla-1.9.1-win32-unittest\build\objdir\dist\firefox-3.5pre.en-US.win32.tests.tar.bz2
Uploading e:\builds\moz2_slave\mozilla-1.9.1-win32-unittest\build\objdir\dist\firefox-3.5pre.en-US.win32.crashreporter-symbols.zip
Uploading e:\builds\moz2_slave\mozilla-1.9.1-win32-unittest\build\objdir\dist\firefox-3.5pre.en-US.langpack.xpi
Running post-upload command: post_upload.py --tinderbox-builds-dir mozilla-1.9.1-win32-unittest -i 20090524025315 -p firefox --release-to-tinderbox-dated-builds
Upload complete
make[1]: Leaving directory `/e/builds/moz2_slave/mozilla-1.9.1-win32-unittest/build/objdir/browser/installer'
program finished with exit code 0
elapsedTime=52.953000
packageUrl: 'http://stage.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-1.9.1-win32-unittest/1243158795/firefox-3.5pre.en-US.win32.zip'
}

Comment 1

[Serge Gautherie (:sgautherie)]

And this needs |make package-tests| first:
{
======== BuildStep started ========
'make package-tests'
=== Output ===
make package-tests
 in dir e:\\builds\\moz2_slave\mozilla-1.9.1-win32-unittest\build/objdir (timeout 1200 secs)
[...]
}

Comment 2

[Robert Kaiser]

Does |make package-tests| work at all on comm-central? I won't even look into possible differences between UnittestBuildFactory and CCUnittestBuildFactory or whatever if we can't verify that first.

Comment 3

[Serge Gautherie (:sgautherie)]

(In reply to comment #2)

Yeah, just give me enough time to test and file bug(s):
I filed bug 494677.

Comment 4

[Robert Kaiser]

1) This is NOT done on "unit test boxes", it's done on normal build cycles (at least in the Firefox world, may not be the standard build cycle though).
2) This is specific to a buildbot config and therefore to an application.
3) I just verified that at least xpcshell tests work from packaged build + packaged tests for SeaMonkey, but bug 494677 will need to forward the package-tests target from the toplevel to mozilla/
4) If this is run on standard build cycles, it just can't be done without larger changes, but I need to investigate what Firefox does before judging this.
5) I probably won't look into that before the slave pools are production-ready, as there are more pressing things to do first, like getting release automation working (or a beta could take me days of intense work).

Comment 5

[Mark Banner (:standard8)]

(In reply to comment #4)
> 1) This is NOT done on "unit test boxes", it's done on normal build cycles (at
> least in the Firefox world, may not be the standard build cycle though).

> 4) If this is run on standard build cycles, it just can't be done without
> larger changes, but I need to investigate what Firefox does before judging
> this.

I believe the intention is that eventually it would be done on release and debug boxes for Firefox - you'd then get two sets of results, but you'd get the debug info (leak stats etc).

Note that Thunderbird can't really do it on the "release" boxes - they are --enable-static builds which don't support making xpcshell (due to its linkages). If we go that way on SeaMonkey there will be the same problem.

(Note switching to libxul would resolve this, but that's a long way off).

> 5) I probably won't look into that before the slave pools are production-ready,
> as there are more pressing things to do first, like getting release automation
> working (or a beta could take me days of intense work).

If cycle times are not a problem, then I don't think you need to worry about this. The only thing it would give is a tests package for which people could download the tests and run them against builds, which given they already run automated I don't see provides much benefit at the moment.

Comment 6

[Serge Gautherie (:sgautherie)]

(In reply to comment #4)
> 1) This is NOT done on "unit test boxes", it's done on normal build cycles (at
> least in the Firefox world, may not be the standard build cycle though).

Afaict, I have to disagree with this (at least _atm_):

> 4) If this is run on standard build cycles, it just can't be done without
> larger changes, but I need to investigate what Firefox does before judging
> this.

WINNT 5.2 mozilla-1.9.1 leak test build on 2009/05/25 06:56:09
does not upload anything
WINNT 5.2 mozilla-1.9.1 build on 2009/05/25 05:46:26
WINNT 5.2 mozilla-1.9.1 nightly on 2009/05/25 03:02:00
uploads but no tests
WINNT 5.2 mozilla-1.9.1 unit test on 2009/05/25 03:46:26
uploads tests !

> 2) This is specific to a buildbot config and therefore to an application.

The factory code seems shared, but enabling the option may be "per app"?

See
http://mxr.mozilla.org/build/search?string=package-tests&case=on&find=custom%2Fprocess%2Ffactory%5C.py%24
{
232 class MercurialBuildFactory(MozillaBuildFactory):
576     def addUploadSteps(self, pkgArgs=None):
585         if self.packageTests:
587              command=['make', 'package-tests'],

1977 class UnittestBuildFactory(MozillaBuildFactory):
1978     def __init__(self, platform, productName, config_repo_path, config_dir,
2078         if self.uploadPackages:
2086              command=['make', 'package-tests'],

2779 class MaemoBuildFactory(MobileBuildFactory):
2842     def addPackageSteps(self):
2851             command=[self.scratchboxPath, '-p', '-d',
2852                      'build/%s/%s/xulrunner' % (self.branchName, self.objdir),
2853                      'make package-tests PYTHON=python2.5'],
}

> 5) I probably won't look into that before the slave pools are production-ready,
> as there are more pressing things to do first, like getting release automation
> working (or a beta could take me days of intense work).

(Uploading tests would seem rather trivial from my (current) point of view,
but it is up to you...)

Comment 7

[Serge Gautherie (:sgautherie)]

(In reply to comment #6)

> > 2) This is specific to a buildbot config and therefore to an application.
> 
> The factory code seems shared, but enabling the option may be "per app"?

Current situation is:

http://mxr.mozilla.org/build/search?string=packageTests&case=on
MercurialBuildFactory 'packageTests' is always 'False'.

http://mxr.mozilla.org/build/search?string=uploadPackages&case=on
This is indeed app specific;
as you wrote to me irc, this "could" be as simple as switching the value of
{
/buildbot-configs/seamonkey-unittest/master.cfg
    * line 217 -- uploadPackages = False
}

Comment 8

[Robert Kaiser]

(In reply to comment #6)
> 1977 class UnittestBuildFactory(MozillaBuildFactory):
> 1978     def __init__(self, platform, productName, config_repo_path,
> config_dir,
> 2078         if self.uploadPackages:
> 2086              command=['make', 'package-tests'],

Hah!

This needs to be ported to CCUnittestBuildFactory (in the same factory.py file). Once that's done, it's trivial to actually turn it on.

(In reply to comment #7)
> /buildbot-configs/seamonkey-unittest/master.cfg

Won't do it on the old boxes, but on the new ones that have their config in my personal http://hg.mozilla.org/users/kairo_kairo.at/bbconf-sm2/ repo while it's not in production yet.

Comment 9

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) CCUnittestBuildFactory: port packaging code from bug 383136

This "duplicates" the missing parts from
http://hg.mozilla.org/build/buildbotcustom/rev/c63793bf2a07
I reordered the param/var settings,
but the package code block is an exact copy and paste.

As is, everything is False/None, so should make no differences yet.
After this, we'll need to port the stage server config patch (at least)...

Comment 10

[Robert Kaiser]

Comment on attachment 379627 [details] [diff] [review]
(Av1) CCUnittestBuildFactory: port packaging code from bug 383136

> class CCUnittestBuildFactory(MozillaBuildFactory):
>     def __init__(self, platform, config_repo_path, config_dir, objdir, mozRepoPath,
>             productName=None, brandName=None, mochitest_leak_threshold=None,
>             mochichrome_leak_threshold=None, mochibrowser_leak_threshold=None,
>             exec_reftest_suites=True, exec_mochi_suites=True, run_a11y=True,
>-            **kwargs):
>+            uploadPackages=False, stageServer=None, stageUsername=None,
>+            stageSshKey=None, unittestMasters=None, **kwargs):

Please keep the params in an order as similar as possible to UnittestBuildFactory, i.e. move ProductName and make it non-optional, unittestMasters before the stage* variables.

>+        # Packaged tests upload configuration.
>+        self.uploadPackages = uploadPackages
>+        self.stageServer = stageServer
>+        self.stageUsername = stageUsername
>+        self.stageSshKey = stageSshKey

Please remove the comments and just follow the UnittestBuildFactory as closely as possible here as well, including the order of the arguments. I want to visible or technical diff between those two classes to be kept as minimal as possible.

>+            self.addStep(GetBuildID,
>+             objdir=self.objdir,
>+            )

This won't just work like that, it needs to set the workdir to build/mozilla and also append the "/mozilla" to the objdir it assigns.


r- for now for those needed changes, though the patch basically looks good to me else.

Comment 11

[Robert Kaiser]

CCing gozer as this class is already actively used by Thunderbird as well.

Comment 12

[Serge Gautherie (:sgautherie)]

Attachment: (Av2) CCUnittestBuildFactory: port packaging code from bug 383136

Av1, with comment 10 suggestion(s).

Comment 13

[Robert Kaiser]

Comment on attachment 379679 [details] [diff] [review]
(Av2) CCUnittestBuildFactory: port packaging code from bug 383136

>+            self.addStep(GetBuildID,
>+             objdir=self.mozillaObjdir,
>+             workdir='build%s' % self.mozillaDir,
>+            )

Good thought (and thanks for catching the identify case) but the unit test factories sucks by not knowing about mozillaDir or mozillaObjdir, which is only defined in the MercurialBuildFactory from they should be derived in theory but aren't in practice. We need to either introduce it at least in CCUnittestFactory or hardcode the "mozilla" subdir (which the other CC*Factory implementations do by passing just that as mozillaDir to the generic ones).

Comment 14

[Serge Gautherie (:sgautherie)]

Attachment: (Av3) CCUnittestBuildFactory: port packaging code from bug 383136

Av2, with comment 10 suggestion(s).

I prefer to leave additional inheritance work to a follow-up bug, as it may involve the non-CC classes.

Comment 15

[Robert Kaiser]

Comment on attachment 379912 [details] [diff] [review]
(Av3) CCUnittestBuildFactory: port packaging code from bug 383136

This is wrong, leave this bogus CCMozillaBuildFactory out of the game, we don't need it. Instead, just defined self.mozillaDir = "/mozilla" and the according mozillaObjdir in CCUnittestBuildFactory directly.

Comment 16

[Serge Gautherie (:sgautherie)]

Attachment: (Av4) CCUnittestBuildFactory: port packaging code from bug 383136 [Checkin: Comment 18]

Av2, with comment 15 suggestion(s).

Comment 17

[Robert Kaiser]

Comment on attachment 379946 [details] [diff] [review]
(Av4) CCUnittestBuildFactory: port packaging code from bug 383136
[Checkin: Comment 18]

Thanks for being patient and following through all those minuses ;-)

This version looks god for the moment (as you said, unifying the classes more should be a different patch).

Comment 18

[Serge Gautherie (:sgautherie)]

Comment on attachment 379946 [details] [diff] [review]
(Av4) CCUnittestBuildFactory: port packaging code from bug 383136
[Checkin: Comment 18]


http://hg.mozilla.org/build/buildbotcustom/rev/1c41411e31b1

Comment 19

[Serge Gautherie (:sgautherie)]

(In reply to comment #17)

> (as you said, unifying the classes more should be a different patch).

I filed bug 495154.

Comment 20

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) master.cfg: port upload config from bug 383136 [Checkin: Comment 21]

(bbconf-sm2 patch:)
*2 sync' with mozilla2.
*Port upload configuration only.

Comment 21

[Serge Gautherie (:sgautherie)]

Comment on attachment 380233 [details] [diff] [review]
(Bv1) master.cfg: port upload config from bug 383136
[Checkin: Comment 21]


http://hg.mozilla.org/users/kairo_kairo.at/bbconf-sm2/rev/a8fa59faee71

Comment 22

[Serge Gautherie (:sgautherie)]

This should be it :-)

I will verify this after KaiRo pulls this on the SM-Ports boxes...

Comment 23

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1pre) Gecko/20090604
SeaMonkey/2.0b1pre] (comm-1.9.1-win32-unittest/1244175469) (W2Ksp4)
(http://hg.mozilla.org/releases/mozilla-1.9.1/rev/010761f9d36b
 +http://hg.mozilla.org/comm-central/rev/afaecfac4fac)

V.Fixed

Comment 24

[Robert Kaiser]

Sorry, I need to reopen this to have a place to put a fixup I needed to do for the CCUnittestBuildFactory.

Comment 25

[Robert Kaiser]

Attachment: wrong patch, please ignore

This patch fixes the objdir we're using. even if we always have a mozillaDir for CC*Factory, I did this workaround so this can potentially be merged when/if those factories get abstracted one time.

Comment 26

[Serge Gautherie (:sgautherie)]

Comment on attachment 383987 [details] [diff] [review]
wrong patch, please ignore

>+# shell-builtin echo on Mac doesn't support -n, external one in /bin/ does

Fwiw,
http://developer.apple.com/documentation/Darwin/Reference/Manpages/man1/echo.1.html
{
Some shells may provide a builtin echo command which is similar or identical to this utility.  Most notably, the builtin echo in sh(1) does not accept the -n option.  Consult the builtin(1) manual page.
}

Firefox doesn't have this.
http://mxr.mozilla.org/comm-central/search?string=echo+-n&case=on&find=%2Fapp%2FMakefile%5C.in%24
Does it need it too?

Comment 27

[Robert Kaiser]

Attachment: fix up the properties/buildid objdir [Checkin: Comment 29]

Serge, forget that other patch, it was the wrong one. But thanks for making me aware of that ;-)

Comment 28

[Philippe M. Chiasson (:gozer)]

Comment on attachment 384018 [details] [diff] [review]
fix up the properties/buildid objdir
[Checkin: Comment 29]

Looks good to me, one more special case for comm-central!

Comment 29

[Robert Kaiser]

Pushed factory.py fixup as http://hg.mozilla.org/build/buildbotcustom/rev/0bff92bdd6fc