Automattic / mongoose

MongoDB object modeling designed to work in an asynchronous environment.
https://mongoosejs.com
MIT License
26.96k stars 3.84k forks source link

Docs: Jest recommendation incorrect as of today #10296

Closed thernstig closed 3 years ago

thernstig commented 3 years ago

Do you want to request a feature or report a bug?

Docs bug

What is the current behavior?

https://mongoosejs.com/docs/jest.html states:

Jest is a client-side JavaScript testing library developed by Facebook. Because Jest is designed primarily for testing React applications, using it to test Node.js server-side applications comes with a lot of caveats. We strongly advise against using Jest for testing any Node.js apps unless you are an expert developer with an intimate knowledge of Jest.

This is not true as of today in Jest version 27. In fact, Jest uses a node testEnvironment and it is the default in Jest 27. Even before that, Node.js tests could set the test environment to use node which has been recommended for a long time.

Mongoose uses nextTick(), which Jest's underlying dependency explicitly doesn't stub by default.

It does now, see https://jestjs.io/docs/next/jest-object#mock-timers. Jest has a new timer implementation since 2020-05 https://jestjs.io/blog/2020/05/05/jest-26 that is better.

Overall, I think that document should be re-written to only apply to Jest 27, or remove it entirely and only state it is for Jest 26 and earlier, if one is using Jest's defaults.

thernstig commented 3 years ago

However, there is one problem. If running a test that is using a SchemaType of Date, the test fails as follows:

Test suite failed to run

    TypeError: Invalid schema configuration: `ClockDate` is not a valid type at path `time`. See http://bit.ly/mongoose-schematypes for a list of valid schema types.

       5 | import { metricsTimelineFindingStrategy } from './commonPiplines/queryMetricsStrategy.js';
       6 |
    >  7 | const alarmDetailSchema = new mongoose.Schema(
         |                           ^
       8 |   {
       9 |     eventType: { type: String, get: convertEventTypeCodeToName },
      10 |     faultId: String,

      at Schema.Object.<anonymous>.Schema.interpretAsType (node_modules/mongoose/lib/schema.js:1005:11)
      at Schema.Object.<anonymous>.Schema.path (node_modules/mongoose/lib/schema.js:676:27)
      at Schema.add (node_modules/mongoose/lib/schema.js:498:12)
      at new Schema (node_modules/mongoose/lib/schema.js:129:10)
      at Object.<anonymous> (mongooseModels/alarms.js:7:27)
      at Object.<anonymous> (mongooseModels/index.js:7:1)
      at Object.<anonymous> (apiv/foo/mongodbfoo.js:1:1)

There could be a recommendation there on how to solve this. Which is my current problem as I am unsure how to do that. One attempt was:

import { SchemaTypes } from "mongoose"
SchemaTypes["ClockDate"] = SchemaTypes.Date

But that did not work, so suggestions are welcome. Those suggestions would be good in the docs.

vkarpov15 commented 3 years ago

We'll look into it, but, in the meantime, we still strongly recommend switching to a different testing framework. Jest is an anti-pattern.

thernstig commented 3 years ago

It is a pretty strong statement for one of biggest testing frameworks for JavaScript (if not the biggest?). That is something we probably will not come to agreement on haha 😆 So probably no idea to delve into that.

The docs issue aside: Is there a solution to my problem that you might know about?

vkarpov15 commented 3 years ago

There's a couple of potential workarounds. First would be to define your dates using 'Date' as a string, rather than Date as a function. Jest's "modern" fake timer implementation overwrites the global Date class with a class whose name is ClockDate rather than Date, so the below would still work:

const mongoose = require('mongoose');

jest.useFakeTimers();

test('mongoose', () => {
  new mongoose.Schema({ date: 'Date' });
});

Another alternative would be to use legacy timers:

jest.useFakeTimers('legacy');

I'll take a look and see if there's a way to turn off nextTick() mocking with Jest 26+, because Mongoose isn't expected to work in an environment where nextTick() is stubbed out.

thernstig commented 3 years ago

Thanks for the suggestions. The "modern" version of Jest's timers is really https://sinonjs.org/releases/v11.1.1/fake-timers/. So would asking a question there maybe be better to solve this?

I find non of the solutions appealing unfortunately 😞 The first one requires me to do a source code change, which I prefer not to need to do. The legacy timers are much worse in other cases where we easily want to step the time.

The nextTick() question should also maybe be asked to SinonJS? Maybe they have workarounds or recommendations to solve this.

Also I am going to try if this works:

import mongoose from "mongoose"
mongoose.SchemaTypes["ClockDate"] = mongoose.SchemaTypes.Date

The tests are transpiling the code from ESM to CJS, and I think the transpiled code might behave differently with some luck doing like that instead. Not at my work computer now but will try later.

thernstig commented 3 years ago

I have found the solution to the problem in Jest. The reason why my previous attempt did not work was obvious when I thought about it. Setting up a custom type on mongoose should be done before the test file executes in setupFilesAfterEnv

Here is the solution for future users.

// jest.config.js
{
  rootDir: 'server/',
  timers: 'modern', // this makes all suites use the 'modern' (Sinon fake timers) implementation
  setupFiles: ['<rootDir>/testSetup/mongooseClockdate.js'],
}
// mongooseClockdate.js
import mongoose from 'mongoose';

mongoose.SchemaTypes.ClockDate = mongoose.SchemaTypes.Date;

EDIT:

Or even better possibly, just mock mongoose like:

// __mocks__/mongoose.js (__mocks__ should be placed next to the node_modules/ directory)
import mongoose from 'mongoose';

mongoose.SchemaTypes.ClockDate = mongoose.SchemaTypes.Date;

export default mongoose;
vkarpov15 commented 3 years ago

We did some refactoring to store nextTick() and setTimeout() internally so useFakeTimers() can't stub them out. As for the ClockDate issue, your solution works. The other alternative is to just use 'Date' as a string, which works well too.

But I think you're starting to see the issue with Jest: Jest isn't a framework, it's a distinct JavaScript runtime. Testing in Jest but running your apps in Node makes about as much sense as testing in Electron but running in Node.

thernstig commented 3 years ago

@vkarpov15 the docs state Jest 26 but Jest 27 is out now.

Jest is running in node with my test environment, albeit in vm modules to isolate the global scope.