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

Add browser test suite to CI #2079

Closed
danielstjules opened this issue Jan 27, 2016 · 18 comments · Fixed by #2231
Closed

Add browser test suite to CI #2079

danielstjules opened this issue Jan 27, 2016 · 18 comments · Fixed by #2231
Assignees

Comments

@danielstjules
Copy link
Contributor

Should hopefully catch some issues going forward. Very simple cross-browser tests with testling.com/saucelabs would be nice too.

@boneskull
Copy link
Contributor

this is necessary before we do any release post-2.4.4.

@danielstjules
Copy link
Contributor Author

Any preference on how this should be done?

  • Testling could be configured to use mocha's latest master as a dev dependency https://ci.testling.com/guide/mocha However, it would make testing a bit awkward, and wouldn't work on PRs. For example, how could you test failures/reporter-output/etc?
  • Saucelabs with http://webdriver.io/ would certainly work, and could be used to test reporter output in different conditions (failure, success, etc) and browsers. However, it can't run on PRs. Possible workaround: https://github.com/twbs/savage
  • casperjs or https://github.com/segmentio/nightmare for webkit testing. Faster, less prone to sporadic network failures as with Testling/Saucelabs. Would work on PRs. But wouldn't give us cross-browser coverage

@danielstjules
Copy link
Contributor Author

How much do we value cross-browser integration/acceptance tests over just plain-old automated browser tests in, e.g. phantomjs? Most of our IE8 issues thus far could have been detected with a grep (it's almost always Array.prototype.forEach lol) Aside from that, we've hit some small snags with Safari in the past.

We could certainly kill two birds with one stone, but I feel like the initial focus might be just hooking up basic browser tests with CI?

@tschaub
Copy link

tschaub commented Jan 27, 2016

Cross browser testing would be nice. But even testing in Phantom would help. You could take the two tasks on separately. Phantom only tests could be done on Travis.

Saucelabs with http://webdriver.io/ would certainly work, and could be used to test reporter output in different conditions (failure, success, etc) and browsers. However, it can't run on PRs.

You can use Sauce Connect on Travis and have your tests run in as many browsers as you want on every commit (PRs included).

You'll find the same thing elsewhere, but here's an example Karma and Travis config and package.json scripts for running tests in Node + Browsers (locally and via Sauce Connect in Travis).

@danielstjules
Copy link
Contributor Author

@tschaub From your link at https://docs.travis-ci.com/user/sauce-connect/

Note that due to security restrictions, the Sauce Labs addon is not available on pull request builds. See the pull requests page for full details on why it is disabled.

This is why my comment mentioned:

Possible workaround: https://github.com/twbs/savage

@tschaub
Copy link

tschaub commented Jan 27, 2016

You're right @danielstjules. Was thinking I'd used it elsewhere, but I've only set it up for pull requests issued from the same origin. Makes sense that the SAUCE_ACCESS_KEY is encrypted per repo.

@danielstjules
Copy link
Contributor Author

No worries :) But I agree, with your earlier point. We could definitely break this down into multiple tasks.

@boneskull
Copy link
Contributor

Yes, yes, and yes. Some notes/questions:

  • Use PhantomJS in Travis as a smoke test. Hell, you might even get 2.x working. @mantoni's mochify might be a shortcut to get something going here, as it bundles up the workflow into a convenient package.
  • You could try to get savage working, but it looks hairy just glancing at its README.
  • I'm unfamiliar with using Saucelabs, so:
    • I would like to understand the risk of exposing the Saucelabs API key.
    • We don't have (functional) Selenium tests; is this a requisite to use Saucelabs? Or can we just run our unit/integration tests on it?

@boneskull
Copy link
Contributor

We could certainly kill two birds with one stone, but I feel like the initial focus might be just hooking up basic browser tests with CI?

I agree with this. Get something running w Phantom on Travis.

@danielstjules
Copy link
Contributor Author

We don't have (functional) Selenium tests; is this a requisite to use Saucelabs? Or can we just run our unit/integration tests on it?

Unfortunately no, we'd need to write our own integration tests. Our browser unit tests aren't really unit tests. Some of them fail, for example, to show that error handling works in the html reporter. So we'd need to write tests that wrap and verify that functionality.

@danielstjules
Copy link
Contributor Author

I agree with this. Get something running w Phantom on Travis.

Sounds good to me!

@boneskull
Copy link
Contributor

re cross-browser compatibility: if we're just looking at testing our JavaScript, there are two browsers. IE8 and everything else. afaik per kangax's tables, anything else non-ES5-compliant has insignificant usage.

@boneskull
Copy link
Contributor

@danielstjules

Unfortunately no, we'd need to write our own integration tests. Our browser unit tests aren't really unit tests. Some of them fail, for example, to show that error handling works in the html reporter. So we'd need to write tests that wrap and verify that functionality.

Hmm, this isn't quite what I meant. I'm not sure how helpful those "browser" tests are. I mean just running the entire suite of server-side tests, excluding inappropriate ones. We should have tests for the HTML reporter as it's integral to mocha's browser functionality. whether those are the existing tests or new ones, I dunno.

@mantoni
Copy link
Contributor

mantoni commented Jan 28, 2016

There are two projects that Mochify uses to get the browserified tests running in PhantomJS and on SauceLabs. These should get you somewhere pretty quickly.

For PhantomJS, that is https://github.com/mantoni/phantomic

$ browserify ./test.js | phantomic

And for WebDriver / SauceLabs support, it's https://github.com/mantoni/min-webdriver

$ browserify -p min-wd ./test.js

In both cases, the raw script is executed without any wrappers or framework.
Hope that helps.

@boneskull
Copy link
Contributor

@danielstjules Did you make any progress here?

@ScottFreeCode
Copy link
Contributor

Is this something that could be helped/fixed by someone outside the project such as myself? E.g. does it just need to get to the point where firing off a bunch of PhantomJS tests (or something like that) can be done just by an NPM command and then adding that command to the CI server is trivial? Or is it going to be more a matter of getting the CI server or a third-party testing service set up such that only a team member really should have the access needed?

I'm wondering because, if this is going to block any other fixes from getting published, I as a user would be eager to try to push it through. That and I need to explore automated JS testing anyway, and I figure there must be at least some overlap with automated testing for Mocha itself, so it might be a great learning opportunity.

@ScottFreeCode
Copy link
Contributor

Also, what's the relationship between this issue and #1732?

@dasilvacontin
Copy link
Contributor

dasilvacontin commented Apr 22, 2016

Sorry for the delay. This one is a dupe of #1732. I've closed #1732 due to having a much shorter discussion.

I believe what we want is to have every PR tested on at least IE8. To accomplish so, I think I've used SauceLabs + zuul for some other projects (thanks to @ndhoule for mentioning it in #1732). I'll take a shot this morning, see what comes out.

I appreciate the positive and constructive comments you've left around a few issues, btw. :)

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 a pull request may close this issue.

6 participants