convert tests to mocha #849

charlierudolph · 2015-07-17T22:49:55Z

just a proof of concept where you can run mocha tests in node and in the browser.
Works right along side the nodeunit tests, so could be transitioned over one by one.

aearly · 2015-07-17T22:53:23Z

It's beautiful. 😂

megawac · 2015-07-17T23:00:26Z

mocha_test/forever_test.js

@@ -0,0 +1,44 @@
+var async = require('../lib/async');


Not a big fan of the suffixed file names. Why not just run tests on everything in mocha_test/*

Thats what happening at the moment anyway. Would you want this named forever.js?

Yeah, file name = function name?

On Fri, Jul 17, 2015 at 7:03 PM, Charlie Rudolph notifications@github.com
wrote:

In mocha_test/forever_test.js
#849 (comment):

@@ -0,0 +1,44 @@
+var async = require('../lib/async');

Thats what happening at the moment anyway. Would you want this named
forever.js?

—
Reply to this email directly or view it on GitHub
https://github.com/caolan/async/pull/849/files#r34938385.

Thats what happening at the moment anyway

No, karma looks only for _test

megawac · 2015-07-17T23:02:26Z

You have to set this in travis.yml

addons:
  firefox: "39.0"

megawac · 2015-07-17T23:04:57Z

This will make it difficult to support index.html without a conversion script.The way we do this in marionette is we expose the mocha & chai stuff globally in a setup script

charlierudolph · 2015-07-17T23:17:23Z

What is the requirement to support index.html?

aearly · 2015-07-17T23:18:14Z

Can karma run nodeunit?

aearly · 2015-07-17T23:18:44Z

Anyway, I say we merge this, and then start improving on it. 👍

charlierudolph · 2015-07-17T23:20:01Z

Yes it does appear karma can run nodeunit. Unsure if it can mocha and nodeunit at the same time though

charlierudolph · 2015-07-17T23:28:15Z

Added karma-mocha-reporter so we get nice output when karma runs tests.

megawac · 2015-07-17T23:49:25Z

What is the requirement to support index.html?

I don't know I think its nice to be able run with index.html, mainly for people without node/npm installed/windows that can't install some modules

megawac · 2015-07-17T23:52:48Z

Yes it does appear karma can run nodeunit. Unsure if it can mocha and nodeunit at the same time though

Would require 2 karma config files

charlierudolph · 2015-07-18T00:54:57Z

Got karma with nodeunit running

> karma start karma.nodeunit.js 
17 07 2015 17:50:04.020:INFO [framework.browserify]: 166580 bytes written (1.03 seconds)
17 07 2015 17:50:04.026:INFO [framework.browserify]: bundle built
17 07 2015 17:50:04.037:WARN [karma]: Port 9876 in use
17 07 2015 17:50:04.038:INFO [karma]: Karma v0.13.2 server started at http://localhost:9877/
17 07 2015 17:50:04.079:INFO [launcher]: Starting browser Firefox
17 07 2015 17:50:06.474:INFO [Firefox 39.0.0 (Mac OS X 10.10.0)]: Connected on socket LoIOgPA7NY7eHwTYAAAA with id 48556242
Firefox 39.0.0 (Mac OS X 10.10.0)  parallel limit object FAILED
    same
    AssertionError: [1,3,2] deepEqual [1,2,3]
Firefox 39.0.0 (Mac OS X 10.10.0): Executed 231 of 148 (1 FAILED) (23.839 secs / 22.632 secs)
>

Looks like a few bugs. We can add this in a separate PR if we want it

convert tests to mocha

aearly · 2015-07-20T06:32:53Z

Ok, we can start migrating this as we see fit. Also, karma makes node_modules huge...

megawac · 2015-07-22T19:45:11Z

I'm having second thoughts on this. I'm not sure if mocha is the right choice for the testRunner.

I would prefer a test runner that allows users to run tests in parallel. Currently the test suite takes about 10-20 seconds (unmeasured) to execute. It'd be sweet if we could use a test runner that allows us to run these tests at the same time.

Mocha doesn't give us this mochajs/mocha#98; I think tape, jest, or lab. I see caolan explicitly decided against parallel tests in node-unit

charlierudolph · 2015-07-22T20:26:59Z

I don't see much benefit from running the tests in parallel. Each test fakes async operations with setTimeout with low to no delay. There are no long running async operations.

megawac · 2015-07-22T20:33:16Z

Minimal delay (its about 20+ ms depending on the test) adds up across hundreds of tests. This is clearly illustrated as we are currently past a 21 second running time.

A lot of this time is spent in the I/O event timer call stack, which could be minimized through parallelization

The 5 mocha tests you've converted thus far take 200ms!

charlierudolph · 2015-07-22T20:36:44Z

1 of those is 174 ms spent making sure forever can avoid a stack overflow.

megawac · 2015-07-22T20:42:52Z

1 of those is 174 ms spent making sure forever can avoid a stack overflow.

Yes, and the majority of that time is spent waiting for recursive setImmediates to resolve

aearly · 2015-07-22T23:43:58Z

Ha, I tried making parallel mocha work a while back: mochajs/mocha#849

The problem is if you have tests running in parallel and an exception is thrown, the process.on("uncaughtError") handler won't know which test failed. Domains were insufficient. You really need multiple processes to get effective parallelism in mocha. Any test runner will have the same issue.

megawac · 2015-07-23T00:11:29Z

Par chance, have you looked into how jest goes about it @aearly

aearly · 2015-07-23T00:25:13Z

At a quick glance, it looks like they use a multi-process test runner.

megawac · 2016-03-16T16:57:57Z

Upon further review, I think https://github.com/sindresorhus/ava would be the best test runner for this project

aearly · 2016-03-21T20:28:49Z

Ugh, always new tools. I'd be hesitant to switch to it, seeing as we're not even halfway through migrating to mocha. Mocha is also has really good adoption in the greater JS world, so it's one less thing for a new contributor to learn most of the time.

megawac · 2016-03-22T23:16:23Z

Sure it's relatively new, however, that library aims to solve the main
issues I have with node unit. Whereas mocha doesnt have much advantage
outside of plugin support.

The nice thing about Ava is it uses tap api so if we run into issues we
could just use tape or prova. Tap is also easier to find replace for node
unit to tap over node unit to expect
On Mar 21, 2016 4:29 PM, "Alex Early" notifications@github.com wrote:

Ugh, always new tools. I'd be hesitant to switch to it, seeing as we're
not even halfway through migrating to mocha. Mocha is also has really good
adoption in the greater JS world, so it's one less thing for a new
contributor to learn most of the time.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#849 (comment)

aearly · 2016-03-22T23:24:29Z

Another thing, I just noticed Ava doesn't support browsers -- only jsdom.

megawac · 2016-05-05T20:19:32Z

Another thing, I just noticed Ava doesn't support browsers -- only jsdom.

Yes, but it runs on the tape protocol which works in the browser

mocha tests

a23d3a2

megawac reviewed Jul 17, 2015
View reviewed changes

switch to firefox for travis

2ec2068

charlierudolph added 5 commits July 17, 2015 16:06

fix travis

eb02a1d

fix travis

8910bde

update files

d82a8fe

update karma

7dd122f

update karma on travis

d70cebe

charlierudolph added 2 commits July 17, 2015 16:21

document

c7e5e23

add karma mocha reporter

7e3fcfc

aearly added a commit that referenced this pull request Jul 20, 2015

Merge pull request #849 from charlierudolph/cr-splitTests

2dae787

convert tests to mocha

aearly merged commit 2dae787 into caolan:master Jul 20, 2015

charlierudolph deleted the cr-splitTests branch July 20, 2015 15:47

megawac mentioned this pull request May 5, 2016

Сonvert some tests to mocha #1145

Merged

ezubarev mentioned this pull request May 6, 2016

Convert the rest of tests to mocha #1146

Merged

megawac mentioned this pull request May 18, 2016

npm test does not work #1160

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

convert tests to mocha #849

convert tests to mocha #849

charlierudolph commented Jul 17, 2015

aearly commented Jul 17, 2015

megawac Jul 17, 2015

charlierudolph Jul 17, 2015

megawac Jul 17, 2015

megawac Jul 17, 2015

megawac commented Jul 17, 2015

megawac commented Jul 17, 2015

charlierudolph commented Jul 17, 2015

aearly commented Jul 17, 2015

aearly commented Jul 17, 2015

charlierudolph commented Jul 17, 2015

charlierudolph commented Jul 17, 2015

megawac commented Jul 17, 2015

megawac commented Jul 17, 2015

charlierudolph commented Jul 18, 2015

aearly commented Jul 20, 2015

megawac commented Jul 22, 2015

charlierudolph commented Jul 22, 2015

megawac commented Jul 22, 2015

charlierudolph commented Jul 22, 2015

megawac commented Jul 22, 2015

aearly commented Jul 22, 2015

megawac commented Jul 23, 2015

aearly commented Jul 23, 2015

megawac commented Mar 16, 2016

aearly commented Mar 21, 2016

megawac commented Mar 22, 2016

aearly commented Mar 22, 2016

megawac commented May 5, 2016

convert tests to mocha #849

convert tests to mocha #849

Conversation

charlierudolph commented Jul 17, 2015

aearly commented Jul 17, 2015

megawac Jul 17, 2015

Choose a reason for hiding this comment

charlierudolph Jul 17, 2015

Choose a reason for hiding this comment

megawac Jul 17, 2015

Choose a reason for hiding this comment

megawac Jul 17, 2015

Choose a reason for hiding this comment

megawac commented Jul 17, 2015

megawac commented Jul 17, 2015

charlierudolph commented Jul 17, 2015

aearly commented Jul 17, 2015

aearly commented Jul 17, 2015

charlierudolph commented Jul 17, 2015

charlierudolph commented Jul 17, 2015

megawac commented Jul 17, 2015

megawac commented Jul 17, 2015

charlierudolph commented Jul 18, 2015

aearly commented Jul 20, 2015

megawac commented Jul 22, 2015

charlierudolph commented Jul 22, 2015

megawac commented Jul 22, 2015

charlierudolph commented Jul 22, 2015

megawac commented Jul 22, 2015

aearly commented Jul 22, 2015

megawac commented Jul 23, 2015

aearly commented Jul 23, 2015

megawac commented Mar 16, 2016

aearly commented Mar 21, 2016

megawac commented Mar 22, 2016

aearly commented Mar 22, 2016

megawac commented May 5, 2016