marchaos / jest-mock-extended

Type safe mocking extensions for Jest https://www.npmjs.com/package/jest-mock-extended
MIT License
828 stars 57 forks source link

Date proxy in 2.0.4 breaks comparison #78

Closed werkshy closed 2 years ago

werkshy commented 2 years ago

I have a test like this that is now breaking because obj.createdAt is a Proxy, not a Date after upgrading to 2.0.4. (excuse the indentation, i omitted some irrelevant context)

class MyModel extends Model {
  id!: string
  otherField!: string
  containerId!: string
  createdAt!: Date
  deletedAt?: Date

  findOne = jest.fn()
  findAll = jest.fn()
}

describe('Paginator', () => {
  const paginator = new Paginator()

  beforeEach(() => {
    jest.resetAllMocks()
  })

    it('when DESC returns less-than where clause', async () => {
      const obj = mock<MyModel>({ id: 'mem_123', createdAt: new Date('2021-05-21T16:12') })
      MyModel.findOne = jest.fn().mockReturnValueOnce(obj)

      const whereOptions = await paginator._getPaginationWhereOptions(
        MyModel,
        { otherField: 'banana' },
        { limit: 10, cursorObjectId: 'mem_123', cursorDirection: 'forwards' },
        'createdAt',
        'DESC',
      )

      expect(whereOptions).toStrictEqual({
        createdAt: {
          [Op.lt]: obj.createdAt,
        },
      })
  })
})

The failure reported is

FAIL ../../src/core/services/__tests__/Paginator.test.ts
  ● Paginator › _getPaginationOptions › when DESC returns less-than where clause

    expect(received).toStrictEqual(expected) // deep equality

    Expected: {"createdAt": {Symbol(lt): "2021-05-21T16:12:00.000Z"}}
    Received: serializes to the same string

      66 |       )
      67 |
    > 68 |       expect(whereOptions).toStrictEqual({
         |                            ^
      69 |         createdAt: {
      70 |           [Op.lt]: obj.createdAt,
      71 |         },

      at Object.<anonymous> (../../src/core/services/__tests__/Paginator.test.ts:68:28)

Workaround: I can get this test to pass by declaring obj like

const obj = mock<MyModel>({ id: 'mem_123', createdAt: mock<Date>() })

but I don't love this, and other tests may require the date to be a specific value and be comparable to other dates.

Is this a bug? Is there a better way to write the expect() that is failing?

Sytten commented 2 years ago

It was most likely a side effect pf https://github.com/marchaos/jest-mock-extended/pull/60 Since this was introduced to fix a bug I expect it will stay that way. Strict equality on JS objects that are not primitives is always brittle at best. I would suggest either creating an operator for jest for dates or just comparing the timestamp that underlines the date.

werkshy commented 2 years ago

I appreciate your comment @Sytten and thank you to all the contributors. This is a useful package.

If I was just testing the value of createdAt I could compare the underlying timestamp, but in this case I want to assert on the structure of this query object (Sequelize) that I'm building up. I don't even know how to write an expectation that this part of the object is a proxied Date that matches the expected timestamp.

either creating an operator for jest for dates

I'm not sure what this would look like, can you think of something similar to base that on?

I looked at #60 and I think I may have experienced that bug a few months ago and just lived without the assertion I was trying to make because I couldn't understand the cause at the time. If this new issue isn't a bug, it at least feels like a limitation and it is surprising to me, as a user (the surprise is that I set createdAt as a Date, but it was converted into a Proxy.)

Is there any way to more generally have a Proxy for a Date pass an equality expectation with a Date? If not, maybe I just need to embrace the workaround of setting createdAt: mock<Date>() in this case.

Sytten commented 2 years ago

Maybe you could reduce the strictness level of your assertion like with toMatchObject, it should pass even if it is a proxy. To me it is no surprising behaviour since it is within a mock, otherwise you would need a spy or a partial mock (which are not a thing in this lib right now).

werkshy commented 2 years ago

Maybe you could reduce the strictness level of your assertion like with toMatchObject, it should pass even if it is a proxy.

🙏 @Sytten thank you! toMatchObject() works here, and is probably what I should have been using all along. I am going to close this issue since that is the more idiomatic expectation.