From 486c76e34f22cf1fd66fa2c99e605d52c7077760 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leyla=20J=C3=A4hnig?= <77127505+xDivisionByZerox@users.noreply.github.com> Date: Mon, 21 Mar 2022 14:14:57 +0100 Subject: [PATCH] fix: mersenne rand invalid input argument (#577) --- src/mersenne.ts | 13 ++++++------- test/mersenne.spec.ts | 30 ++++++++---------------------- 2 files changed, 14 insertions(+), 29 deletions(-) diff --git a/src/mersenne.ts b/src/mersenne.ts index 6539d738bc7..521456a1e89 100644 --- a/src/mersenne.ts +++ b/src/mersenne.ts @@ -22,18 +22,17 @@ export class Mersenne { * Generates a random number between `[min, max)`. * * @param max The maximum number. Defaults to `0`. - * @param min The minimum number. Defaults to `32768`. Required if `max` is set. + * @param min The minimum number. Defaults to `32768`. * * @example * faker.mersenne.rand() // 15515 * faker.mersenne.rand(500, 1000) // 578 */ - rand(max?: number, min?: number): number { - // TODO @Shinigami92 2022-01-11: This is buggy, cause if min is not passed but only max, - // then min will be undefined and this result in NaN for the whole function - if (max === undefined) { - min = 0; - max = 32768; + rand(max = 32768, min = 0): number { + if (min > max) { + const temp = min; + min = max; + max = temp; } return Math.floor(this.gen.genrandReal2() * (max - min) + min); diff --git a/test/mersenne.spec.ts b/test/mersenne.spec.ts index b2201240f03..e7ca6198966 100644 --- a/test/mersenne.spec.ts +++ b/test/mersenne.spec.ts @@ -8,7 +8,7 @@ type SeededRun = { type SeededRunExpectations = { rand: { noArgs: number; - minMax: { max?: number; min?: number; expected?: number; skip?: boolean }[]; + minMax: { max?: number; min?: number; expected?: number }[]; }; }; @@ -21,7 +21,7 @@ const seededRuns: SeededRun[] = [ minMax: [ { max: 100, min: 0, expected: 37 }, { max: undefined, min: 0, expected: 12272 }, - { max: 100, min: undefined, expected: 37, skip: true }, + { max: 100, min: undefined, expected: 37 }, ], }, }, @@ -34,7 +34,7 @@ const seededRuns: SeededRun[] = [ minMax: [ { max: 100, min: 0, expected: 26 }, { max: undefined, min: 0, expected: 8586 }, - { max: 100, min: undefined, expected: 26, skip: true }, + { max: 100, min: undefined, expected: 26 }, ], }, }, @@ -47,7 +47,7 @@ const seededRuns: SeededRun[] = [ minMax: [ { max: 100, min: 0, expected: 92 }, { max: undefined, min: 0, expected: 30425 }, - { max: 100, min: undefined, expected: 92, skip: true }, + { max: 100, min: undefined, expected: 92 }, ], }, }, @@ -60,7 +60,7 @@ const seededRuns: SeededRun[] = [ minMax: [ { max: 100, min: 0, expected: 85 }, { max: undefined, min: 0, expected: 28056 }, - { max: 100, min: undefined, expected: 85, skip: true }, + { max: 100, min: undefined, expected: 85 }, ], }, }, @@ -73,7 +73,7 @@ const seededRuns: SeededRun[] = [ minMax: [ { max: 100, min: 0, expected: 17 }, { max: undefined, min: 0, expected: 5895 }, - { max: 100, min: undefined, expected: 17, skip: true }, + { max: 100, min: undefined, expected: 17 }, ], }, }, @@ -86,7 +86,7 @@ const seededRuns: SeededRun[] = [ minMax: [ { max: 100, min: 0, expected: 89 }, { max: undefined, min: 0, expected: 29217 }, - { max: 100, min: undefined, expected: 89, skip: true }, + { max: 100, min: undefined, expected: 89 }, ], }, }, @@ -122,9 +122,7 @@ describe('mersenne twister', () => { }); } - for (const { min, max, expected } of expectations.rand.minMax.filter( - (x) => !x.skip - )) { + for (const { min, max, expected } of expectations.rand.minMax) { it(`should return ${expected} for rand(${max}, ${min})`, () => { const actual = mersenne.rand(max, min); @@ -132,18 +130,6 @@ describe('mersenne twister', () => { }); } - // TODO @piotrekn 2022-02-13: There is a bug: https://github.com/faker-js/faker/issues/479 - // remove minMax.skip when fixed - for (const { min, max, expected } of expectations.rand.minMax.filter( - (x) => x.skip - )) { - it.todo(`should return ${expected} for rand(${max}, ${min})`, () => { - const actual = mersenne.rand(max, min); - - expect(actual).toEqual(expected); - }); - } - it.todo(`should return 0 for rand(1)`, () => { const actual = mersenne.rand(1);