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

Already on GitHub? Sign in to your account

Work In Progress: added sauce labs test support #74

Closed
wants to merge 1 commit into from

Conversation

wesleytodd
Copy link
Member

This is the initial work for running these test in browsers on Sauce Labs. I ran them using my credentials used for another project and it all passed in Chrome, FF and IE10. We just need a few things:

  • Sauce Labs account for the project
  • Add encrypted key to the .travis.yml
  • Add a badge to the README.md once we have the account setup

Here are instructions on encrypting the key. My assumption would be that @dougwilson would want to have all the accounts and place the ownership under the Node Foundation, but that is up to the @pillarjs/express-tc I guess. Let me know when you have the encrypted keys, or just take over this PR.

Here is what my working .travis.yml looks like for reference: https://github.com/wesleytodd/nighthawk/blob/master/.travis.yml

@jonathanong
Copy link
Member

@wesleytodd the test failed but it's hard to see what happened on my phone. Any ideas? Do we need to change something with Travis?

@vvo
Copy link

vvo commented Mar 26, 2016

Would put the keys public, otherwise PRs will not be able to run tests

@wesleytodd
Copy link
Member Author

@jonathanong The tests failed because I added the scaffold for running sauce labs, but we need the actually account and keys for it to succeed. It will not pass until we can add those to the .travis.yml.

@vvo The way you integrate these into travis and sauce labs is documented in the link I posted above, which encrypts them so that they cannot be used publicly, but they CAN be used when running the tests for this repo. Just need those to be generated as the link shows and this will start working.

@wesleytodd
Copy link
Member Author

If someone wants to trust me to set this up for the project I can, but I don't want to presume. If that is fine then I can forward the credentials I setup to the @pillarjs/express-tc and they can change them as they see fit. That would enable me to get the test to run and pass. Otherwise I need someone to actually run the setup steps I listed above before this PR will pass.

@vvo
Copy link

vvo commented Mar 27, 2016

@wesleytodd sorry was not precise enough, you will be able to run tests for PRs for repo branches but not from external forks

@wesleytodd
Copy link
Member Author

@vvo Ahh, I see your point. I still don't think that is a good enough reason to make the keys public. Personally I would be fine if forks can only run in phantomjs. The PR will still run just fine in sauce labs, and if the contributor really wants to run them then can setup their own account or run selenium locally.

That all being said, I would be interested in others opinions on that.

@vvo
Copy link

vvo commented Mar 28, 2016

Example in ember.js with public keys: https://github.com/emberjs/ember.js/blob/master/.travis.yml#L31

It all depends on how much you expect to have external contributor, but yes if phantomjs is sufficient for external forks, why not.

@wesleytodd
Copy link
Member Author

I am not sure what all that is in the ember .travis.yml. I am assuming that I could run a command like SAUCE_USERNAME=ember-ci SAUCE_ACCESS_KEY=b5cff982-069f-4a0d-ac34-b77e57c2e295 npm run test-browser and be running with their credentials.

That above command is what I ran while testing for this repo, and it worked just fine with my credentials which have nothing to do with this project. We don't want to run malicious, or even different, code on our sauce labs account. The best case scenario allows a user to make the tests look like they are failing when they are not.

@wesleytodd
Copy link
Member Author

*I didnt try it with the ember repo because I clearly didn't want to mess with their project, so maybe I am wrong, feel free to correct me.

@jonathanong
Copy link
Member

ah so sauce labs won't even be able to run in PRs, huh? that sucks

@wesleytodd
Copy link
Member Author

It should run in PR's, feel free to open a test one on that other repo if we want to test.

I think the issue @vvo was talking about was when you fork you cannot manually run the tests on sauce labs.

@vvo
Copy link

vvo commented Mar 28, 2016

ah so sauce labs won't even be able to run in PRs, huh? that sucks

"Please note that encrypted environment variables are not available for pull requests from forks."

https://docs.travis-ci.com/user/pull-requests#Security-Restrictions-when-testing-Pull-Requests

This is the only thing I wanted to point. It has been a pain for me because every "external contributor" fork was not able to run tests. But you could also say that external forks PRS are only phantomjs (but it's a bit annoying).

@wesleytodd
Copy link
Member Author

Yeah well that sucks :)

I guess I will throw [ "${TRAVIS_PULL_REQUEST}" = "false" ] in there if we want to keep the "secret key" secret, but if @jonathanong and @dougwilson don't think that is important then we can just add the un-encrypted keys.

@jonathanong
Copy link
Member

from a contributor's standpoint, i would hate if my tests didn't run in browsers and something ended up breaking. what do you think, @dougwilson? i think at least running phantomjs would be an okay compromise.

@dougwilson
Copy link

I think in general, the tests running for external contributors (the term GitHub uses) is more of an issue for the project maintainers. If they run, then it's easy for a project maintainer to know if the pull request is acceptable, without a lot of additional work. I'm not sure what the implications of exposing the keys really are, though.

@vvo
Copy link

vvo commented Mar 30, 2016

If you expose the saucelabs keys of an already open source project then the only drawbacks I can see is that someone would use your saucelabs keys (to run .. browsers) and at some point your account would be blocked.

But that's really not something that should happen, since saucelabs is already free for opensource, why would you do that?

I would just go for public keys and move on

@wesleytodd
Copy link
Member Author

I am fine with public keys if everyone else is.

@crandmck
Copy link

crandmck commented Apr 7, 2016

@rmg do you have any experience or thoughts on this matter? Is there any risk of having public keys that we're not thinking of?

@rmg
Copy link

rmg commented Apr 7, 2016

@crandmck it's not something I would do, but I don't really have a horse in this particular race.

Someone should probably check with SauceLabs to see what they recommend for taking advantage of the offer without taking advantage of them.

@ForbesLindesay
Copy link
Contributor

For what it's worth, I've just published my keys for all the public modules I test on sauce labs. Hasn't been an issue so far :)

@wesleytodd
Copy link
Member Author

Ok, I will take a look this weekend and get a key that we can publicly add. I will post the account details here so the tc can take over the account.

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

8 participants