Closed Bug 1128099 Opened 9 years ago Closed 9 years ago

Write mozharness script for luciddream (dev tools harness)

Categories

(Testing :: General, defect)

Product:
Testing
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: jgriffin, Assigned: jgriffin)

References

(Blocks 1 open bug)

Details

Attachments

(4 files, 2 obsolete files)

mozharness script for luciddream,
21.64 KB, patch
armenzg
: review+
Details | Diff | Splinter Review
Add config for luciddream,
2.04 KB, patch
armenzg
: review+
Details | Diff | Splinter Review
Add missing mozharness options,
2.21 KB, patch
armenzg
: review+
Details | Diff | Splinter Review
Parse luciddream output and print a summary,
3.73 KB, patch
armenzg
: review+
Details | Diff | Splinter Review 
Ted has written a not-quite-functional prototype of a mozharness script to run the luciddream test harness.

The script prototype is at https://gist.github.com/luser/c1e948eb16d43e6863bf

The luciddream harness is at https://github.com/luser/luciddream

We need to finish the mozharness script, which should execute the luciddream test harness against a Firefox build and a Firefox OS emulator.
Ted, shall we check luciddream into the tree or would you prefer to leave it separate in github?
Flags: needinfo?(ted)
I think we should and did it in attachment 8569945 [details] [diff] [review].
I don't know yet where I am going with this patch, but that's handy!
I have a POC I'll attach soon.
Assignee: nobody → jgriffin
(In reply to Jonathan Griffin (:jgriffin) from comment #4)
> Created attachment 8570131 [details] [diff] [review]
> mozharness script for luciddream, WIP

This works locally, but needs some cleanup before it's ready for production.  CLI I use locally to test:

python scripts/luciddream_unittest.py --installer-url http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64/latest/firefox-39.0a1.en-US.linux-x86_64.tar.bz2 --test-url http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux64/latest/firefox-39.0a1.en-US.linux-x86_64.tests.zip --b2g-desktop-path ~/b2gdesktop/obj-x86_64-unknown-linux-gnu/dist/bin/b2g --xre-url http://tooltool.pvt.build.mozilla.org/build/sha512/64b655694963a05b9cf8ac7f2e7480898e6613714c9bedafc3236ef633ce76e726585d0a76dbe4b428b5142ce85bebe877b70b5daaecf073e592cb505690839f
We will eventually move luciddream into the tree, so the luciddream cloning step will go away, but this is sufficient to start testing the script in CI.  If you don't have time to review, let me know and I'll pass to someone else.
Attachment #8571676 - Flags: review?(armenzg)
Attachment #8570131 - Attachment is obsolete: true
I'm totally on board with checking it into the tree.
Flags: needinfo?(ted)
Comment on attachment 8571676 [details] [diff] [review]
mozharness script for luciddream,

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

This is in the right direction, however, there are some pieces of code that should belong to GaiaMixin which would require more work and testing that we don't regress other scripts.
If you have time and could fix it, it would be great.
If you just want to get something commited for others to use let's only deal with non-refactoring comments that I made.

::: scripts/luciddream_unittest.py
@@ +206,5 @@
> +        tar = self.query_exe('tar', return_type='list')
> +        self.run_command(tar + ['zxf', self.emulator_path],
> +                         cwd=dirs['abs_emulator_dir'],
> +                         error_list=TarErrorList,
> +                         halt_on_failure=True, fatal_exit_code=3)

There's an EmulatorMixin inside of unittest.py which the Android jobs use if you would like to reshare code.
However, I don't expect it since it is not straight forward.

@@ +235,5 @@
> +                self.mkdir_p(parent_dir, error_level=FATAL)
> +            self.run_command(command,
> +                             cwd=parent_dir,
> +                             error_list=ZipErrorList,
> +                             halt_on_failure=True, fatal_exit_code=3)

This functionality should be in GaiaMixin instead of GaiaTest.
This is part of our technichal debt in mozharness.
If you're rushed for time I won't require you to *only* refactor this part.
I had a wip a while back in here if you would like to tackle this:
https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=1071219&attachment=8498860

If not, please add a comment saying that this should be refactor as in bug 1071219.

@@ +240,5 @@
> +
> +    def install_b2g_desktop(self):
> +        dirs = self.query_abs_dirs()
> +        self.mkdir_p(dirs['abs_b2g_desktop'])
> +        b2gdesktop = self.download_file(self.config.get('b2gdesktop_url'), 

nit: white space.

@@ +269,5 @@
> +                'repo_path': 'https://hg.mozilla.org/%s' % self.buildbot_config['properties']['repo_path']
> +            })
> +
> +        self.clone_gaia(dest, repo,
> +                        use_gaia_json=self.buildbot_config is not None)

Up to here is the same logic as "pull" from the GaiaMixin.

