fix(expect): fix custom equality test case #4286
Conversation
|
@eryue0220 |
Ok, I'll try for this. |
…0/deno_std into fix/expect-custom-equality-case * 'fix/expect-custom-equality-case' of github.com:eryue0220/deno_std: (63 commits) docs: link to `assertThrows()` and `assertRejects()` (denoland#4395) chore(log): sync `level` and `levelName` in BaseHandler (denoland#4393) docs: ignore bad snippet examples (denoland#4388) chore(media_types): format test names (denoland#4380) docs: clarify underscore guidance in README (denoland#4385) feat(collections): add `pick` and `omit` (denoland#4218) chore(msgpack): format test names (denoland#4381) refactor(encoding): prepare for `noUncheckedIndexedAccess` (denoland#4275) refactor(streams): prepare for `noUncheckedIndexedAccess` (denoland#4377) chore: fix .editorconfig syntax (denoland#4376) chore(semver): remove legacy `Range.ranges` object definition (denoland#4374) chore(semver): move breaking versions (denoland#4372) refactor(semver): rename `comparatorFormat()` to `formatComparator()` (denoland#4373) test(semver): add test for parse_range (denoland#4345) chore: use 'release' event for triggering jsr publish (denoland#4370) chore(http): fix spawned tests after migration script (denoland#4368) chore(crypto): move test scripts to own files (denoland#4367) 0.217.0 (denoland#4369) build: update _ to - in workspace converter script (denoland#4357) chore(media_types): move `extensions` utility (denoland#4358) ...
|
Sorry for the late response, I've check the api and its usage again, and I restructure the codebase. But the codebase is not just for the specific case, it's also a public api in jest, which is exported from jest/expect-utils. And other expect apis, such as |
| const IS_KEYED_SENTINEL = "@@__IMMUTABLE_KEYED__@@"; | ||
| const IS_SET_SENTINEL = "@@__IMMUTABLE_SET__@@"; | ||
| const IS_LIST_SENTINEL = "@@__IMMUTABLE_LIST__@@"; | ||
| const IS_ORDERED_SENTINEL = "@@__IMMUTABLE_ORDERED__@@"; | ||
| const IS_RECORD_SYMBOL = "@@__IMMUTABLE_RECORD__@@"; |
There was a problem hiding this comment.
Checks for these sentinel keys/values don't make much sense to me because nothing seems setting these keys.
I guess we can just remove these constants and checks related to these
There was a problem hiding this comment.
For internal codebase, it is yes. But as for the apps which uses immutable.js, they will rely on these keys/value to test it, such as the following:
import { OrderedMap } from "npm:[email protected]";
import { expect, iterableEquality } from "https://deno.land/std@$STD_VERSION/expect/mod.ts";
Deno.test("immutable test", () => {
const a = OrderedMap().set("saving", true);
const b = OrderedMap().merge({saving: true});
expect(iterableEquality(a, b)).toBe(true);
});There was a problem hiding this comment.
Does jest support such cases natively?
There was a problem hiding this comment.
Yes, the iterableEquality is exported from expect-utils, and the test codebase is here
expect/mod.ts
Outdated
| @@ -85,3 +85,4 @@ | |||
| */ | |||
| export { expect } from "./expect.ts"; | |||
| export { fn } from "./fn.ts"; | |||
| export { iterableEquality } from "./iterable_equality.ts"; | |||
There was a problem hiding this comment.
I don't think it's good idea to expose iterableEquality from here.
iterableEquality is exported from @jest/expect-utils, but the README of @jest/expect-utils seems discouraging importing things directly from it. ref. https://www.npmjs.com/package/@jest/expect-utils
I'll make iterableEquality private API in std/expect
There was a problem hiding this comment.
I agree in some degree, because it doesn't indeed belong to expect module. But if it's private, how to use in public?
There was a problem hiding this comment.
I suggest we shouldn't provide direct import of iterableEquality. The purpose of std/expect is compatiblity to jest's expect, but jest doesn't document iterableEquality in their docs. Supporting iterableEquality is out of scope here in my view.
|
I'm landing this PR at this point. Let's discuss in another issue if you have further questions about the scope of |
This pr is to support iterable object test check and fix expect custom equality test case.
Background: #4255 (comment)