826933 - Update jetpack test suite to run in-tree version

Status: VERIFIED FIXED

(Release Engineering :: General, defect, P1)

Description

[Dave Townsend [:mossop]]

Attachment: run hidden gecko jetpack jobs differently

As we ship the APIs with Firefox we also want to run the tests against the in-tree versions. Bug 793928 which is currently on the Larch branch embeds the SDK APIs into Firefox as well as adding the test harness and test files to the same tests package that mochitests etc. go into. This bug is about updating buildbot to correctly run those in-tree tests instead of downloading them from a separate repository.

The attached patch is a guess at what needs to change in buildbot to do this, however since we don't want to break existing trees I'm guess we need something more, probably creating a new Jetpack test for the tree, only enabled for the larch branch and then migrating the other trees over to that when necessary.

Comment 1

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Summary from mtg w/mossop this morning:

1) this bug is about having the tests which are currently run on addon-sdk branch now also be run on mozilla-central, m-a, m-b, m-r. There are no new/different tests suites here. The only difference is the location from where jetpack code is pulled for testing.
1a) once the jetpack code is landed into mozilla-central, the tests on m-c would be testing the checked-in-m-c jetpack
1b) this test would replace the existing "JP" testsuite on mozilla-central (and anywhere else it is being run)

2) Mossop will find someone in jetpack group to review the build/test results on the "addon-sdk" branch, and periodically choose a green jetpack changeset on "addon-sdk" branch to land into mozilla-central. 
2a) The "addon-sdk" branch will continue to test the not-yet-landed github version of jetpack. It is therefore still useful to jetpack team that addon-sdk branch exists, and that jetpack tests be run on mozilla-central. Without "addon-sdk" branch, the alternative would be for jetpack team to send potentially green changesets to TryServer. This is more manual, so not preferred.
2b) mossop to confirm whether on "addon-sdk", there is any more need to run jetpack tests for m-a, m-b, m-r.

3) larch is being used for makefile changes to landing of jetpack code into mozilla-central. Once this is all green, mossap will land from larch to mozilla-central, and return rentable "larch" branch. Bug#826933 is tracking this work.


Open questions
4) We believe these jetpack tests *might* be runnable on VMs but need to verify. Once we have usable VM, joduinn to spin out dep bug for RelEng to loan a VM to jetpack team for eval.

5) do jetpack need tests run on *all* OS? both opt *and* debug? per-checkin *and* nightly?
5a) mossop to verify if testing on all OS needed
5b) mossop to verify if both opt and debug needed per checkin? Maybe do all for nightly, but only do some per-checkin?

6) version support - transition plans?
mossop to find link of transition/rollout/support plan and add link here so we all on same understanding of transition plan.

Comment 2

[Phil Ringnalda (:philor)]

When I wanted to refresh my memory of jetpack test failures, the first bug I found was bug 774709, where the first six words of the description, "Happens only on Win 7 debug" say that yes, opt+debug and every OS, it's no more OS agnostic than any other test suite.

Comment 3

[Dave Townsend [:mossop]]

(In reply to John O'Duinn [:joduinn] from comment #1)
> Summary from mtg w/mossop this morning:
> 
> 1) this bug is about having the tests which are currently run on addon-sdk
> branch now also be run on mozilla-central, m-a, m-b, m-r. There are no
> new/different tests suites here. The only difference is the location from
> where jetpack code is pulled for testing.
> 1a) once the jetpack code is landed into mozilla-central, the tests on m-c
> would be testing the checked-in-m-c jetpack
> 1b) this test would replace the existing "JP" testsuite on mozilla-central
> (and anywhere else it is being run)
> 
> 2) Mossop will find someone in jetpack group to review the build/test
> results on the "addon-sdk" branch, and periodically choose a green jetpack
> changeset on "addon-sdk" branch to land into mozilla-central. 
> 2a) The "addon-sdk" branch will continue to test the not-yet-landed github
> version of jetpack. It is therefore still useful to jetpack team that
> addon-sdk branch exists, and that jetpack tests be run on mozilla-central.
> Without "addon-sdk" branch, the alternative would be for jetpack team to
> send potentially green changesets to TryServer. This is more manual, so not
> preferred.
> 2b) mossop to confirm whether on "addon-sdk", there is any more need to run
> jetpack tests for m-a, m-b, m-r.

