Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

Initial work on #57 with cross-browser + node testing [do not merge] #77

Closed
wants to merge 1 commit into from
Closed

Initial work on #57 with cross-browser + node testing [do not merge] #77

wants to merge 1 commit into from

Conversation

victorb
Copy link
Contributor

@victorb victorb commented Oct 18, 2015

This is a PR work in progress, to fix #57 and to discuss the approach using testem.

Basically, I commented out the tests that are not currently working for the browser in one way or another. Removed the daemon control from inside the test and moved it out to a shell script instead.

This is all run with testem ( https://github.com/airportyh/testem ) but that can changed, important part is to make the test cases isolated from the setup the different environments need.

Issues with this: kind of slow startup, after that it works fine due to reloading of tests on filechange. Have to run different command when in CI environment.

Please let me know what you think of this and if it's worth keep working on.

Todo

  • make sure the startup script works correctly (some issues with the daemon starting currently)
  • make sure we really want to use testem vs karma (karma seems more popular+better support everywhere)
  • refactor tests to pass in nodejs environment
  • add saucelabs (or similar) integration
  • open issue to fix this library to pass in browser environments

Current status:

@daviddias
Copy link
Contributor

nice! Looking forward to this :)

Saw that the tool you picked has support for SauceLabs, which is awesome because we definitely will want CI :)

@dignifiedquire
Copy link
Contributor

noooooooo.. you have to use karma 😜

@dignifiedquire
Copy link
Contributor

In all seriousness, the benefit of using karma would be that I'm the core maintainer of it and you have direct line to the source so to speak ;)

@victorb
Copy link
Contributor Author

victorb commented Oct 19, 2015

@diasdavid thanks, you think the approach is right? Yeah, SauceLabs support was something I was looking for from the beginning.

@dignifiedquire Haha, yeah, Karma is good, used it in the past and would be trivial to port it, as long as the tests are isolated and can run in both browser+node. Have some issues with testem regarding starting the ipfs daemon before running the tests so might give Karma a shot instead... We'll see :)

@dignifiedquire
Copy link
Contributor

the tests are isolated and can run in both browser+node.

That shouldn't be a problem as long as the tests are written in a way that supports that

with testem regarding starting the ipfs daemon before running the tests so might give Karma a shot instead

Let me know if you have any karma specific questions, happy to help out :)

@victorb
Copy link
Contributor Author

victorb commented Oct 19, 2015

@diasdavid added a todo in the description of the PR, anything else I should do related to the tests?

Note to self regarding using karma

[11:23:24]  <dignifiedquire>    VictorBjelkholm: I know how you could solve it with karma, you can tell karma to serve arbitrary files and not include them, so that you can load them via simple http request, see http://karma-runner.github.io/0.13/config/files.html for more details on how to configure that. Then you could write a helper method, `getFile` which detects if it's run in the browser and then uses an http request to fetch the file or if it's run under
[11:23:24]  <dignifiedquire>    and fetch the file va fs.readFile

@victorb
Copy link
Contributor Author

victorb commented Oct 19, 2015

PR #78 seems a lot better and in my limited testing, works better as well. We should probably close this and focus on that one instead.

@daviddias
Copy link
Contributor

Thank you for figuring things out and joining forces @victorbjelkholm and @dignifiedquire :D :D Closing this one to focus on #78

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add CI for Node.js and Browser tests
3 participants