Effect-TS / effect

An ecosystem of tools to build robust applications in TypeScript
https://effect.website
MIT License
6.46k stars 205 forks source link

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

Open sukovanej opened 3 months ago

sukovanej commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

mikearnaldi commented 3 months ago

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 commented 3 months ago

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

sukovanej commented 3 months ago

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.

mikearnaldi commented 3 months ago

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 commented 3 months ago

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

mikearnaldi commented 3 months ago

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

mikearnaldi commented 3 months ago

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)
  })
})