-
Notifications
You must be signed in to change notification settings - Fork 572
test(compat/each): replace each duplicate tests with an alias check against forEach #1873
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
Merged
+5
−242
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,246 +1,9 @@ | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
| import { forEach as each } from './forEach'; | ||
| import { MAX_SAFE_INTEGER } from '../_internal/MAX_SAFE_INTEGER'; | ||
| import { slice } from '../_internal/slice'; | ||
| import { stubTrue } from '../util/stubTrue'; | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { each } from './each'; | ||
| import { forEach } from './forEach'; | ||
|
|
||
| describe('each', () => { | ||
| it('should iterate over array elements', () => { | ||
| const array = [1, 2, 3]; | ||
| const result: number[] = []; | ||
|
|
||
| each(array, value => { | ||
| result.push(value); | ||
| }); | ||
|
|
||
| expect(result).toEqual([1, 2, 3]); | ||
| }); | ||
|
|
||
| it('should iterate over string characters', () => { | ||
| const string = 'abc'; | ||
| const result: string[] = []; | ||
|
|
||
| each(string, value => { | ||
| result.push(value); | ||
| }); | ||
|
|
||
| expect(result).toEqual(['a', 'b', 'c']); | ||
| }); | ||
|
|
||
| it('should iterate over object properties', () => { | ||
| const object = { a: 1, b: 2 }; | ||
| const result: Array<[number, string]> = []; | ||
|
|
||
| each(object, (value, key) => { | ||
| result.push([value, key]); | ||
| }); | ||
|
|
||
| expect(result).toEqual([ | ||
| [1, 'a'], | ||
| [2, 'b'], | ||
| ]); | ||
| }); | ||
|
|
||
| it('should return the original array after iteration', () => { | ||
| const array = [1, 2, 3]; | ||
| const result = each(array, value => value * 2); | ||
|
|
||
| expect(result).toBe(array); | ||
| }); | ||
|
|
||
| it('should return the original string after iteration', () => { | ||
| const string = 'abc'; | ||
| const result = each(string, value => value.toUpperCase()); | ||
|
|
||
| expect(result).toBe(string); | ||
| }); | ||
|
|
||
| it('should return the original object after iteration', () => { | ||
| const object = { a: 1, b: 2 }; | ||
| const result = each(object, value => value + 1); | ||
|
|
||
| expect(result).toBe(object); | ||
| }); | ||
|
|
||
| it('should return the input collection if null or undefined is passed', () => { | ||
| const nullValue = null; | ||
| const undefinedValue = undefined; | ||
|
|
||
| const resultForNull = each(nullValue, vi.fn()); | ||
| const resultForUndefined = each(undefinedValue, vi.fn()); | ||
|
|
||
| expect(resultForNull).toBe(null); | ||
| expect(resultForUndefined).toBe(undefined); | ||
| }); | ||
|
|
||
| it('should use identity function as the callback if no callback is provided', () => { | ||
| const array = [1, 2, 3]; | ||
| const result = each(array); | ||
|
|
||
| expect(result).toBe(array); | ||
| }); | ||
|
|
||
| it('should iterate over array-like structures', () => { | ||
| const arrayLike = { 0: 'a', 1: 'b', length: 2 }; | ||
| const result: Array<[number, string]> = []; | ||
|
|
||
| each(arrayLike, (value, index) => { | ||
| result.push([index, value]); | ||
| }); | ||
|
|
||
| expect(result).toEqual([ | ||
| [0, 'a'], | ||
| [1, 'b'], | ||
| ]); | ||
| }); | ||
|
|
||
| const array = [1, 2, 3]; | ||
| const func = each; | ||
| const methodName = 'each'; | ||
|
|
||
| it(`\`_.${methodName}\` should provide correct iteratee arguments`, () => { | ||
| let args: any; | ||
| const expected = [1, 0, array]; | ||
|
|
||
| func(array, function () { | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-expressions, prefer-rest-params | ||
| args || (args = slice.call(arguments)); | ||
| }); | ||
| expect(args).toEqual(expected); | ||
| }); | ||
|
|
||
| it(`\`_.${methodName}\` should treat sparse arrays as dense`, () => { | ||
| const array = [1]; | ||
| array[2] = 3; | ||
|
|
||
| const expected = [ | ||
| [1, 0, array], | ||
| [undefined, 1, array], | ||
| [3, 2, array], | ||
| ]; | ||
|
|
||
| const argsList: any[] = []; | ||
| func(array, function () { | ||
| // eslint-disable-next-line prefer-rest-params | ||
| argsList.push(slice.call(arguments)); | ||
| return true; | ||
| }); | ||
|
|
||
| expect(argsList).toEqual(expected); | ||
| }); | ||
|
|
||
| const isEvery = false; | ||
|
|
||
| it(`\`_.${methodName}\` should not iterate custom properties on arrays`, () => { | ||
| const array: any = [1, 2, 3]; | ||
| array.a = 1; | ||
| const keys: any[] = []; | ||
| func(array, (value, key) => { | ||
| keys.push(key); | ||
| return isEvery; | ||
| }); | ||
|
|
||
| expect(keys.includes('a')).toBeFalsy(); | ||
| }); | ||
|
|
||
| it(`\`_.${methodName}\` iterates over own string keyed properties of objects`, () => { | ||
| function Foo(this: any) { | ||
| // eslint-disable-next-line | ||
| // @ts-ignore | ||
| this.a = 1; | ||
| } | ||
| Foo.prototype.b = 2; | ||
|
|
||
| const values: any[] = []; | ||
| // eslint-disable-next-line | ||
| // @ts-ignore | ||
| func(new Foo(), value => { | ||
| values.push(value); | ||
| }); | ||
| expect(values).toEqual([1]); | ||
| }); | ||
|
|
||
| it(`\`_.${methodName}\` should return the collection`, () => { | ||
| const array = [1, 2, 3]; | ||
| expect(func(array, Boolean)).toBe(array); | ||
| }); | ||
|
|
||
| it(`\`_.${methodName}\` should use \`isArrayLike\` to determine whether a value is array-like`, () => { | ||
| const isIteratedAsObject = function (object: any) { | ||
| let result = false; | ||
| func(object, () => { | ||
| result = true; | ||
| }); | ||
| return result; | ||
| }; | ||
|
|
||
| const values = [-1, '1', 1.1, Object(1), MAX_SAFE_INTEGER + 1]; | ||
| const expected = values.map(stubTrue); | ||
|
|
||
| const actual = values.map(length => isIteratedAsObject({ length: length })); | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| const Foo = function (a: any) {}; | ||
| Foo.a = 1; | ||
|
|
||
| expect(actual).toEqual(expected); | ||
| expect(isIteratedAsObject(Foo)).toBeTruthy(); | ||
| expect(isIteratedAsObject({ length: 0 })).toBeFalsy(); | ||
| }); | ||
|
|
||
| it(`\`_.${methodName}\` should ignore changes to \`length\``, () => { | ||
| if (func) { | ||
| let count = 0; | ||
| const array = [1]; | ||
|
|
||
| func(array, () => { | ||
| if (++count === 1) { | ||
| array.push(2); | ||
| } | ||
| return true; | ||
| }); | ||
|
|
||
| expect(count).toBe(1); | ||
| } | ||
| }); | ||
|
|
||
| it(`\`_.${methodName}\` should ignore added \`object\` properties`, () => { | ||
| let count = 0; | ||
| const object = { a: 1 }; | ||
|
|
||
| func(object, () => { | ||
| if (++count === 1) { | ||
| // eslint-disable-next-line | ||
| // @ts-ignore | ||
| object.b = 2; | ||
| } | ||
| return true; | ||
| }); | ||
|
|
||
| expect(count).toBe(1); | ||
| }); | ||
|
|
||
| it(`\`_.${methodName}\` can exit early when iterating arrays`, () => { | ||
| const array = [1, 2, 3]; | ||
| const values: any[] = []; | ||
|
|
||
| func(array, (value, other) => { | ||
| values.push(Array.isArray(value) ? other : value); | ||
| return false; | ||
| }); | ||
|
|
||
| expect(values).toEqual([1]); | ||
| }); | ||
|
|
||
| it(`\`_.${methodName}\` can exit early when iterating objects`, () => { | ||
| const object = { a: 1, b: 2, c: 3 }; | ||
| const values = []; | ||
|
|
||
| func(object, (value, other) => { | ||
| values.push(Array.isArray(value) ? other : value); | ||
| return false; | ||
| }); | ||
|
|
||
| expect(values.length).toBe(1); | ||
| it('should be an alias of forEach', () => { | ||
| expect(each).toBe(forEach); | ||
| }); | ||
| }); | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I noticed that alias functions didn't have dedicated test files before. How about keeping the test file, as it is now, but only adding a test that verifies the alias references the same implementation as the original function?
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.
As far as I understand, that's actually already what this PR does.
each.spec.tsstays right where it is; we just trimmed it down to a single test confirmingeachis the same implementation asforEach:Only the duplicated behavioral cases were removed, since
forEach.spec.tsalready covers those. Let me know if you meant something a bit different, happy to adjust!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.
I actually meant asking whether we should add the same kind of test for the other aliases too, like
first,extend...It looks like this is already being worked on :) -> #1883