Confirmed, nce we're landed on m-c we can turn off the m-a, m-b and m-r tests from the addon-sdk tree.

> 5) do jetpack need tests run on *all* OS? both opt *and* debug? per-checkin
> *and* nightly?
> 5a) mossop to verify if testing on all OS needed
> 5b) mossop to verify if both opt and debug needed per checkin? Maybe do all
> for nightly, but only do some per-checkin?

As philor says, we do hit platform dependent problems but I do think those are rarer. I think we can get away with only testing on one platform per checkin and then running tests on all platforms on every nightly for now and see how that works out. If it's possible to be able to go back and trigger changes on specific platforms to narrow down regressions after the fact then that makes things even easier.

Picking the platform is kind of guesswork, I'd go for a debug build at least (since that will help us catch assertions) and maybe Windows since most of my team work on Linux and OSX right now so we should already be doing internal testing there.

Comment 4

[Jeff Griffiths (:canuckistani) (:⚡︎)]

(In reply to Dave Townsend (:Mossop) from comment #3)

Agreed with Mossop:

* a Windows debug platform per check-in
* all platforms nightly
* turn off m-a / m-b / m-r, I assume that uplifts to these channels get enough scrutiny

I think we should go with this as soon as feasible and then set a specific date around Firefox 22 uplift to review and see if we need to adjust. We should know by then if it isn't working.

Comment 5

[Armen [:armenzg]]

I will be helping with this. I will touch base with Mossop to make sure I understand the scope of this.

Comment 6

[Armen [:armenzg]]

I would like to recommend to keep running per-checkin for these reasons:
* even if we have one platform coverage per-checkin it becomes a churn to find out on which changeset something broke
** we would not have a way of doing regression hunting without:
** A) involving releng to manufacture hand made sendchanges
** B) get a developer to go back and download builds and test packages
* we would not be increasing current load with my proposal but maintain current status-quo of load
* we would be meeting the tier1 expectations
* it makes my work easier
* let the machines do the job that is expensive to humans
* we can go later and trim if we need to
* we have not done such trimming on the current old jetpack model

Please raise concerns if I'm missing anything.

This is the plan that I went over with Mossop.
We hope to move fast and get things running on Larch by next week.

Plan of record:
1) (Mossop) manually run tests from artifacts on Larch
** to be accomplished today
2) (armenzg) add Jetpack jobs to Larch
** possibly through mozharness
3) green things out & evaluate where we stand

Questions by armenzg - answers by Mossop:
* do the test packages on Larch already produce the tests?
YES
* have you been able to download a build and tests from Larch and run jetpack tests?
WIP
* run side by side on Larch old-jetpack test jobs plus new way?
Just the new way. We should not see differences.
* do jetpack need tests run on *all* OS?
YES
* both opt *and* debug?
BOTH
* per-checkin *and* nightly?
PER-CHECKIN (as per explanation at the beginning of the comment)
* what is the transition plan?
** target FF21 or FF22
** so far, we will shut old Jetpack from Addon-sdk for m-a, m-b and m-r once we're live on m-c

Comment 7

[Dave Townsend [:mossop]]

Windows:

> unzip firefox-21.0a1.en-US.win32.zip
> unzip firefox-21.0a1.en-US.win32.tests.zip
> python jetpack/bin/cfx --binary=firefox/firefox.exe --parseable testpkgs

Linux:

> tar -xjf firefox-21.0a1.en-US.linux-i686.tar.bz2
> unzip firefox-21.0a1.en-US.linux-i686.tests.zip
> python jetpack/bin/cfx --binary=firefox/firefox --parseable testpkgs

