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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::TestAgent, defect)

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: 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!

Comment 2

[Gareth Aye [:gaye] (back from PTO)]

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

Comment 3

[Andrew Halberstadt [:ahal]]

(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!

Comment 4

[Julien Wajsberg [:julienw]]

(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?

Comment 5

[Andrew Halberstadt [:ahal]]

(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

Comment 6

[Andrew Halberstadt [:ahal]]

https://github.com/mozilla-b2g/gaia-node-modules/commit/476aded15da0bf577557e36fe3d05daad975859a

README.md change:
https://github.com/mozilla-b2g/gaia-node-modules/commit/39f44c720a041fe1a6bd07c9bc18452776338b9a

I'll update gaia_node_modules.revision as part of bug 994847.

Comment 7

[Andrew Halberstadt [:ahal]]

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?

Comment 8

[Andrew Halberstadt [:ahal]]

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

Comment 9

[Andrew Halberstadt [:ahal]]

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

Comment 10

[Andrew Halberstadt [:ahal]]

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.

Comment 11

[Julien Wajsberg [:julienw]]

Is there a way to detect that libzmq is missing and abort cleanly suggesting to install it?

Comment 12

[Andrew Halberstadt [:ahal]]

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.

Comment 13

[Malini Das [:mdas] - Away, not checking bugmail]

Bug 1088835 resolved this

Comment 14

[Julien Wajsberg [:julienw]]

Not really as I explained in my latest comment there :/