Add socket host, socket reporter and corredor-js to gaia-node-modules

RESOLVED FIXED

Status

Firefox OS
Gaia::TestAgent
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: ahal, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Reporter)

Comment 1

4 years ago
Created attachment 8456398 [details] [review]
Add corredor-js, marionette-socket-host and mocha-socket-reporter to gaia-node-modules

Hey Gareth, here's a pull request to add the previous repos you've reviewed to gaia-node-modules.

I can hold off on this PR if there are still issues you'd like me to fix with marionette-socket-host first.

Thanks!
Attachment #8456398 - Flags: review?(gaye)
Comment on attachment 8456398 [details] [review]
Add corredor-js, marionette-socket-host and mocha-socket-reporter to gaia-node-modules

LGTM. Before you land you should go ahead and test though by

(1) creating a fork of gaia
(2) creating a feature branch
(3) modifying https://github.com/mozilla-b2g/gaia/blob/master/Makefile#L711 to point to your [remote] branch of gaia-node-modules
(4) Changing $gaia/gaia_node_modules.revision to match the sha of your commit to your fork of gaia-node-modules
(5) Try running |make clean && make really-clean| and then trying your REPORTER=marionette-socket-host make test-integration etc, etc and note whether it does what you expect for it to
Attachment #8456398 - Flags: review?(gaye) → review+
(Reporter)

Comment 3

4 years ago
(In reply to Gareth Aye [:gaye] from comment #2)
> Comment on attachment 8456398 [details] [review]
> Add corredor-js, marionette-socket-host and mocha-socket-reporter to
> gaia-node-modules
> 
> LGTM. Before you land you should go ahead and test though by
> 
> (1) creating a fork of gaia
> (2) creating a feature branch
> (3) modifying https://github.com/mozilla-b2g/gaia/blob/master/Makefile#L711
> to point to your [remote] branch of gaia-node-modules
> (4) Changing $gaia/gaia_node_modules.revision to match the sha of your
> commit to your fork of gaia-node-modules
> (5) Try running |make clean && make really-clean| and then trying your
> REPORTER=marionette-socket-host make test-integration etc, etc and note
> whether it does what you expect for it to

Yep, I actually did this already with https://github.com/ahal/gaia-node-modules/tree/add_socket_repos (though just on a local branch of gaia). Worked as expected. Thanks for the review!
(In reply to Gareth Aye [:gaye] from comment #2)
> Comment on attachment 8456398 [details] [review]
> Add corredor-js, marionette-socket-host and mocha-socket-reporter to
> gaia-node-modules
> 
> LGTM. Before you land you should go ahead and test though by
> 
> (1) creating a fork of gaia
> (2) creating a feature branch
> (3) modifying https://github.com/mozilla-b2g/gaia/blob/master/Makefile#L711
> to point to your [remote] branch of gaia-node-modules
> (4) Changing $gaia/gaia_node_modules.revision to match the sha of your
> commit to your fork of gaia-node-modules
> (5) Try running |make clean && make really-clean| and then trying your
> REPORTER=marionette-socket-host make test-integration etc, etc and note
> whether it does what you expect for it to

Gareth, would make sense to put this in gaia-node-module's README file?
(Reporter)

Comment 5

4 years ago
(In reply to Julien Wajsberg [:julienw] from comment #4)
> (In reply to Gareth Aye [:gaye] from comment #2)
> > Comment on attachment 8456398 [details] [review]
> > Add corredor-js, marionette-socket-host and mocha-socket-reporter to
> > gaia-node-modules
> > 
> > LGTM. Before you land you should go ahead and test though by
> > 
> > (1) creating a fork of gaia
> > (2) creating a feature branch
> > (3) modifying https://github.com/mozilla-b2g/gaia/blob/master/Makefile#L711
> > to point to your [remote] branch of gaia-node-modules
> > (4) Changing $gaia/gaia_node_modules.revision to match the sha of your
> > commit to your fork of gaia-node-modules
> > (5) Try running |make clean && make really-clean| and then trying your
> > REPORTER=marionette-socket-host make test-integration etc, etc and note
> > whether it does what you expect for it to
> 
> Gareth, would make sense to put this in gaia-node-module's README file?

+1, I'll add a note to the README about this along with this patch. For the record you also have to modify this line [1] and s/mozilla-b2g/<your github username>. Maybe we should also pull that and [2] into a new variable in the Makefile to make this a bit easier? Possibly even make it default to an environment variable?

[1] https://github.com/mozilla-b2g/gaia/blob/master/Makefile#L726
[2] https://github.com/mozilla-b2g/gaia/blob/master/Makefile#L711
(Reporter)

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Reporter)

Comment 7

4 years ago
This broke gaia-build tests presumably because we tried to run zmq's tests without libzmq installed on the machine:
https://tbpl.mozilla.org/php/getParsedLog.php?id=44299637&tree=Gaia-Try&full=1

Is there a way to skip the tests of a particular module in Gb?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 8

4 years ago
I reverted it for now until I can figure out what to do about it:
https://github.com/mozilla-b2g/gaia-node-modules/commit/86c8f00c4301fbc8fa4a5ffa04744678af08f783
(Reporter)

Comment 9

4 years ago
So there's basically two options.

1) Install libzmq prior to |make node_modules| in the Makefile. Probably the easiest way to do this is to download the tarball [1] and run |configure && make && make install| and point it somewhere in user space. This could potentially cause problems for people who don't care at all about running gaia-integration though.

-or-

2) Don't include these packages in gaia-node-modules and have whatever entry point we end up using (e.g Makefile, mach target..) be responsible for installing libzmq as well as all three of these packages.

[1] http://download.zeromq.org/zeromq-4.0.4.tar.gz
(Reporter)

Comment 10

4 years ago
I guess a third option is to update our test slave images to include libzmq, but this doesn't solve the case where a dev would want to run tests locally.
Is there a way to detect that libzmq is missing and abort cleanly suggesting to install it?
(Reporter)

Comment 12

4 years ago
Yeah that's another option, and might even be the best one.

I actually already have a patch that downloads and compiles it if it isn't detected. Doing it this way we'd need to install it in user space though which might cause other complications :/

I'll file another bug about this, upload my WIP patch and we can figure out the best way forward from there.
(Reporter)

Updated

4 years ago
Depends on: 1042183
(Reporter)

Updated

4 years ago
Assignee: ahalberstadt → nobody
Bug 1088835 resolved this
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Not really as I explained in my latest comment there :/
You need to log in before you can comment on or make changes to this bug.