@@ +279,5 @@
> +                                         self.config.get('xre_path'),
> +                                         'bin', 'xpcshell')
> +            if not os.access(xulrunner_bin, os.F_OK):
> +                xre = self.download_file(xre_url, parent_dir=dirs['abs_work_dir'])
> +                self.extract_xre(xre, parent_dir=dirs['abs_gaia_dir'])

This is part of download_and_extract of GaiaTest. This is also something that should be in GaiaMixin.
Again, I won't require you to do the refactoring if you're in a rush, however, I would like to have a comment in here in case someone passes by and would like to work on it.

@@ +287,5 @@
> +        self.make_gaia(dirs['abs_gaia_dir'],
> +                       self.config.get('xre_path'),
> +                       debug=False,
> +                       noftu=False,
> +                       build_config_path=None)

I think make_gaia should also be part of GaiaMixin and make all gaia related scripts use a "make_gaia" action.
Anyways, don't worry about this.

@@ +297,5 @@
> +            self.install_emulator()
> +        if self.config.get('b2gdesktop_path') or self.config.get('b2gdesktop_url'):
> +            self.setup_gaia()
> +        if self.config.get('b2gdesktop_url'):
> +            self.install_b2g_desktop()

Could you please create an install function in here, call TestingMixin's install and then do your install steps there?

@@ +298,5 @@
> +        if self.config.get('b2gdesktop_path') or self.config.get('b2gdesktop_url'):
> +            self.setup_gaia()
> +        if self.config.get('b2gdesktop_url'):
> +            self.install_b2g_desktop()
> +        self.clone_luciddream()

Can you please move all actions related to fetching repos to a new action called "pull"? Similar to what we do for GaiaTest.
Attachment #8571676 - Flags: review?(armenzg) → review-
Updated per review comments.  Some explanation:

- I moved extract_xre out of all scripts and bundled in inside of make_gaia in GaiaMixin.  This is needed by the gaia make process and is best consolidated there, IMO.

- I created an install method and put the installation of b2gdesktop there.  I had to modify the parent method in TestingMixin to handle the case where we're installing a second binary, as is the case here.

- I didn't attempt to use EmulatorMixin.  That code is really pretty Android specific and it would take a lot of work that isn't really needed to make it generic.

- I removed the code to clone gaia and am relying on GaiaMixin to do that for us.  I consolidated all of the repo activity in a new pull method.
Attachment #8572708 - Flags: review?(armenzg)
Attachment #8571676 - Attachment is obsolete: true
After this gets an r+, I'll run this on try before checking in.
Comment on attachment 8572708 [details] [diff] [review]
mozharness script for luciddream,

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

r+ with the changes mentioned. Thanks for cleaning this up!

::: scripts/luciddream_unittest.py
@@ +206,5 @@
> +        dirs = self.query_abs_dirs()
> +        self.mkdir_p(dirs['abs_emulator_dir'])
> +        tarfile = self.download_proxied_file(self.emulator_url,
> +                                             parent_dir=dirs['abs_work_dir'],
> +                                             error_level=FATAL)

Could you please use download_file()? It will use download_proxied_file() only when not running on developer_mode.
http://hg.mozilla.org/build/mozharness/file/default/mozharness/mozilla/testing/testbase.py#l115

@@ +217,5 @@
> +
> +    def install(self, **kwargs):
> +        super(LuciddreamTest, self).install(**kwargs)
> +        dirs = self.query_abs_dirs()
> +        self.install_app(app='b2g', 

Nit: white space.
Attachment #8572708 - Flags: review?(armenzg) → review+
(In reply to Jonathan Griffin (:jgriffin) from comment #12)
> nits addressed, pushed to try: 
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb707467be4d

I neglected to remove the obsolete version of extract_xre from gaia_test.py.  Fixed, and a new try run:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d68bef2c21f3
https://hg.mozilla.org/build/mozharness/rev/f31676ebc518

Next, will write a corresponding config file.
This is a provisional mozharness config for testing luciddream on cedar; I'll make a separate patch later for introducing in-tree config.  b2g_desktop_url is hardcoded by design...this tests the current Firefox build against a set build of b2g_desktop.  Will add some more flexibility here in the future.
Attachment #8574929 - Flags: review?(armenzg)
Attachment #8574929 - Flags: review?(armenzg) → review+
This fixes some problems seen on cedar, namely missing read-buildbot-config step and missing blobber config options.
Attachment #8577023 - Flags: review?(armenzg)
Comment on attachment 8577023 [details] [diff] [review]
Add missing mozharness options,

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

Fun!
Attachment #8577023 - Flags: review?(armenzg) → review+
fyi, Armen is probably not going to get to this until Monday.
Comment on attachment 8580988 [details] [diff] [review]
Parse luciddream output and print a summary,

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

Excellent!
Attachment #8580988 - Flags: review?(armenzg) → review+
Script is running well; I'll open separate bugs for any follow-ups.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: