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

Mocking setImmediate by default breaks Co #527

Closed
moll opened this issue Jul 30, 2014 · 5 comments
Closed

Mocking setImmediate by default breaks Co #527

moll opened this issue Jul 30, 2014 · 5 comments

Comments

@moll
Copy link

moll commented Jul 30, 2014

This is the corollary to tj/co#139:

I'm not sure who's problem this is, but Co v3.1.0 breaks when Sinon's fake timers are enabled, whereas v3.0.6 doesn't.

I found this when using co-mocha with Sinon and the following before blocks:

beforeEach(function() { this.time = Sinon.useFakeTimers(+NOON) })
afterEach(function() { this.time.restore() })

Now that generators with Co are used so liberally, this might require a better default.

@mroderick
Copy link
Member

Just like @jonathanong asked for in the tj/co#139 issue, it would be greatly beneficial if you could provide a runnable test case that shows the issue.

@moll
Copy link
Author

moll commented Jul 31, 2014

Kay, fair enough. I thought the report above was explanatory and obvious, but to speed things up:

package.json:

{
        "dependencies": {
                "mocha": ">= 1.18.2 < 2",
                "co": ">= 3.1.0 < 4",
                "sinon": ">= 1.9.1 < 2"
        }
}

index.js:

var Sinon = require("sinon")
var co = require("co")
var NOON = new Date(2000, 0, 1, 12)

describe("Test with Sinon and Co", function() {
        beforeEach(function() { this.time = Sinon.useFakeTimers(+NOON) })
        afterEach(function() { this.time.restore() })
        it("must not timeout", co(function*() {}))
})

Run it with:

./node_modules/.bin/mocha --harmony-generators index.js

Observe:

  1) Test with Sinon and Co must not timeout:
     Error: timeout of 2000ms exceeded

@cjohansen
Copy link
Contributor

sinon.useFakeTimers(now, "setTimeout", "clearTimeout");

You can name the functions you want to fake - the default is all of 'em.

@moll
Copy link
Author

moll commented Aug 4, 2014

Thanks. I was actually aware of this, that's why I added the last the bit at the end of "Now that generators with Co are used so liberally, this might require a better default.".

As @yanickrochon pointed out, and I agree, that setImmediate is more an event queue function than a timing function. You wouldn't mock process.nextTick by default would you? setImmediate in Node-land could be considered a synonym for that.

@mantoni
Copy link
Member

mantoni commented Aug 5, 2014

Actually, I would prefer mocking process.nextTick by default. In an ideal world, sinon could guarantee that there is no asynchronous execution based on timings. To me setImmediate is a timing function, but you express that you want the delay to be as small as possible.

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

No branches or pull requests

4 participants