Closed Bug 494676 Opened 12 years ago Closed 12 years ago

Upload Packaged tests for SeaMonkey

Categories

(SeaMonkey :: Build Config, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0b1

People

(Reporter: sgautherie, Assigned: sgautherie)

References

Details

Attachments

(3 files, 4 obsolete files)

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'
}
Flags: in-testsuite-
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)
[...]
}
Summary: Call |make upload| on unit tests boxes (too) → Call |make package-tests| and |make upload| on unit tests boxes
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.
Depends on: 494677
(In reply to comment #2)

Yeah, just give me enough time to test and file bug(s):
I filed bug 494677.
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).
Assignee: nobody → build-config
Depends on: 485821
Product: MailNews Core → SeaMonkey
QA Contact: build-config → build-config
Summary: Call |make package-tests| and |make upload| on unit tests boxes → Upload Packaged tests for SeaMonkey
(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.
(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...)
(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
}
(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.
Depends on: 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)...
Assignee: build-config → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #379627 - Flags: review?(kairo)
Attachment #379627 - Flags: review?(kairo) → review-
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.
CCing gozer as this class is already actively used by Thunderbird as well.
Av1, with comment 10 suggestion(s).
Attachment #379627 - Attachment is obsolete: true
Attachment #379679 - Flags: review?(kairo)
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).
Attachment #379679 - Flags: review?(kairo) → review-
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.
Attachment #379679 - Attachment is obsolete: true
Attachment #379912 - Flags: review?(kairo)
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.
Attachment #379912 - Flags: review?(kairo) → review-
Av2, with comment 15 suggestion(s).
Attachment #379912 - Attachment is obsolete: true
Attachment #379946 - Flags: review?(kairo)
Attachment #379946 - Flags: review?(kairo) → review+
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 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
Attachment #379946 - Attachment description: (Av4) CCUnittestBuildFactory: port packaging code from bug 383136 → (Av4) CCUnittestBuildFactory: port packaging code from bug 383136 [Checkin: Comment 18]
(In reply to comment #17)

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

I filed bug 495154.
(bbconf-sm2 patch:)
*2 sync' with mozilla2.
*Port upload configuration only.
Attachment #380233 - Flags: review?(kairo)
Attachment #380233 - Flags: review?(kairo) → review+
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
Attachment #380233 - Attachment description: (Bv1) master.cfg: port upload config from bug 383136 → (Bv1) master.cfg: port upload config from bug 383136 [Checkin: Comment 21]
This should be it :-)

I will verify this after KaiRo pulls this on the SM-Ports boxes...
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0b1
Blocks: 495299
Blocks: 496534
[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
Status: RESOLVED → VERIFIED
Sorry, I need to reopen this to have a place to put a fixup I needed to do for the CCUnittestBuildFactory.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Attached patch wrong patch, please ignore (obsolete) — Splinter Review
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.
Attachment #383987 - Flags: review?(gozer)
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?
Serge, forget that other patch, it was the wrong one. But thanks for making me aware of that ;-)
Attachment #383987 - Attachment is obsolete: true
Attachment #384018 - Flags: review?(gozer)
Attachment #383987 - Flags: review?(gozer)
Attachment #383987 - Attachment description: fix up the properties/buildid objdir → wrong patch, please ignore
No longer blocks: 496534
Depends on: 496534
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!
Attachment #384018 - Flags: review?(gozer) → review+
Pushed factory.py fixup as http://hg.mozilla.org/build/buildbotcustom/rev/0bff92bdd6fc
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Attachment #384018 - Attachment description: fix up the properties/buildid objdir → fix up the properties/buildid objdir [Checkin: Comment 29]
You need to log in before you can comment on or make changes to this bug.