findOne does not resolve if test code uses sinonFakeTimers

[SgSridhar]

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

[nguymin4]

I can confirm this. Downgrading to 5.10.4 fixes the issue for me.

[vkarpov15]

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.

[nguymin4]

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.

[vkarpov15]

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, mongodb/node-mongodb-native#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.

[sibelius]

how can we tell jest to only mock setTimeout ?

[vkarpov15]

@sibelius it doesn't look like you can, jest.useFakeTimers() doesn't take any parameters: https://deltice.github.io/jest/docs/en/jest-object.html#jestusefaketimers.

Jest uses sinon under the hood anyway, so instead of using jest.useFakeTimers() you can just require('sinon') and bypass Jest. Here's some more info on using sinon timer mocks with Jest.