Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

more explicit node dependency #36

Merged
merged 1 commit into from
Feb 16, 2016
Merged

more explicit node dependency #36

merged 1 commit into from
Feb 16, 2016

Conversation

jedireza
Copy link
Contributor

@jedireza jedireza commented Feb 9, 2016

It wasn't obvious that I need to have a node to run the selenium tests. This PR adds a package.json file so we can have a proper place for dev dependencies as well as defining the version of node that is supported (via the engines key).

Now locally we can simply setup via:

$ npm install

And then run the tests by having the Rust server running on port 3000 and:

$ npm test

馃摕 @michielbdejong @JohanLorenzo

If this is ok with you I'd like to add some instructions about this to the readme before merging.

@michielbdejong
Copy link
Contributor

About adding package.json - if that's ok with @fabricedesre then I also fully support that change - I kept it out because nodejs is only used for running the GUI tests right now.

About setting the hostname back to localhost - right now, there is no build process that sets the foxbox.local hostname, but I would like this to exist in the future, that's why I kinda like it being there in the GUI tests and in the user manual.

@fabricedesre
Copy link
Collaborator

I'm fine with adding package.json too.

About the foxbox.local name setting, it should be set by the daemon itself to reflect the --name command line argument. We'll need the avahi bindings that @azasypkin is working on and implement something similar to http://www.avahi.org/wiki/Examples/PythonPublishAlias (without dbus).

@jedireza
Copy link
Contributor Author

I updated the README in #37 since I changed more than just adding details for Node. We should merge this first and that second.

@jedireza
Copy link
Contributor Author

About setting the hostname back to localhost - right now, there is no build process that sets the foxbox.local hostname, but I would like this to exist in the future, that's why I kinda like it being there in the GUI tests and in the user manual.

I'm not a fan of the added complexity of needing to edit my /ect/hosts just to run tests locally, especially when I have the server (http://localhost:3000/) running already.

@JohanLorenzo
Copy link
Contributor

This PR adds a package.json

馃憤

I'm not a fan of the added complexity of needing to edit my /ect/hosts

I agree. What do you all think of:

  • using localhost as the default hostname for selenium tests
  • and create a separate test called FoxBox should be accessible by the domain name called "foxbox.local" (given by avahi daemon) ?

Then, if such a test failed on one's computer, the failure would document the intent.

@jedireza
Copy link
Contributor Author

Rebased to resolve conflicts. r? @fabricedesre

fabricedesre added a commit that referenced this pull request Feb 16, 2016
more explicit node dependency
@fabricedesre fabricedesre merged commit 266bbfe into fxbox:master Feb 16, 2016
@jedireza jedireza deleted the explicit-node branch February 20, 2016 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants