Do you want to request a feature or report a bug?
Bug
What is the current behavior?
I have a few tests which uses fakeTimers and advances time programatically. After updating mongoose, find/(other mongoose functions that are returned from .model
) doesn't resolve when fake timers are set. This used to work in older versions.
const mongoose = require("mongoose")
const sinon = require("sinon")
const connectToMongo = () => {
return mongoose.connect("mongodb://localhost:27017/test-db", {
poolSize: 10,
useNewUrlParser: true,
useFindAndModify: false,
useCreateIndex: true,
connectTimeoutMS: 30000,
useUnifiedTopology: true
})
}
const schema = mongoose.Schema({
foo: String
})
const model = mongoose.model("Foo", schema)
connectToMongo().then(() => {
console.log("CONNECTED")
})
sinon.useFakeTimers()
model.findOne({foo: "1"}).then(val => console.log(val))
// It logs only CONNECTED. Doesn't return null for model.findOne
What is the expected behavior?
find function should not depend on timers.
What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.
NodeJS: 12.16.1
Mongoose: 5.10.5
mongodb: 3.6.3
I can confirm this. Downgrading to 5.10.4 fixes the issue for me.
We'll take a look, we strongly advise against using useFakeTimers()
when testing Mongoose apps, or any apps in general. Stubbing language built-ins is dangerous because you may end up breaking underlying libraries that rely on these built-ins.
Thank you. I agree that jest mock timers is very bad. It causes a lot of troubles. But in my opinion, useFakeTimers
is still valid and necessary for some tests.
The issue here is that everything work in 5.10.4 and earlier versions but stop working in 5.10.5. If this is intentional, then at least we should have bumped version to 5.11 and with the documentation mentioning the breaking change.
useFakeTimers()
is a dubious practice for similar reasons as jest mock timers: setImmediate()
is the only way to defer work to the next event loop macrotask. While Mongoose explicitly avoids setImmediate()
to prevent this issue, underlying libraries may use setImmediate()
and may not work if you stub that out.
We strongly recommend that, instead of relying on useFakeTimers()
to break your JavaScript environment, you create a wrapper around getting the current time that you can require in, like const time = require('util/time'); time.now();
, and then use sinon.stub()
to stub the current time.
As a temporary workaround, you can use the toFake
option for sinon timers: sinon.useFakeTimers({ toFake: ['setTimeout', 'setInterval'] })
to tell sinon to not stub setImmediate()
.
That being said, I mentioned this to the MongoDB driver team, https://github.com/mongodb/node-mongodb-native/pull/2537 uses setImmediate()
to defer some work to the next tick of the event loop. We'll see if they can use process.nextTick()
instead.
Most helpful comment
useFakeTimers()
is a dubious practice for similar reasons as jest mock timers:setImmediate()
is the only way to defer work to the next event loop macrotask. While Mongoose explicitly avoidssetImmediate()
to prevent this issue, underlying libraries may usesetImmediate()
and may not work if you stub that out.We strongly recommend that, instead of relying on
useFakeTimers()
to break your JavaScript environment, you create a wrapper around getting the current time that you can require in, likeconst time = require('util/time'); time.now();
, and then usesinon.stub()
to stub the current time.As a temporary workaround, you can use the
toFake
option for sinon timers:sinon.useFakeTimers({ toFake: ['setTimeout', 'setInterval'] })
to tell sinon to not stubsetImmediate()
.That being said, I mentioned this to the MongoDB driver team, https://github.com/mongodb/node-mongodb-native/pull/2537 uses
setImmediate()
to defer some work to the next tick of the event loop. We'll see if they can useprocess.nextTick()
instead.