effect icon indicating copy to clipboard operation
effect copied to clipboard

Wrong comparison of Option with vitest's expect(...).not.toStrictEqual(...)

Open sukovanej opened this issue 10 months ago • 9 comments

What version of Effect is running?

3.0.5

What steps can reproduce the bug?

https://stackblitz.com/edit/stackblitz-starters-8c4cgb?file=index.test.ts

What is the expected behavior?

No response

What do you see instead?

No response

Additional information

3.0.4 and 3.0.5 seems to be affected, works correctly in 3.0.3

sukovanej avatar Apr 25 '24 14:04 sukovanej

Started playing with it in the effect codebase and seems like the expect(<option>).toStrictEqual(<another option>) always succeeds independently of what are the option values, for example this succeeds in the test/Option.test.ts

    expect(_.some(1)).toStrictEqual(_.some(2))
    expect(_.none()).toStrictEqual(_.some(2))

sukovanej avatar Apr 25 '24 14:04 sukovanej

Okay, seems to be the Symbol.iterator that makes the difference - which makes sense, that is the thing introduced in 3.0.4 to enable to yield without an adapter, right?

  [Symbol.iterator]() {
    return new SingleShotGen.SingleShotGen(this) as any
  },

When I remove it from the Effect prototype the comparison starts working. cc @mikearnaldi

sukovanej avatar Apr 25 '24 14:04 sukovanej

Yup that comes from 3.0.4, I don't see how this could ever be considered a bug of effect, it's behaviour of expect...

mikearnaldi avatar Apr 25 '24 14:04 mikearnaldi

The Effect way of comparing options is Equals.equals(a, b)

mikearnaldi avatar Apr 25 '24 14:04 mikearnaldi

In this codebase you rely on the expect().toStrictEqual() / Util.deepStrictEqual etc checks in the tests as well. Therefore, from 3.0.4, tests in here doing checks on options actually test nothing because they all always succeed. And this will apply for all the consumers of effect lib as well.

sukovanej avatar Apr 25 '24 14:04 sukovanej

Yeah but that's a bug in expect not in Effect... like I am trying to compare two objects with a[Symbol.iterator] = [1, 2, 3, Math.random()][Symbol.iterator] and it says that they are equal, why on earth???

mikearnaldi avatar Apr 25 '24 14:04 mikearnaldi

seems like a vitest issue, jest had a similar https://github.com/jestjs/jest/issues/8280

mikearnaldi avatar Apr 25 '24 14:04 mikearnaldi

https://github.com/vitest-dev/vitest/issues/5620

mikearnaldi avatar Apr 25 '24 14:04 mikearnaldi

The following works:

import { jestExpect as expect } from "@jest/expect"
import { describe, it } from "vitest"

describe("Vitest Issue", () => {
  it("compares", () => {
    const a = {
      value: 0,
      [Symbol.iterator]: [0, 1, 2, Math.random()][Symbol.iterator]
    }
    const b = {
      value: 1,
      [Symbol.iterator]: [0, 1, 2, Math.random()][Symbol.iterator]
    }
    expect(a).toStrictEqual(b)
  })
})

mikearnaldi avatar Apr 25 '24 15:04 mikearnaldi