search

kefirjs / kefir

A Reactive Programming library for JavaScript
https://kefirjs.github.io/kefir/
MIT License
1.87k stars 97 forks source link

Async tests fail in browser #213

Closed rpominov closed 7 years ago

rpominov commented 8 years ago

This is an issue with tests themselves, library code is fine.

http://rpominov.github.io/kefir/test/in-browser/SpecRunner.html

image

mAAdhaTTah commented 8 years ago

Looks related to this: https://github.com/mhevery/jasmine-node/issues/371

mAAdhaTTah commented 8 years ago

I'm fairly familiar with Karma for running unit tests in a browser. Any interest in using that as a test runner? We'd also be able to solve the "ES6 in tests" problem, as we can use a browserify preprocessor on the tests before running them.

rpominov commented 8 years ago

I used Karma and I like how it works in watch mode and creates quick response loop. So maybe it would make sense to use Karma to improve dev experience. But while developing we probably don't want to run tests in browser, and I'm not sure Karma supports running in node (does it?).

As for this particular bug, I don't think Karma will help directly. I'm guessing that problem is that we use different versions of Jasmine on node and in browser. One supports async tests and one don't. Btw we do use browserify already to build browser tests (see https://github.com/rpominov/kefir/blob/master/Gruntfile.coffee#L29).

Also I kinda like the "Run tests in your browser" link. Will it be possible to create such bundle with Karma?

I think Karma is cool, but just don't know if it will solve our problems and won't create new ones. I think main problem with current setup is dev experience, it's takes too long to run the tests every time, a good watch mode would help. And this bug is kinda minor, probably not too hard to fix, I just didn't look at it to be honest.

mAAdhaTTah commented 8 years ago

But while developing we probably don't want to run tests in browser, and I'm not sure Karma supports running in node (does it?).

Not sure of that exactly, but it does support running in PhantomJS, which would be headless, so node-like (CLI-only).

Also I kinda like the "Run tests in your browser" link. Will it be possible to create such bundle with Karma?

No reason we couldn't continue to generate that bundle with grunt.

Up to you if you want it, but since I'm bouncing around this repo, figured I'd try and solve as many problems as possible :).

rpominov commented 8 years ago

Did a bit of research. The tests fail because we use Jasmine 1.3 which doesn't support it('', done => ...) functionality. But in jasmine-node a modified version of Jasmine 1.3 is used, and it supports done. The jasmine-node package seems abandoned and it doesn't support Jasmine 2.x, which makes it harder for us to upgrade to Jasmine 2.x. Seems like the most correct way of running Jasmine on node is to use jasmine package, but it doesn't seem to support preprocessing out of the box (we need it for coffee and es6+). Not sure what we should do yet, but two thoughts I have so far: 1) This particular issue is harder to fix than I thought 2) The test environment is very outdated and needs overall upgrade.

As for Karma, it doesn't support node, and if we add Karma it won't be able to replace any tools that we use (we still will need something to run on node, and to create "Run tests in your browser" bundle). So Karma will be just additional tool in the testing environment, not sure it's worth it.

mAAdhaTTah commented 8 years ago

It's actually a good thing you did this research, because I was considering switching from Mocha -> Jasmine for another project. Mocha supports preprocessing, but I don't know how much work it would be to switch over. The describe / it constructs are the same, but the assertion syntax would different, and I don't know how easily the test helpers will convert.

That's definitely a bunch of work though, so if you want completely overhaul the testing suite, it may be worth considering what you would want to do as if you were doing it from scratch.

mAAdhaTTah commented 7 years ago

Looking back over this convo, you don't need Karma to "support" node; you just run them in with node, and use Karma only when you're trying to run the tests in the browser.

I'm revisiting this issue because I'm currently refactoring my test suite for brookjs, and I'd love to be able to use the matchers / assertions you wrote here. Any interest in extracting them to a separate package? If so, couple other questions:

  1. Do you want to stick w/ Jasmine? I'd get some benefits / harmony from switching to mocha / chai and extracting the matchers to a chai plugin, but I don't want to impose my requirements on you.
  2. Do you want to leave them in coffeescript or can we "decaffeinate" them?
rpominov commented 7 years ago

Oh, I've deleted the browser tests build, so it should be easier for any further improvements. #1 and #2 both sound good for me, feel free to open a PR!

Closing this issue.