Closed Bug 1209378 Opened 4 years ago Closed 4 years ago

Mozharness.zip is set as packageUrl during build and thus affecting tests.

Categories

(Thunderbird :: Testing Infrastructure, defect, critical)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ewong, Assigned: aleth)

Details

Attachments

(1 file, 8 obsolete files)

Currently (and for some time now), all tests are perma-red due to changes
in how the tests are packed up and run.  In the past, we used the mozbase stuff
included in the tests zip file.  These are no longer in the zip file.

From preliminary scanning of Firefox's testing logs, these mozbase (et. al)
dependent programs are installed via pip into a virtualenv and then
the tests are run in that virtualenv.

This bug is to track and fix the testing infrastructure.
Attachment #8667082 - Attachment is obsolete: true
The change in how the tests are packaged was done in bug 917999.

The general issue is that our buildbotcustom code (something like http://hg.mozilla.org/build/buildbotcustom/file/3bf30933e140/process/factory.py#l197) assumes that the found testUrl is the right one.

But it isn't because when it uploads the tests to the server,
the list of files uploaded are as follows:

http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/seamonkey-2.41a1.en-US.linux-i686.tar.bz2
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/seamonkey-2.41a1.en-US.langpack.xpi
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/seamonkey-2.41a1.en-US.linux-i686.common.tests.zip
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/seamonkey-2.41a1.en-US.linux-i686.cppunittest.tests.zip
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/seamonkey-2.41a1.en-US.linux-i686.xpcshell.tests.zip
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/seamonkey-2.41a1.en-US.linux-i686.mochitest.tests.zip
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/seamonkey-2.41a1.en-US.linux-i686.talos.tests.zip
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/seamonkey-2.41a1.en-US.linux-i686.reftest.tests.zip
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/seamonkey-2.41a1.en-US.linux-i686.web-platform.tests.zip
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/seamonkey-2.41a1.en-US.linux-i686.crashreporter-symbols.zip
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/seamonkey-2.41a1.en-US.linux-i686.txt
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/seamonkey-2.41a1.en-US.linux-i686.json
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/seamonkey-2.41a1.en-US.linux-i686.mozinfo.json
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/test_packages.json
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/mar
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/mbsdiff
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/comm-central-trunk-linux-debug/1443623507/seamonkey-2.41a1.en-US.linux-i686.checksums
Upload complete

(for example)

The last item that matches *.tests.zip (or tar.bz2) is the web-platforms one.
So it sets that as the testsUrl, which is wrong. (We don't even run web-platforms
tests.)  So for all our mochitests/reftests/xpcshell/crashtests,  it uses
the web-platform tests.zip.  

The second issue is mozbase/* aren't included in the zip files anymore
as comment #0 mentioned.
Attached patch [c-c] proposed patch(v1) (obsolete) — Splinter Review
This patch basically adds a suite/ rule to the testing to create a mozbase.zip
which includes the necessary mozbase, certs, bin files.
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #8672887 - Flags: review?(bugspam.Callek)
Attachment #8672887 - Attachment description: proposed patch(v1) → [c-c] proposed patch(v1)
With the tests split into different zip files (mochitest, reftest, crashtest,
xpcshell and web-platform), the buildbotcustom code cannot handle this new
testing setup. This wip patch enables the code to find out the actual 
test zip it needs and download it.
Attachment #8672890 - Flags: review?(bugspam.Callek)
Comment on attachment 8672887 [details] [diff] [review]
[c-c] proposed patch(v1)

context for this patch:

the bin/ mozbase/ cert/* as mentioned in http://hg.mozilla.org/build/buildbotcustom/file/81b1838ceb86/steps/misc.py#l546
do not exist in the tests.zip files.  Since we need these, they must
be packaged into a separate mozbase.zip file (we're not ready for
testing with mozharness, which is what Firefox does).  

This is what this patch does.  It creates a mozbase.zip file based
on the SeaMonkey version and from the necessary bin/ certs/...
directories.
Attachment #8672887 - Flags: review?(bugspam.Callek) → review?(kairo)
Comment on attachment 8672887 [details] [diff] [review]
[c-c] proposed patch(v1)

Review of attachment 8672887 [details] [diff] [review]:
-----------------------------------------------------------------

After some discussion on IRC, please just put that target right into build.mk where you else would include it, and please add a comment that states that 1) this is needed for getting the base files we need for running tests (as we are not using mozharness that would automate all this) and 2) that we're calling this target directly from buildbot.
Attachment #8672887 - Flags: review?(kairo) → review-
Attached patch [c-c] proposed patch (v2) (obsolete) — Splinter Review
Attachment #8672887 - Attachment is obsolete: true
Attachment #8678621 - Flags: review?(kairo)
Comment on attachment 8678621 [details] [diff] [review]
[c-c] proposed patch (v2)

Review of attachment 8678621 [details] [diff] [review]:
-----------------------------------------------------------------

::: suite/build.mk
@@ +26,5 @@
>  
> +# This target is needed to get the mozbase files (bin/*, certs/*
> +# config/* mozbase/* modules/* into a mozbase.zip file which is
> +# needed for our suite's tests and is called from buildbotcustom's
> +# |make_pkg_mozbase| step.

Nit: Closing bracket missing.

@@ +27,5 @@
> +# This target is needed to get the mozbase files (bin/*, certs/*
> +# config/* mozbase/* modules/* into a mozbase.zip file which is
> +# needed for our suite's tests and is called from buildbotcustom's
> +# |make_pkg_mozbase| step.
> +package-mozbase:

Why is this outside of ENABLE_TESTS now?

@@ +36,4 @@
>  
>  # mochitests need to be run from the Mozilla build system
>  ifdef ENABLE_TESTS
> +include $(topsrcdir)/../suite/testsuite-targets.mk

Umm, that file doesn't exist any more, right?
Comment on attachment 8678621 [details] [diff] [review]
[c-c] proposed patch (v2)

This isn't going to work.  mozbase.zip needs mozbase/ which is no longer
included in any of the tests.zip nor in the proposed patch's package-mozbase
rule.
Attachment #8678621 - Attachment is obsolete: true
Attachment #8678621 - Flags: review?(kairo)
Comment on attachment 8667086 [details] [diff] [review]
[puppet] temp patch to disable c-cen-t-lnx-dbg clobbering

I pushed this again to the puppet repo.

https://hg.mozilla.org/SeaMonkey/puppet/rev/6f4a15710abd
Attachment #8672890 - Attachment is obsolete: true
Attachment #8672890 - Flags: review?(bugspam.Callek)
Attachment #8678702 - Flags: review?(bugspam.Callek)
Attached patch [c-c] build.mk changes (obsolete) — Splinter Review
Attachment #8678703 - Flags: review?(kairo)
The concept behind the wip fix is as follows:

Our tests require mozbase (and the separate tests.zip files).  mozbase
used to be included in the tests.zip but have since been excluded.
Since the change was in m-c, I believe we need to add the 'package-up-mozbase'
in suite's build.mk (which is what the c-c patch does).

The [custom] code basically allows the buildbot to differentiate the
different tests.zip files there are in the tinderbox-build folder.
Since SeaMonkey only does mochitests, crashtests, reftests and xpcshell tests
(leaving out talos and web-platform), there needs to be some checks
as to what the test is being run and then use the appropriate test.

The issue right now is I am not 100% convinced the c-c/custom patches
are finished because I have not yet determined if the mozbase packaged zip
file has exactly the same files as what we had prior to the separation
of tests.  So instead of asking for review, I'll be asking for feedback
instead.
Attachment #8678702 - Flags: review?(bugspam.Callek) → feedback?(bugspam.Callek)
Attachment #8678703 - Flags: review?(kairo) → feedback?(kairo)
Comment on attachment 8678703 [details] [diff] [review]
[c-c] build.mk changes

Review of attachment 8678703 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, this looks good to me if it works. Thanks for the comments explaining what's going on there!
Attachment #8678703 - Flags: feedback?(kairo) → feedback+
need to unpack the mozbase package before running the tests.
Attachment #8678702 - Attachment is obsolete: true
Attachment #8678702 - Flags: feedback?(bugspam.Callek)
Attachment #8679214 - Flags: feedback?(bugspam.Callek)
Comment on attachment 8679214 [details] [diff] [review]
[custom] Differentiate the different tests.zip files. (incl mozbase) (v4)

Review of attachment 8679214 [details] [diff] [review]:
-----------------------------------------------------------------

::: process/factory.py
@@ +216,5 @@
>              retval['packageRpmUrl'] = m
>          elif m.endswith("crashreporter-symbols.zip"):
>              retval['symbolsUrl'] = m
> +        elif 'mozharness' in m and m.endswith('.zip'):
> +            retval['mozharnessUrl'] = m

As per the discussion on #releng with nthomas, this should be "pass" to avoid mozharness.zip being passed along in the sendchange and then breaking the test steps.
Attachment #8679214 - Flags: feedback?(nthomas)
Comment on attachment 8679214 [details] [diff] [review]
[custom] Differentiate the different tests.zip files. (incl mozbase) (v4)

Review of attachment 8679214 [details] [diff] [review]:
-----------------------------------------------------------------

f+. This looks to be against the seamonkey branch in buildbotcustom and we'd need to port it to default for Thunderbird. Are you willing to look at that ?

::: process/factory.py
@@ +216,5 @@
>              retval['packageRpmUrl'] = m
>          elif m.endswith("crashreporter-symbols.zip"):
>              retval['symbolsUrl'] = m
> +        elif 'mozharness' in m and m.endswith('.zip'):
> +            retval['mozharnessUrl'] = m

This would prevent mozharness being set as packageUrl, which would fix Thunderbird up. If mozharnessUrl is unused then 'pass' is marginally preferable, but I don't really mind.

@@ +2631,5 @@
>  
>              files = [WithProperties('%(packageUrl)s')]
>              if '1.9.1' not in self.branchName:
> +                files.append(WithProperties('%(mochitesttestsUrl)s'))
> +                files.append(WithProperites('%(mozbaseUrl)s'))

Mismatch here with earlier mozbasepkgUrl.
Attachment #8679214 - Flags: feedback?(nthomas) → feedback+
(In reply to Nick Thomas [:nthomas] from comment #18)
> Comment on attachment 8679214 [details] [diff] [review]
> [custom] Differentiate the different tests.zip files. (incl mozbase) (v4)
> 
> Review of attachment 8679214 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> f+. This looks to be against the seamonkey branch in buildbotcustom and we'd
> need to port it to default for Thunderbird. Are you willing to look at that ?
> 
Sure.  

> ::: process/factory.py
> @@ +216,5 @@
> >              retval['packageRpmUrl'] = m
> >          elif m.endswith("crashreporter-symbols.zip"):
> >              retval['symbolsUrl'] = m
> > +        elif 'mozharness' in m and m.endswith('.zip'):
> > +            retval['mozharnessUrl'] = m
> 
> This would prevent mozharness being set as packageUrl, which would fix
> Thunderbird up. If mozharnessUrl is unused then 'pass' is marginally
> preferable, but I don't really mind.
> 

I figured that if TB or SM ever uses mozharness, this would help; but
pass is another option and leave it for a future bug to deal with
it.

> @@ +2631,5 @@
> >  
> >              files = [WithProperties('%(packageUrl)s')]
> >              if '1.9.1' not in self.branchName:
> > +                files.append(WithProperties('%(mochitesttestsUrl)s'))
> > +                files.append(WithProperites('%(mozbaseUrl)s'))
> 
> Mismatch here with earlier mozbasepkgUrl.

Thanks! Will fix that.
I split out this part in the hope of helping to unbust TB tests quickly.
Attachment #8681716 - Flags: review?(nthomas)
Attachment #8681716 - Flags: review?(nthomas) → review+
Attachment #8678703 - Flags: review?(bugspam.Callek)
Attachment #8678703 - Flags: review?(kairo)
Attachment #8678703 - Flags: review?(kairo) → review+
Morphing this to a TB bug as it fixed the mozharness-being-used-as-packageUrl
bug.
Assignee: ewong → aleth
Summary: [Tracking bug] Fix SeaMonkey's testing infrastructure → Mozharness.zip is set as packageUrl during build and thus affecting tests.
Product: SeaMonkey → Thunderbird
Attachment #8667086 - Attachment is obsolete: true
Attachment #8678703 - Attachment is obsolete: true
Attachment #8678703 - Flags: review?(bugspam.Callek)
Attachment #8679214 - Attachment is obsolete: true
Attachment #8679214 - Flags: feedback?(bugspam.Callek)
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.