Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
sukovanej opened this issue Apr 25, 2024 · 9 comments

Comments

@sukovanej
Copy link
Contributor

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 sukovanej added the bug Something isn't working label Apr 25, 2024
@sukovanej
Copy link
Contributor Author

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
Copy link
Contributor Author

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 mikearnaldi removed the bug Something isn't working label Apr 25, 2024
@mikearnaldi
Copy link
Member

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
Copy link
Member

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

@sukovanej
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Member

seems like a vitest issue, jest had a similar jestjs/jest#8280

@mikearnaldi
Copy link
Member

vitest-dev/vitest#5620

@mikearnaldi
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants