From aad52147754106ebc363ff357f7996238ce423b0 Mon Sep 17 00:00:00 2001 From: Tommy Brunn Date: Wed, 3 Jun 2020 15:35:49 +0200 Subject: [PATCH 1/3] Make Array shuffle test pass in Node >= 11.0.0 Due to a change in how array sorting works, the shuffle test would fail in Node >= 11.0.0. Instead of relying on implementation details of how the engine implements sorting, we'll add a requirement on the shuffle implementation to always return a different order than the input array. That way we can simply assert that the returned array is different from the input, rather than asserting on the exact order, which may differ between engine versions. --- src/utils/shuffle.js | 20 +++++++++++++++++++- src/utils/shuffle.spec.js | 24 +++++++++++++----------- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/utils/shuffle.js b/src/utils/shuffle.js index 27526596c..9ea387c99 100644 --- a/src/utils/shuffle.js +++ b/src/utils/shuffle.js @@ -1 +1,19 @@ -module.exports = array => [...array].sort(() => Math.random() - 0.5) +const shuffle = array => array.sort(() => Math.random() - 0.5) + +module.exports = array => { + if (!Array.isArray(array)) { + throw new TypeError("'array' is not an array") + } + + if (array.length < 2) { + return array + } + + let shuffled = [...array] + + while (shuffled.every((value, index) => value === array[index])) { + shuffled = shuffle(shuffled) + } + + return shuffled +} diff --git a/src/utils/shuffle.spec.js b/src/utils/shuffle.spec.js index 076bf0147..8be9f48e5 100644 --- a/src/utils/shuffle.spec.js +++ b/src/utils/shuffle.spec.js @@ -3,17 +3,19 @@ const shuffle = require('./shuffle') describe('Utils > shuffle', () => { it('shuffles', () => { const array = [1, 2, 3] - jest - .spyOn(Math, 'random') - .mockImplementationOnce(() => 0.4) - .mockImplementationOnce(() => 0.7) - .mockImplementationOnce(() => 0.4) - .mockImplementationOnce(() => 0.6) - .mockImplementationOnce(() => 0.6) - .mockImplementationOnce(() => 0.6) + const shuffled = shuffle(array) - expect(shuffle(array)).toEqual([1, 3, 2]) - expect(shuffle(array)).toEqual([3, 2, 1]) - expect(Math.random).toHaveBeenCalledTimes(6) + expect(shuffled).not.toEqual(array) + expect(shuffled).toIncludeSameMembers(array) + }) + + it('returns the same order for single element arrays', () => { + expect(shuffle([1])).toEqual([1]) + }) + + it('throws if it receives a non-array', () => { + expect(() => shuffle()).toThrowError(TypeError) + expect(() => shuffle('foo')).toThrowError(TypeError) + expect(() => shuffle({})).toThrowError(TypeError) }) }) From eac178b4314ab31231f6fabd8287e4916705a706 Mon Sep 17 00:00:00 2001 From: Tommy Brunn Date: Wed, 3 Jun 2020 16:20:29 +0200 Subject: [PATCH 2/3] Bail out early of shuffle comparison --- src/utils/shuffle.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/shuffle.js b/src/utils/shuffle.js index 9ea387c99..bd5c326b3 100644 --- a/src/utils/shuffle.js +++ b/src/utils/shuffle.js @@ -11,7 +11,7 @@ module.exports = array => { let shuffled = [...array] - while (shuffled.every((value, index) => value === array[index])) { + while (!shuffled.some((value, index) => value !== array[index])) { shuffled = shuffle(shuffled) } From b69a2a812ada9ed0d99d5e3de0a21d831e219d24 Mon Sep 17 00:00:00 2001 From: Tommy Brunn Date: Wed, 3 Jun 2020 19:50:53 +0200 Subject: [PATCH 3/3] Improve array shuffle Replace slow naive implementation with Durstenfeld Shuffle, to allow us to run the test against a larger array. --- src/utils/shuffle.js | 13 +++++++------ src/utils/shuffle.spec.js | 4 +++- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/utils/shuffle.js b/src/utils/shuffle.js index bd5c326b3..88e375129 100644 --- a/src/utils/shuffle.js +++ b/src/utils/shuffle.js @@ -1,5 +1,3 @@ -const shuffle = array => array.sort(() => Math.random() - 0.5) - module.exports = array => { if (!Array.isArray(array)) { throw new TypeError("'array' is not an array") @@ -9,11 +7,14 @@ module.exports = array => { return array } - let shuffled = [...array] + const copy = array.slice() - while (!shuffled.some((value, index) => value !== array[index])) { - shuffled = shuffle(shuffled) + for (let i = copy.length - 1; i > 0; i--) { + const j = Math.floor(Math.random() * (i + 1)) + const temp = copy[i] + copy[i] = copy[j] + copy[j] = temp } - return shuffled + return copy } diff --git a/src/utils/shuffle.spec.js b/src/utils/shuffle.spec.js index 8be9f48e5..02d22d6ba 100644 --- a/src/utils/shuffle.spec.js +++ b/src/utils/shuffle.spec.js @@ -2,7 +2,9 @@ const shuffle = require('./shuffle') describe('Utils > shuffle', () => { it('shuffles', () => { - const array = [1, 2, 3] + const array = Array(500) + .fill() + .map((_, i) => i) const shuffled = shuffle(array) expect(shuffled).not.toEqual(array)