Skip to content

Node 7 + Travis #249

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gauntface opened this issue Oct 28, 2016 · 8 comments
Closed

Node 7 + Travis #249

gauntface opened this issue Oct 28, 2016 · 8 comments

Comments

@gauntface
Copy link

I found a few issues on Travis + Node 7 + Selenium + Firefox.

It seemed liked permissions weren't being set correctly for notifications OR the prompt required interaction.

I ended with the following changes / additions to selenium to get it running: GoogleChromeLabs/web-push-testing-service#2

Worth keeping in mind if / when we add Node 7 to travis.

@marco-c
Copy link
Member

marco-c commented Nov 2, 2016

Interesting that security.turn_off_all_security_so_that_viruses_can_take_over_this_computer is working, it shouldn't as of https://bugzilla.mozilla.org/show_bug.cgi?id=1277691.

I found a few issues on Travis + Node 7 + Selenium + Firefox.

Did it work locally (not on Travis)?

@gauntface
Copy link
Author

gauntface commented Nov 2, 2016

I thought it worked locally at one point, but last time I tried it didn't work locally (i.e. kept asking for permission) and then Node 7 + Travis it wasn't working, would just time out, matching what I was seeing.

To be honest, it's all a bit weird and I could be barking up the wrong tree / mixing up what is actually happening.

Looking at your referenced issue, the pref looks like it's still picked up: https://hg.mozilla.org/mozilla-central/rev/1e7b38078918#l3.36 It just has an extra check before it's used.

@marco-c
Copy link
Member

marco-c commented Nov 4, 2016

@gauntface have you tried updating the selenium-webdriver package?

The only recent change I can see in geckodriver that might be related is the renaming of the firefoxOptions option to moz:firefoxOptions. Perhaps the latest selenium-webdriver package was updated to handle that.

@marco-c
Copy link
Member

marco-c commented Nov 4, 2016

Yes, from https://github.com/SeleniumHQ/selenium/blob/master/javascript/node/selenium-webdriver/CHANGES.md#v300:

Update the selenium-webdriver/firefox module to use geckodriver's "moz:firefoxOptions" dictionary for Firefox-specific configuration values.

I'm creating a PR to pin some of our packages to a specific version, because the tests are currently broken. We should decide what to do (selenium-webdriver 3 requires Node 6.9.0).

@marco-c
Copy link
Member

marco-c commented Nov 4, 2016

In my opinion, we should support Node.js versions that are still in their active LTS period, especially because I don't see a compelling reason to update quickly.

So, I think we should still support Node.js 4 until Apr 2017: https://github.com/nodejs/LTS/#lts-schedule.

What do you think?

@marco-c
Copy link
Member

marco-c commented Nov 4, 2016

I filed SeleniumHQ/selenium#3059.

@gauntface
Copy link
Author

Yes we should sorry node4 but I'd keep to the latest selenium webdriver. Firefox had been a real headache and updating to the latest has generally been the easiest approach to get it working.

The selenium tests can be run on node 6+ and should give us the test coverage we need.

I'll pin the deps on selenium-assistant as well as that gave me a tonne of headaches with web push php tests.

@gauntface
Copy link
Author

Closing this as you got node 7 working and the permissions aren't coming up, so presumably it was still a selenium-webdriver issue.

I've raised a PR to add Node 6 to the test matric.

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

No branches or pull requests

2 participants