fix: it should returns inclusive bounds

faker-js · Feb 22, 2025 · a900efb · a900efb
1 parent fd245e9
commit a900efb
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 26 deletions.
diff --git a/src/modules/number/index.ts b/src/modules/number/index.ts
@@ -471,10 +471,9 @@ export class NumberModule extends SimpleModuleBase {
           length: delta.toString(10).length,
           allowLeadingZeros: true,
         })
-      ) % delta;
-    const random = effectiveMin * multipleOf + offset;
-    const remaining = multipleOf - (random % multipleOf);
-    return random + remaining;
+      ) %
+      (delta + 1n);
+    return (effectiveMin + offset) * multipleOf;
   }
 
   /**

diff --git a/test/modules/__snapshots__/number.spec.ts.snap b/test/modules/__snapshots__/number.spec.ts.snap
@@ -1,20 +1,20 @@
 // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
-exports[`number > 42 > bigInt > noArgs 1`] = `397511086709822n`;
+exports[`number > 42 > bigInt > noArgs 1`] = `397511086709821n`;
 
-exports[`number > 42 > bigInt > with big options 1`] = `19556777749482489605814696n`;
+exports[`number > 42 > bigInt > with big options 1`] = `19556777749482489605814694n`;
 
-exports[`number > 42 > bigInt > with bigint value 1`] = `29n`;
+exports[`number > 42 > bigInt > with bigint value 1`] = `25n`;
 
 exports[`number > 42 > bigInt > with boolean value 1`] = `1n`;
 
-exports[`number > 42 > bigInt > with multipleOf 1`] = `18675377700n`;
+exports[`number > 42 > bigInt > with multipleOf 1`] = `147890295638632n`;
 
-exports[`number > 42 > bigInt > with number value 1`] = `40n`;
+exports[`number > 42 > bigInt > with number value 1`] = `39n`;
 
-exports[`number > 42 > bigInt > with options 1`] = `23n`;
+exports[`number > 42 > bigInt > with options 1`] = `19n`;
 
-exports[`number > 42 > bigInt > with string value 1`] = `40n`;
+exports[`number > 42 > bigInt > with string value 1`] = `39n`;
 
 exports[`number > 42 > binary > noArgs 1`] = `"0"`;
 
@@ -66,21 +66,21 @@ exports[`number > 42 > romanNumeral > with only max 1`] = `"LXII"`;
 
 exports[`number > 42 > romanNumeral > with only min 1`] = `"MDI"`;
 
-exports[`number > 1211 > bigInt > noArgs 1`] = `982966736876849n`;
+exports[`number > 1211 > bigInt > noArgs 1`] = `982966736876848n`;
 
-exports[`number > 1211 > bigInt > with big options 1`] = `25442250580110979794946302n`;
+exports[`number > 1211 > bigInt > with big options 1`] = `25442250580110979794946298n`;
 
-exports[`number > 1211 > bigInt > with bigint value 1`] = `122n`;
+exports[`number > 1211 > bigInt > with bigint value 1`] = `114n`;
 
 exports[`number > 1211 > bigInt > with boolean value 1`] = `1n`;
 
-exports[`number > 1211 > bigInt > with multipleOf 1`] = `99016744867n`;
+exports[`number > 1211 > bigInt > with multipleOf 1`] = `784113589297853n`;
 
-exports[`number > 1211 > bigInt > with number value 1`] = `15n`;
+exports[`number > 1211 > bigInt > with number value 1`] = `12n`;
 
-exports[`number > 1211 > bigInt > with options 1`] = `53n`;
+exports[`number > 1211 > bigInt > with options 1`] = `44n`;
 
-exports[`number > 1211 > bigInt > with string value 1`] = `30n`;
+exports[`number > 1211 > bigInt > with string value 1`] = `28n`;
 
 exports[`number > 1211 > binary > noArgs 1`] = `"1"`;
 
@@ -132,21 +132,21 @@ exports[`number > 1211 > romanNumeral > with only max 1`] = `"CLIV"`;
 
 exports[`number > 1211 > romanNumeral > with only min 1`] = `"MMMDCCXIV"`;
 
-exports[`number > 1337 > bigInt > noArgs 1`] = `212435297136195n`;
+exports[`number > 1337 > bigInt > noArgs 1`] = `212435297136194n`;
 
-exports[`number > 1337 > bigInt > with big options 1`] = `27379244885156992800029993n`;
+exports[`number > 1337 > bigInt > with big options 1`] = `27379244885156992800029992n`;
 
-exports[`number > 1337 > bigInt > with bigint value 1`] = `90n`;
+exports[`number > 1337 > bigInt > with bigint value 1`] = `88n`;
 
-exports[`number > 1337 > bigInt > with boolean value 1`] = `1n`;
+exports[`number > 1337 > bigInt > with boolean value 1`] = `0n`;
 
-exports[`number > 1337 > bigInt > with multipleOf 1`] = `86156732331n`;
+exports[`number > 1337 > bigInt > with multipleOf 1`] = `682275118016671n`;
 
-exports[`number > 1337 > bigInt > with number value 1`] = `22n`;
+exports[`number > 1337 > bigInt > with number value 1`] = `21n`;
 
-exports[`number > 1337 > bigInt > with options 1`] = `60n`;
+exports[`number > 1337 > bigInt > with options 1`] = `58n`;
 
-exports[`number > 1337 > bigInt > with string value 1`] = `22n`;
+exports[`number > 1337 > bigInt > with string value 1`] = `21n`;
 
 exports[`number > 1337 > binary > noArgs 1`] = `"0"`;
 

diff --git a/test/modules/number.spec.ts b/test/modules/number.spec.ts
@@ -686,6 +686,68 @@ describe('number', () => {
         expect(generateBigInt).toBe(-9n);
       });
 
+      it('should return inclusive positive min/max value', () => {
+        const positive4 = 4n;
+        const positive5 = 5n;
+        let foundPositive4 = false;
+        let foundPositive5 = false;
+
+        for (let iter = 0; iter < 1000; iter++) {
+          const actual = faker.number.bigInt({
+            min: positive4,
+            max: positive5,
+          });
+
+          if (actual === positive4) {
+            foundPositive4 = true;
+          } else if (actual === positive5) {
+            foundPositive5 = true;
+          }
+
+          expect(actual).toBeTypeOf('bigint');
+          expect(actual).toBeGreaterThanOrEqual(positive4);
+          expect(actual).toBeLessThanOrEqual(positive5);
+
+          if (foundPositive4 && foundPositive5) {
+            break;
+          }
+        }
+
+        expect(foundPositive4).toBeTruthy();
+        expect(foundPositive5).toBeTruthy();
+      });
+
+      it('should return inclusive negative min/max value', () => {
+        const negative4 = -4n;
+        const negative5 = -5n;
+        let foundNegative4 = false;
+        let foundNegative5 = false;
+
+        for (let iter = 0; iter < 1000; iter++) {
+          const actual = faker.number.bigInt({
+            min: negative5,
+            max: negative4,
+          });
+
+          if (actual === negative4) {
+            foundNegative4 = true;
+          } else if (actual === negative5) {
+            foundNegative5 = true;
+          }
+
+          expect(actual).toBeTypeOf('bigint');
+          expect(actual).toBeGreaterThanOrEqual(negative5);
+          expect(actual).toBeLessThanOrEqual(negative4);
+
+          if (foundNegative4 && foundNegative5) {
+            break;
+          }
+        }
+
+        expect(foundNegative4).toBeTruthy();
+        expect(foundNegative5).toBeTruthy();
+      });
+
       it('should throw for non-positive multipleOf', () => {
         expect(() => faker.number.bigInt({ multipleOf: 0n })).toThrow(
           new FakerError('multipleOf should be greater than 0.')