-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Support arrays in t.like() assertion #3185
Conversation
05fa702
to
a50b1c8
Compare
Force pushed because my local user was setup incorrectly. |
Thanks @tommy-mitchell! I need to find some time to check the current behavior and your changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the long wait @tommy-mitchell. Work is pretty intense so I don't spend much time with computers outside of it.
This looks good, left a comment but the approach looks fine.
Could you also update the documentation at https://github.com/avajs/ava/blob/0574e5e6d7959b7dc506cb2204f432f73ba39451/docs/03-assertions.md#likeactual-selector-message?
The type definition also needs an overload:
Line 166 in 0574e5e
<Expected extends Record<string, any>>(value: any, selector: Expected, message?: string): value is Expected; |
test-tap/assert.js
Outdated
passes(t, () => assertions.like([1, 2, 3], [1, 2, 3])); | ||
|
||
fails(t, () => assertions.like([1, 2, 3], [3, 2, 1])); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the desired behavior, or should t.like()
only support arrays as properties on an object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now you've opened a can of worms!
If I understand / remember correctly, the second argument recursively selects the entries of the first argument which are then compared against the second. So for objects {a: 1, b: 2}
and {a: 3}
we compare {a: 1}
with {a: 3}
. Swap the arguments and we compare {a: 3, b: undefined}
with {a: 1, b: 2}
.
For arrays then we can only compare the indexes for which the selector has a value: [1, 2]
and [3]
compares [1]
with [3]
. Swap the arguments and we compare [3, undefined]
with [1, 2]
.
I think that's all consistent enough and so it'd be fine to support arrays as top-level arguments. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @tommy-mitchell, bit of an oopsy here. I went to check up on this PR and found a pending comment I never submitted 🤦
test-tap/assert.js
Outdated
passes(t, () => assertions.like([1, 2, 3], [1, 2, 3])); | ||
|
||
fails(t, () => assertions.like([1, 2, 3], [3, 2, 1])); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now you've opened a can of worms!
If I understand / remember correctly, the second argument recursively selects the entries of the first argument which are then compared against the second. So for objects {a: 1, b: 2}
and {a: 3}
we compare {a: 1}
with {a: 3}
. Swap the arguments and we compare {a: 3, b: undefined}
with {a: 1, b: 2}
.
For arrays then we can only compare the indexes for which the selector has a value: [1, 2]
and [3]
compares [1]
with [3]
. Swap the arguments and we compare [3, undefined]
with [1, 2]
.
I think that's all consistent enough and so it'd be fine to support arrays as top-level arguments. What do you think?
t.like
): support deep objects/arrays[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [ava](https://avajs.dev) ([source](https://github.com/avajs/ava)) | [`5.2.0` -> `5.3.1`](https://renovatebot.com/diffs/npm/ava/5.2.0/5.3.1) | [![age](https://developer.mend.io/api/mc/badges/age/npm/ava/5.3.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/ava/5.3.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/ava/5.2.0/5.3.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/ava/5.2.0/5.3.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>avajs/ava (ava)</summary> ### [`v5.3.1`](https://github.com/avajs/ava/releases/tag/v5.3.1) [Compare Source](https://github.com/avajs/ava/compare/v5.3.0...v5.3.1) #### What's Changed - Update `t.like()` to support Symbol keys and ignore non-enumerable properties by [@​gibson042](https://github.com/gibson042) in [https://github.com/avajs/ava/pull/3209](https://github.com/avajs/ava/pull/3209) - Fix circular selector detection in `t.like()` by [@​novemberborn](https://github.com/novemberborn) in [https://github.com/avajs/ava/pull/3212](https://github.com/avajs/ava/pull/3212) **Full Changelog**: avajs/ava@v5.3.0...v5.3.1 ### [`v5.3.0`](https://github.com/avajs/ava/releases/tag/v5.3.0) [Compare Source](https://github.com/avajs/ava/compare/v5.2.0...v5.3.0) #### What's Changed - Support arrays in `t.like()` assertions by [@​tommy-mitchell](https://github.com/tommy-mitchell) in [https://github.com/avajs/ava/pull/3185](https://github.com/avajs/ava/pull/3185) - Recognize typical assertion errors (`expect` and `assert`) and use their formatting by [@​Irvenae](https://github.com/Irvenae) in [https://github.com/avajs/ava/pull/3187](https://github.com/avajs/ava/pull/3187) #### New Contributors - [@​MartynasZilinskas](https://github.com/MartynasZilinskas) made their first contribution in [https://github.com/avajs/ava/pull/3172](https://github.com/avajs/ava/pull/3172) - [@​flovogt](https://github.com/flovogt) made their first contribution in [https://github.com/avajs/ava/pull/3194](https://github.com/avajs/ava/pull/3194) - [@​ondreian](https://github.com/ondreian) made their first contribution in [https://github.com/avajs/ava/pull/3192](https://github.com/avajs/ava/pull/3192) - [@​tommy-mitchell](https://github.com/tommy-mitchell) made their first contribution in [https://github.com/avajs/ava/pull/3185](https://github.com/avajs/ava/pull/3185) - [@​craigahobbs](https://github.com/craigahobbs) made their first contribution in [https://github.com/avajs/ava/pull/3198](https://github.com/avajs/ava/pull/3198) - [@​Irvenae](https://github.com/Irvenae) made their first contribution in [https://github.com/avajs/ava/pull/3187](https://github.com/avajs/ava/pull/3187) **Full Changelog**: avajs/ava@v5.2.0...v5.3.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/aepfli/markdownlint-rule-max-one-sentence-per-line). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMDIuMTAiLCJ1cGRhdGVkSW5WZXIiOiIzNi44LjExIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [ava](https://avajs.dev) ([source](https://github.com/avajs/ava)) | [`^5.0.0` -> `^6.0.0`](https://renovatebot.com/diffs/npm/ava/5.0.1/6.0.0) | [![age](https://developer.mend.io/api/mc/badges/age/npm/ava/6.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/ava/6.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/ava/5.0.1/6.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/ava/5.0.1/6.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>avajs/ava (ava)</summary> ### [`v6.0.0`](https://github.com/avajs/ava/releases/tag/v6.0.0) [Compare Source](https://github.com/avajs/ava/compare/v5.3.1...v6.0.0) #### Breaking Changes - AVA now requires Node.js versions 18.18, 20.8 or 21. Versions 14 and 16 are no longer supported. [#​3251](https://github.com/avajs/ava/issues/3251) [#​3216](https://github.com/avajs/ava/issues/3216) - When tests finish, worker threads or child processes are no longer exited through `proces.exit()`. If your test file does not exit on its own, the test run will time out. [#​3260](https://github.com/avajs/ava/issues/3260) - Changes to watch mode [#​3218](https://github.com/avajs/ava/issues/3218): - Watch mode can no longer be started via the `ava.config.*` or `package.json` configuration. - The `ignoredByWatcher` configuration has moved to the `watchMode` object, under the `ignoreChanges` key. - Watch mode now uses the built-in [`fs.watch()`](https://nodejs.org/api/fs.html#fswatchfilename-options-listener) in recursive mode. This is supported on Linux in Node.js 20 or newer, and MacOS and Windows in Node.js 18 as well. There are [caveats](https://nodejs.org/api/fs.html#caveats) to keep in mind. - Failed assertions now throw, meaning that any subsequent code is not executed. This also impacts the type definitions. [#​3246](https://github.com/avajs/ava/issues/3246) - [Only native errors](https://nodejs.org/api/util.html#utiltypesisnativeerrorvalue) are now considered errors by the `t.throws()` and `t.throwsAsync()` assertions. [`Object.create(Error.prototype)` is **not** a native error](Object.create\(Error.prototype\)). [#​3229](https://github.com/avajs/ava/issues/3229) - Changes to modules loaded through the `require` configuration [#​3184](https://github.com/avajs/ava/issues/3184): - If such modules export a default function, this function is now invoked. - Local files are loaded through `@ava/typescript` if necessary. #### Improvements ##### Rewritten watcher The watcher has been rewritten. It’s now built on [`fs.watch()`](https://nodejs.org/api/fs.html#fswatchfilename-options-listener) in recursive mode. [`@vercel/nft`](https://github.com/vercel/nft) is used to perform static dependency analysis, supporting ESM and CJS imports for JavaScript & TypeScript source files. This is a huge improvement over the previous runtime tracking of CJS imports, which did not support ESM. Integration with [`@ava/typescript`](https://github.com/avajs/typescript) has been improved. The watcher can now detect a change to a TypeScript source file, then wait for the corresponding build output to change before re-running tests. The ignoredByWatcher configuration has moved to the watchMode object, under the ignoreChanges key. See [#​3218](https://github.com/avajs/ava/issues/3218) and [#​3257](https://github.com/avajs/ava/issues/3257). ##### Failed assertions now throw Assertions now throw a `TestFailure` error when they fail. This error is not exported or documented and should not be used or thrown manually. You cannot catch this error in order to recover from a failure, use `t.try()` instead. All assertions except for `t.throws()` and `t.throwsAsync()` now return `true` when they pass. This is useful for some of the assertions in TypeScript where they can be used as a type guard. Committing a failed `t.try()` result now also throws. See [#​3246](https://github.com/avajs/ava/issues/3246). ##### `t.throws()` and `t.throwsAsync()` can now expect any error By default, the thrown error (or rejection reason) must be a native error. You can change the assertion to expect any kind of error by setting `any: true` in the expectation object: ```js t.throws(() => { throw 'error' }, {any: true}) ``` See [#​3245](https://github.com/avajs/ava/issues/3245) by [@​adiSuper94](https://github.com/adiSuper94). ##### The `require` configuration is now more powerful It now loads ES modules. Local files are loaded through `@ava/typescript` if necessary, so you can also write these in TypeScript. If there is a default export function, it is invoked after loading. The function is awaited so it can do asynchronous setup before further modules are loaded. Arguments from the configuration can be passed to the function (as a \[[structured clone](https://developer.mozilla.org/en-US/docs/Web/API/structuredClone)]\(https://developer.mozilla.org/en-US/docs/Web/API/structuredClone)). See [#​3184](https://github.com/avajs/ava/issues/3184) by [@​sculpt0r](https://github.com/sculpt0r). ##### Other changes worth noting - Internal events can now be observed (experimentally). See [#​3247](https://github.com/avajs/ava/issues/3247) by [@​codetheweb](https://github.com/codetheweb). It’s experimental and undocumented. - You can now use `t.timeout.clear()` to restore a previous `t.timeout()`. [#​3221](https://github.com/avajs/ava/issues/3221) - Code coverage is flushed to disk at opportune moments. [#​3220](https://github.com/avajs/ava/issues/3220) #### New Contributors - [@​sculpt0r](https://github.com/sculpt0r) made their first contribution in [https://github.com/avajs/ava/pull/3184](https://github.com/avajs/ava/pull/3184) - [@​ZachHaber](https://github.com/ZachHaber) made their first contribution in [https://github.com/avajs/ava/pull/3233](https://github.com/avajs/ava/pull/3233) - [@​adiSuper94](https://github.com/adiSuper94) made their first contribution in [https://github.com/avajs/ava/pull/3245](https://github.com/avajs/ava/pull/3245) - [@​bricker](https://github.com/bricker) made their first contribution in [https://github.com/avajs/ava/pull/3250](https://github.com/avajs/ava/pull/3250) **Full Changelog**: avajs/ava@v5.3.1...v6.0.0 ### [`v5.3.1`](https://github.com/avajs/ava/releases/tag/v5.3.1) [Compare Source](https://github.com/avajs/ava/compare/v5.3.0...v5.3.1) #### What's Changed - Update `t.like()` to support Symbol keys and ignore non-enumerable properties by [@​gibson042](https://github.com/gibson042) in [https://github.com/avajs/ava/pull/3209](https://github.com/avajs/ava/pull/3209) - Fix circular selector detection in `t.like()` by [@​novemberborn](https://github.com/novemberborn) in [https://github.com/avajs/ava/pull/3212](https://github.com/avajs/ava/pull/3212) **Full Changelog**: avajs/ava@v5.3.0...v5.3.1 ### [`v5.3.0`](https://github.com/avajs/ava/releases/tag/v5.3.0) [Compare Source](https://github.com/avajs/ava/compare/v5.2.0...v5.3.0) #### What's Changed - Support arrays in `t.like()` assertions by [@​tommy-mitchell](https://github.com/tommy-mitchell) in [https://github.com/avajs/ava/pull/3185](https://github.com/avajs/ava/pull/3185) - Recognize typical assertion errors (`expect` and `assert`) and use their formatting by [@​Irvenae](https://github.com/Irvenae) in [https://github.com/avajs/ava/pull/3187](https://github.com/avajs/ava/pull/3187) #### New Contributors - [@​MartynasZilinskas](https://github.com/MartynasZilinskas) made their first contribution in [https://github.com/avajs/ava/pull/3172](https://github.com/avajs/ava/pull/3172) - [@​flovogt](https://github.com/flovogt) made their first contribution in [https://github.com/avajs/ava/pull/3194](https://github.com/avajs/ava/pull/3194) - [@​ondreian](https://github.com/ondreian) made their first contribution in [https://github.com/avajs/ava/pull/3192](https://github.com/avajs/ava/pull/3192) - [@​tommy-mitchell](https://github.com/tommy-mitchell) made their first contribution in [https://github.com/avajs/ava/pull/3185](https://github.com/avajs/ava/pull/3185) - [@​craigahobbs](https://github.com/craigahobbs) made their first contribution in [https://github.com/avajs/ava/pull/3198](https://github.com/avajs/ava/pull/3198) - [@​Irvenae](https://github.com/Irvenae) made their first contribution in [https://github.com/avajs/ava/pull/3187](https://github.com/avajs/ava/pull/3187) **Full Changelog**: avajs/ava@v5.2.0...v5.3.0 ### [`v5.2.0`](https://github.com/avajs/ava/releases/tag/v5.2.0) [Compare Source](https://github.com/avajs/ava/compare/v5.1.1...v5.2.0) #### What's Changed - Infer thrown error from expectations by [@​tao-cumplido](https://github.com/tao-cumplido) in [https://github.com/avajs/ava/pull/3156](https://github.com/avajs/ava/pull/3156) #### New Contributors - [@​tao-cumplido](https://github.com/tao-cumplido) made their first contribution in [https://github.com/avajs/ava/pull/3156](https://github.com/avajs/ava/pull/3156) **Full Changelog**: avajs/ava@v5.1.1...v5.2.0 ### [`v5.1.1`](https://github.com/avajs/ava/releases/tag/v5.1.1) [Compare Source](https://github.com/avajs/ava/compare/v5.1.0...v5.1.1) #### What's Changed - Fix de-registration of shared workers to ensure AVA exits correctly, by [@​codetheweb](https://github.com/codetheweb) in [https://github.com/avajs/ava/pull/3149](https://github.com/avajs/ava/pull/3149) & [https://github.com/avajs/ava/pull/3151](https://github.com/avajs/ava/pull/3151) **Full Changelog**: avajs/ava@v5.1.0...v5.1.1 ### [`v5.1.0`](https://github.com/avajs/ava/releases/tag/v5.1.0) [Compare Source](https://github.com/avajs/ava/compare/v5.0.1...v5.1.0) ##### What's Changed - Output logs for tests that remain pending when AVA exits by [@​kevo1ution](https://github.com/kevo1ution) in [https://github.com/avajs/ava/pull/3125](https://github.com/avajs/ava/pull/3125) - Check for --config file extensions after they fail to load, allowing custom loaders by [@​panva](https://github.com/panva) in [https://github.com/avajs/ava/pull/3135](https://github.com/avajs/ava/pull/3135) ##### New Contributors - [@​kevo1ution](https://github.com/kevo1ution) made their first contribution in [https://github.com/avajs/ava/pull/3125](https://github.com/avajs/ava/pull/3125) - [@​panva](https://github.com/panva) made their first contribution in [https://github.com/avajs/ava/pull/3135](https://github.com/avajs/ava/pull/3135) **Full Changelog**: avajs/ava@v5.0.1...v5.1.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/gr2m/github-project). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy44MS4zIiwidXBkYXRlZEluVmVyIjoiMzcuODEuMyIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Continues from #3023. Adds a test case (the repro from #2627) and handles the
Reflect.ownKeys(array).length
issue.Fixes #2627.
It's been a couple months since someone last said they'd add this, so I went for it.