Both cases ran the full test suite but reported one error which seems to be a mistake and I've filed bug 836419 to take care of it.

I think regardless though this shows that the tests are being packaged enough and you should be able to proceed with the mozharness work. I'll try to get the test failure solved today in parallel with that.

Comment 8

[Ed Morley [:emorley]]

Note if we run only one platform per checkin, these tests will not be tier 1 and will not be made visible by default in TBPL. As such, I strongly recommend doing what Armen proposed in comment 6.

Comment 9

[Dave Townsend [:mossop]]

(In reply to Ed Morley [:edmorley UTC+0] from comment #8)
> Note if we run only one platform per checkin, these tests will not be tier 1
> and will not be made visible by default in TBPL. As such, I strongly
> recommend doing what Armen proposed in comment 6.

I wasn't aware that was a restriction, we absolutely want these tests to end up visible by default.

Comment 10

[Ed Morley [:emorley]]

(In reply to Dave Townsend (:Mossop) from comment #9)
> I wasn't aware that was a restriction, we absolutely want these tests to end
> up visible by default.

Otherwise we end up with the following scenario:
1) OS X specific bustage -> nightly OS X jetpack test run turns orange.
2) Tree is closed whilst what to backout is determined (since visible = tree closing)
3) Regression range is 24 hours, so we either keep the tree closed for hours bisecting (not going to go down well with devs), or else hide that suite after all (in which case, why was it visible in the first place?).

Comment 11

[Dave Townsend [:mossop]]

(In reply to Dave Townsend (:Mossop) from comment #7)
> Windows:
> 
> > unzip firefox-21.0a1.en-US.win32.zip
> > unzip firefox-21.0a1.en-US.win32.tests.zip
> > python jetpack/bin/cfx --binary=firefox/firefox.exe --parseable testpkgs
> 
> Linux:
> 
> > tar -xjf firefox-21.0a1.en-US.linux-i686.tar.bz2
> > unzip firefox-21.0a1.en-US.linux-i686.tests.zip
> > python jetpack/bin/cfx --binary=firefox/firefox --parseable testpkgs
> 
> Both cases ran the full test suite but reported one error which seems to be
> a mistake and I've filed bug 836419 to take care of it.

I've pushed an update to larch that updates to the latest SDK and merges in mozilla-central. It should fix this one test failure.

Comment 12

[Armen [:armenzg]]

Attachment: [mozharness] wip - jetpack.py

I reuse TestingMixin to be able to use --installer-url, --test-url and the extracting logic.

It's unfortunate that TestingMixin uses VirtualenvMixin. For some reason self.query_python_path('python') cannot find /usr/local/bin/python and instead it gives me build/venv/bin/python.

I had to add it to the actions.

Comment 13

[Aki Sasaki (not active)]

(In reply to Armen Zambrano G. [:armenzg] from comment #12)
> It's unfortunate that TestingMixin uses VirtualenvMixin. For some reason
> self.query_python_path('python') cannot find /usr/local/bin/python and
> instead it gives me build/venv/bin/python.

Hm.

We could change TestingMixin to default to self.query_exe("python") if self.config['use_virtualenv'] is False, or something.

But we do use mozinstall to install Firefox to test, so we probably want to rely on mozbase here anyway.

Comment 14

[Jeff Hammel]

If you want the system python, why not use sys.executable?

Comment 15

[Dave Townsend [:mossop]]

(In reply to Jeff Hammel [:jhammel] from comment #14)
> If you want the system python, why not use sys.executable?

I'm not totally sure we care which python we use FWIW

Comment 16

[Armen [:armenzg]]

Attachment: [mozharness] jetpack.py

This runs on my Mac.

I will work on the patch to add it to Larch.

Comment 17

[Dave Townsend [:mossop]]

We've now landed on mozilla-inbound.

Comment 18

[Armen [:armenzg]]

Attachment: wip - [mozharness] jetpack.py + buildbot

I think I will be able to go and use desktop_unitests.py if I put effort into it.

What do you think?

Comment 19

[Armen [:armenzg]]

Attachment: wip - buildbot-configs

Comment 20

[Armen [:armenzg]]

Attachment: wip - buildbotcustom

Comment 21

[Aki Sasaki (not active)]

Comment on attachment 709234 [details] [diff] [review]
wip - [mozharness] jetpack.py + buildbot

Nit: pyflakes says "scripts/jetpack.py:72: local variable 'code' is assigned to but never used"

>diff --git a/mozharness/base/script.py b/mozharness/base/script.py
>--- a/mozharness/base/script.py
>+++ b/mozharness/base/script.py
>@@ -142,16 +142,18 @@ class OSMixin(object):
> 
>     # http://www.techniqal.com/blog/2008/07/31/python-file-read-write-with-urllib2/
>     # TODO thinking about creating a transfer object.
>     def download_file(self, url, file_name=None, parent_dir=None,
>                       create_parent_dir=True, error_level=ERROR,
>                       num_retries=None, exit_code=-1):
>         """Python wget.
>         """
>+        if not url.startswith("http:"):
>+            url = "file://%s" % url

Hm, does this work?
I thought urllib, or urllib2 (I forget which) didn't support file://.
I think this is great if it works.

However, this is problematic in that we might have an ftp:// or https://.  (Probably not a gopher://)
Maybe |if not urlparse.urlparse(url)[0]:|

>-    def download_and_extract(self, target_unzip_dirs=None):
>+    def download_and_extract(self, target_unzip_dirs=None, install_binary=False):
>         """
>         download and extract test zip / download installer
>         """
>         if self.test_url:
>             self._download_test_zip()
>             self._extract_test_zip(target_unzip_dirs=target_unzip_dirs)
>         self._download_installer()
>+        if install_binary:
>+            self.install()
>         if self.config.get('download_symbols'):
>             self._download_and_extract_symbols()

Haha.

Instead of doing this, you can create the download_and_extract_and_install() method here, and have it call download_and_extract() then install().  Your choice which you use in your script.

Also, download_and_extract_and_install() is accurate, but I was already thinking download_and_extract() was a bit verbose.
Not sure what would be better.  setup? install? (install would require that you override self.install() in JetpackUnittest() to first call self.download_and_extract() and then TestingMixin.install(self) )

>+class JetpackUnittest(TestingMixin, BaseScript):
>+    config_options = [
>+        [["--specific-platform"],
>+        {"action": "store",
>+         "dest": "specific_platform",
>+         "help": "The type of platform specifics to use",
>+        }],
>+    ] + copy.deepcopy(testing_config_options)

I bet you want virtualenv_config_options as well.

>+    def _pre_config_lock(self, rw_config):
>+        c = self.config
>+        self.specific_platform = c["specific_platform"]
>+        platform_specifics = self.config['platform_specifics']
>+        for platforms_list in platform_specifics.keys():
>+            if self.specific_platform in platforms_list:
>+                for k, v in platform_specifics[platforms_list].iteritems():
>+                    self.config[k] = v

Clever.
We may be able to avoid this type of hackery once we get multiple config file support (e.g. script -c config1,config2 -c config3, since config2 or 3 could be platform-specific) but this is ok for now.

>+    def download_and_extract_and_install(self):
>+        c = self.config
>+        target_unzip_dirs = c.get("tests_zip_dirs")
>+        self.download_and_extract(target_unzip_dirs=target_unzip_dirs, install_binary=True)

As mentioned above, unless you need the install to happen before symbols, you can download_and_extract() then install().

>+    def run_jetpack(self):
>+        dirs = self.query_abs_dirs()
>+        python = self.query_python_path('python')
>+        cmd = [
>+            python, os.path.join(dirs['abs_work_dir'], 'tests/jetpack/bin/cfx'),
>+            "--binary=%s" % self.binary_path, "--parseable", "testpkgs"
>+        ]
>+        cmd = ["ls"]
>+        code = self.run_command(cmd)

You probably want to tie in the TBPL status stuff here.

Overall, this looks like a great start.

Comment 22

[Armen [:armenzg]]

This is likely to be slip into next week. I'm off tomorrow and the Gaia work won't be finished until Monday.

Comment 23

[Armen [:armenzg]]

Unless there are any surprises I should be able to pick this up soon.

Comment 24

[Armen [:armenzg]]

The guessed patch works (attachment 698184 [details] [diff] [review]) (tested on staging) but if we landed as-is it will turn the jetpack jobs on Aurora red [1].

I also assume that the mozilla-beta [2] and mozilla-release [3] jobs on the Jetpack tree will go red.

If this is fine with you guys I can request the review and have it enabled before EOW.

Can I get a sign off on this plan?

If this plan is not fine, it will take me a day or two to customize which branches run which way (support old model).

I have to put the mozharness work on the side as we won't be switching over until after the merge day. I will nevertheless be useful at that time.

My b2g work is not yet completed but I can take this back again.

[1] https://tbpl.mozilla.org/?tree=Mozilla-Aurora&jobname=jetpack&noignore=1
[2]https://tbpl.mozilla.org/?tree=Jetpack&jobname=jetpack-mozilla-beta
[3] https://tbpl.mozilla.org/?tree=Jetpack&jobname=jetpack-mozilla-release

Comment 25

[Dave Townsend [:mossop]]

(In reply to Armen Zambrano G. [:armenzg] from comment #24)
> The guessed patch works (attachment 698184 [details] [diff] [review])
> (tested on staging) but if we landed as-is it will turn the jetpack jobs on
> Aurora red [1].

I'm ok with that, once m-c merges to aurora next week they should go green again. Beta and Release tinderboxen don't run the JP tests at the moment, I'll be looking to get them turned on once Fx 21 has ridden that far down the trees.

> I also assume that the mozilla-beta [2] and mozilla-release [3] jobs on the
> Jetpack tree will go red.

This should not be affecting the tests run on the Jetpack tree (https://tbpl.mozilla.org/?tree=Jetpack). Those should continue to use run_jetpack.py to get the SDK and Firefox build to test.

> If this is fine with you guys I can request the review and have it enabled
> before EOW.
> 
> Can I get a sign off on this plan?

Provided that this doesn't affect the Jetpack tree then I'm fine with this.

Comment 26

[Armen [:armenzg]]

(In reply to Dave Townsend (:Mossop) from comment #25)
> (In reply to Armen Zambrano G. [:armenzg] from comment #24)
> > The guessed patch works (attachment 698184 [details] [diff] [review])
> > (tested on staging) but if we landed as-is it will turn the jetpack jobs on
> > Aurora red [1].
> 
> I'm ok with that, once m-c merges to aurora next week they should go green
> again. Beta and Release tinderboxen don't run the JP tests at the moment,
> I'll be looking to get them turned on once Fx 21 has ridden that far down
> the trees.
> 

Correct.

> > I also assume that the mozilla-beta [2] and mozilla-release [3] jobs on the
> > Jetpack tree will go red.
> 
> This should not be affecting the tests run on the Jetpack tree
> (https://tbpl.mozilla.org/?tree=Jetpack). Those should continue to use
> run_jetpack.py to get the SDK and Firefox build to test.
> 

Code-wise it seems like it.

The Addon-SDK tree should continue running the old way ("buildfarm/utils/run_jetpack.py")
http://mxr.mozilla.org/build/source/buildbotcustom/misc.py#2478

The Gecko jetpack jobs get generated with MozillaPackagedJetpackTests IIUC
http://mxr.mozilla.org/build/source/buildbotcustom/process/factory.py#5035
which is the code path the patch adjusts.

> > If this is fine with you guys I can request the review and have it enabled
> > before EOW.
> > 
> > Can I get a sign off on this plan?
> 
> Provided that this doesn't affect the Jetpack tree then I'm fine with this.

I'm not seeing it pass on staging.
'python' 'scripts/buildfarm/utils/run_jetpack.py' '-p' 'win7' '-t' 'http://hg.mozilla.org/users/armenzg_mozilla.com/jetpack/archive/tip.tar.bz2' '-b' 'mozilla-beta' '-f' 'ftp://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/%(branch)s-%(platform)s' '-e' 'win32.zip'

Do you know what this could mean?
SDK_URL: http://hg.mozilla.org/users/armenzg_mozilla.com/jetpack/archive/tip.tar.bz2
Traceback (most recent call last):
  File "scripts/buildfarm/utils/run_jetpack.py", line 268, in <module>
NameError: name 'sdkdir' is not defined
program finished with exit code 1
elapsedTime=19.495000

The full log is in here:
http://people.mozilla.com/~armenzg/bug826933/jetpack.log

Comment 27

[Dave Townsend [:mossop]]

(In reply to Armen Zambrano G. [:armenzg] from comment #26)
> (In reply to Dave Townsend (:Mossop) from comment #25)
> > (In reply to Armen Zambrano G. [:armenzg] from comment #24)
> > > If this is fine with you guys I can request the review and have it enabled
> > > before EOW.
> > > 
> > > Can I get a sign off on this plan?
> > 
> > Provided that this doesn't affect the Jetpack tree then I'm fine with this.
> 
> I'm not seeing it pass on staging.
> 'python' 'scripts/buildfarm/utils/run_jetpack.py' '-p' 'win7' '-t'
> 'http://hg.mozilla.org/users/armenzg_mozilla.com/jetpack/archive/tip.tar.
> bz2' '-b' 'mozilla-beta' '-f'
> 'ftp://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/%(branch)s-
> %(platform)s' '-e' 'win32.zip'
> 
> Do you know what this could mean?

Looks like run_jetpack.py expects the SDK code to be extracted to a dir named addon-sdk-<sha> but in your case it was extracted to jetpack-<sha>. I bet it's because your test version of the repo is called jetpack and not addon-sdk. Rather flakey code :(

Comment 28

[Armen [:armenzg]]

I noticed the same. I named my user repo incorrectly. I will try again.

Nevertheless, I'm confident that this should leave the Addon-SDK untouched.

Comment 29

[Armen [:armenzg]]

Comment on attachment 698184 [details] [diff] [review]
run hidden gecko jetpack jobs differently

This has run green for the gecko jetpack jobs (the ones that are hidden).

I'm just verifying (and I'm 99% sure) that it won't affect the Addon-SDK jobs (which code-path-wise it shouldn't).

Comment 30

[Aki Sasaki (not active)]

Comment on attachment 698184 [details] [diff] [review]
run hidden gecko jetpack jobs differently

Can you get this pep8 friendly?  (either indented 4 or lined up with the above lines)

Comment 31

[Armen [:armenzg]]

Comment on attachment 698184 [details] [diff] [review]
run hidden gecko jetpack jobs differently

http://hg.mozilla.org/build/buildbotcustom/rev/422bff524fde

Comment 32

[Armen [:armenzg]]

Re-triggered the following jobs to verify that everything goes as expected:
* all re-triggered
https://tbpl.mozilla.org/?noignore=1&jobname=jetpack&rev=31e89328fe12
* only one re-triggered (it should go red; it turned orange though)
https://tbpl.mozilla.org/?tree=Mozilla-Aurora&noignore=1&jobname=jetpack&rev=9e4a356f693e
* triggered few to see that they stay green:
https://tbpl.mozilla.org/?tree=Jetpack&rev=fa15e07eddcf

Comment 33

[Dave Townsend [:mossop]]

(In reply to Armen Zambrano G. [:armenzg] from comment #32)
> * only one re-triggered (it should go red; it turned orange though)
> https://tbpl.mozilla.org/?tree=Mozilla-
> Aurora&noignore=1&jobname=jetpack&rev=9e4a356f693e

I'm happy to live with that.

Thanks for all your work Armen!

Comment 34

[Armen [:armenzg]]

You're welcome!