Repository: dubzzz/fast-check
Pull request #1608 (2021-03-12 08:24:24)

✅ Better errors for isValidValue in helpers
<!-- Why is this PR for? -->
<!-- Describe the reason why you opened this PR or give a link towards the associated issue. -->

## In a nutshell

❌ New feature
❌ Fix an issue
❌ Documentation improvement
✔️ Other: *tests*

(✔️: yes, ❌: no)

## Potential impacts

None


codesandbox[bot]: This pull request is automatically built and testable in [CodeSandbox](https://codesandbox.io).

  To see build info of the built libraries, click [here](https://ci.codesandbox.io/status/dubzzz/fast-check/pr/1608/builds/109051) or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 89646e39adbf5ae84f26fb0a5af458e2ae689fe2:

|Sandbox| Source |
|--|--|
|[Vanilla](https://codesandbox.io/s/vanilla-n0i4m)| Configuration |
|[fast-check-examples](https://codesandbox.io/s/fast-check-examples-6dcv5)| Configuration |


Patch:
diff --git a/documentation/Arbitraries.md b/documentation/Arbitraries.md
index 02718e13..d8de6fc6 100644
--- a/documentation/Arbitraries.md
+++ b/documentation/Arbitraries.md
@@ -2153,7 +2153,7 @@ fc.oneof(fc.char(), fc.boolean(), fc.nat())
 *&#8195;Signatures*
 
 - `fc.frequency(...{ arbitrary, weight })`
-- `fc.frequency({withCrossShrink?, maxDepth?}, ...{ arbitrary, weight })`
+- `fc.frequency({withCrossShrink?, maxDepth?, depthIdentifier?}, ...{ arbitrary, weight })`
 
 *&#8195;with:*
 
@@ -2161,6 +2161,7 @@ fc.oneof(fc.char(), fc.boolean(), fc.nat())
 - `withCrossShrink?` — default: `false` — _in case of failure the shrinker will try to check if a failure can be found by using the first specified arbitrary (if and only if its weight is strictly greater than 0). It may be pretty useful for recursive structures as it can easily help reducing their depth in case of failure_
 - `depthFactor?` — default: `0` — _this factor will be used to increase the probability to generate instances of the first passed arbitrary if its weight is not zero_
 - `maxDepth?` — default: `Number.POSITIVE_INFINITY` — _when reaching maxDepth, the first arbitrary will be used to generate the value even if its weight is zero_
+- `depthIdentifier?` — default: `undefined` — _share the depth between instances using the same `depthIdentifier`_
 
 *&#8195;Usages*
 
diff --git a/src/check/arbitrary/FrequencyArbitrary.ts b/src/check/arbitrary/FrequencyArbitrary.ts
index e1be46c0..8bef6195 100644
--- a/src/check/arbitrary/FrequencyArbitrary.ts
+++ b/src/check/arbitrary/FrequencyArbitrary.ts
@@ -2,12 +2,7 @@ import { Random } from '../../random/generator/Random';
 import { Stream } from '../../stream/Stream';
 import { Arbitrary } from './definition/Arbitrary';
 import { Shrinkable } from './definition/Shrinkable';
-
-/** @internal */
-type DepthContext = {
-  /** Current depth (starts at 0) */
-  depth: number;
-};
+import { DepthContext, getDepthContextFor } from './helpers/DepthContext';
 
 /**
  * Conjonction of a weight and an arbitrary used by {@link frequency}
@@ -56,7 +51,7 @@ export class FrequencyArbitrary<T> extends Arbitrary<T> {
     if (totalWeight <= 0) {
       throw new Error(`${label} expects the sum of weights to be strictly superior to 0`);
     }
-    return new FrequencyArbitrary(warbs, constraints, { depth: 0 });
+    return new FrequencyArbitrary(warbs, constraints, getDepthContextFor(constraints.depthIdentifier));
   }
 
   private constructor(
@@ -193,6 +188,13 @@ export type FrequencyContraints = {
    * @remarks Since 2.14.0
    */
   maxDepth?: number;
+  /**
+   * Depth identifier can be used to share the current depth between several instances.
+   *
+   * By default, if not specified, each instance of frequency will have its own depth.
+   * In other words: you can have depth=1 in one while you have depth=100 in another one.
+   */
+  depthIdentifier?: string;
 };
 
 /**
diff --git a/src/check/arbitrary/helpers/DepthContext.ts b/src/check/arbitrary/helpers/DepthContext.ts
new file mode 100644
index 00000000..350ab42a
--- /dev/null
+++ b/src/check/arbitrary/helpers/DepthContext.ts
@@ -0,0 +1,31 @@
+/** @internal */
+export type DepthContext = {
+  /** Current depth (starts at 0) */
+  depth: number;
+};
+
+/**
+ * Internal cache for depth contexts
+ * @internal
+ */
+const depthContextCache = new Map<string, DepthContext>();
+
+/**
+ * Get back the requested DepthContext
+ * @internal
+ */
+export function getDepthContextFor(contextMeta: DepthContext | string | undefined): DepthContext {
+  if (contextMeta === undefined) {
+    return { depth: 0 };
+  }
+  if (typeof contextMeta !== 'string') {
+    return contextMeta;
+  }
+  const cachedContext = depthContextCache.get(contextMeta);
+  if (cachedContext !== undefined) {
+    return cachedContext;
+  }
+  const context = { depth: 0 };
+  depthContextCache.set(contextMeta, context);
+  return context;
+}


Test patch:
diff --git a/test/unit/check/arbitrary/FrequencyArbitrary.itest.spec.ts b/test/unit/check/arbitrary/FrequencyArbitrary.itest.spec.ts
index a7d42545..a8a91e09 100644
--- a/test/unit/check/arbitrary/FrequencyArbitrary.itest.spec.ts
+++ b/test/unit/check/arbitrary/FrequencyArbitrary.itest.spec.ts
@@ -6,6 +6,26 @@ import { constant } from '../../../../src/check/arbitrary/ConstantArbitrary';
 
 import * as genericHelper from './generic/GenericArbitraryHelper';
 
+import { mocked } from 'ts-jest/utils';
+import * as DepthContextMock from '../../../../src/check/arbitrary/helpers/DepthContext';
+jest.mock('../../../../src/check/arbitrary/helpers/DepthContext');
+
+const depthContextData: Record<string, DepthContextMock.DepthContext> = {};
+
+beforeEach(() => {
+  // Cleaning between runs (WARNING: Doew not clean within properties themselves)
+  for (const k in depthContextData) delete depthContextData[k];
+
+  // Mocking
+  const { getDepthContextFor } = mocked(DepthContextMock);
+  getDepthContextFor.mockImplementation((key) => {
+    if (key === undefined) return { depth: 0 };
+    if (typeof key !== 'string') return key;
+    if (!(key in depthContextData)) depthContextData[key] = { depth: 0 };
+    return depthContextData[key];
+  });
+});
+
 describe('FrequencyArbitrary', () => {
   describe('frequency', () => {
     const seedGenerator = fc.record(
@@ -28,14 +48,32 @@ describe('FrequencyArbitrary', () => {
             withCrossShrink: fc.boolean(),
             depthFactor: fc.double({ min: 0, max: Number.MAX_VALUE, noNaN: true }),
             maxDepth: fc.nat(),
+            depthIdentifierMetas: fc.record(
+              {
+                identifier: fc.constantFrom('toto', 'titi', 'tata'), // no need to have thousands of them
+                initial: fc.nat(),
+              },
+              { requiredKeys: ['identifier', 'initial'] }
+            ),
           },
           { requiredKeys: [] }
         ),
       },
       { requiredKeys: ['data'] }
     );
+
     type SeedGeneratorType = typeof seedGenerator extends fc.Arbitrary<infer T> ? T : never;
 
+    function canProduce(dataFromMetas: SeedGeneratorType['data'][number], value: number): boolean {
+      switch (dataFromMetas.type) {
+        case 'unique':
+          return dataFromMetas.value === value;
+        case 'range':
+          return dataFromMetas.value - 10 <= value && value <= dataFromMetas.value;
+      }
+      return false;
+    }
+
     genericHelper.isValidArbitrary(
       (metas: SeedGeneratorType) => {
         const arbs = metas.data.map((m) =>
@@ -46,20 +84,34 @@ describe('FrequencyArbitrary', () => {
         if (metas.constraints === undefined) {
           return frequency(...arbs);
         }
-        return frequency(metas.constraints, ...arbs);
+
+        const sanitizedConstraints =
+          metas.constraints.depthIdentifierMetas !== undefined
+            ? { ...metas.constraints, depthIdentifier: metas.constraints.depthIdentifierMetas.identifier }
+            : metas.constraints;
+        if (metas.constraints.depthIdentifierMetas !== undefined) {
+          // WARNING - This side-effect is not supposed to alter other tests but who knows!?
+          const { identifier, initial } = metas.constraints.depthIdentifierMetas;
+          depthContextData[identifier] = { depth: initial };
+        }
+        return frequency(sanitizedConstraints, ...arbs);
       },
       {
         seedGenerator,
         isValidValue: (v: number, metas: SeedGeneratorType) => {
-          // If maxDepth is 0, then only the first arbitrary can be called
-          const data =
-            metas.constraints !== undefined && metas.constraints.maxDepth === 0 ? [metas.data[0]] : metas.data;
-          for (const m of data) {
-            if (m.weight === 0) continue;
-            if (m.type === 'unique' && m.value === v) return true;
-            if (m.type === 'range' && m.value - 10 <= v && v <= m.value) return true;
+          const constraints = metas.constraints || {};
+          const { identifier, initial: initialDepth = 0 } = constraints.depthIdentifierMetas || {};
+          // Check if depth has been reset after generate
+          if (identifier !== undefined) {
+            expect(depthContextData[identifier]).toEqual({ depth: initialDepth });
+          }
+          // If maxDepth is <= initialDepth, then only the first arbitrary can be called
+          if (constraints.maxDepth !== undefined && constraints.maxDepth <= initialDepth) {
+            expect(canProduce(metas.data[0], v)).toBe(true);
+          } else {
+            expect(metas.data.some((m) => m.weight !== 0 && canProduce(m, v))).toBe(true);
           }
-          return false;
+          return true;
         },
         isStrictlySmallerValue: (a: number, b: number, metas: SeedGeneratorType) => {
           // When withCrossShrink is toggled, the shrinker can jump from one arbitrary to the first one on shrink
diff --git a/test/unit/check/arbitrary/FrequencyArbitrary.utest.spec.ts b/test/unit/check/arbitrary/FrequencyArbitrary.utest.spec.ts
index 95a60e84..ea681456 100644
--- a/test/unit/check/arbitrary/FrequencyArbitrary.utest.spec.ts
+++ b/test/unit/check/arbitrary/FrequencyArbitrary.utest.spec.ts
@@ -145,6 +145,37 @@ describe('FrequencyArbitrary', () => {
       expect(nextInt).toHaveBeenCalledTimes(3); // once per instance of frequency
       expect(g).toEqual([[1]]);
     });
+    it('Should share depth accross distinct instances of frequency if requested', () => {
+      // Arrange
+      const arb: Arbitrary<any> = frequency(
+        { maxDepth: 1, depthIdentifier: 'toto' },
+        { weight: 0, arbitrary: stubArb.single(0) },
+        {
+          weight: 1,
+          arbitrary: frequency(
+            { maxDepth: 1, depthIdentifier: 'titi' },
+            { weight: 0, arbitrary: stubArb.single(0) },
+            {
+              weight: 1,
+              arbitrary: frequency(
+                { maxDepth: 1, depthIdentifier: 'toto' },
+                { weight: 0, arbitrary: stubArb.single(0) },
+                { weight: 1, arbitrary: stubArb.single(1) }
+              ).map((d) => [d]),
+            }
+          ).map((d) => [d]),
+        }
+      );
+      const nextInt: jest.Mock<number, [] | [number] | [number, number]> = jest.fn().mockReturnValue(0);
+      const fakeRandom = { nextInt: nextInt as Random['nextInt'] } as Random;
+
+      // Act
+      const g = arb.generate(fakeRandom).value_;
+
+      // Assert
+      expect(nextInt).toHaveBeenCalledTimes(2); // one for toto, one for titi
+      expect(g).toEqual([[0]]);
+    });
     it('Should ask ranges containing negative values as we go deeper in the structure if depthFactor', () => {
       // Arrange
       const arb: Arbitrary<any> = frequency(
diff --git a/test/unit/check/arbitrary/generic/GenericArbitraryHelper.ts b/test/unit/check/arbitrary/generic/GenericArbitraryHelper.ts
index c916d51b..d62258ee 100644
--- a/test/unit/check/arbitrary/generic/GenericArbitraryHelper.ts
+++ b/test/unit/check/arbitrary/generic/GenericArbitraryHelper.ts
@@ -92,7 +92,7 @@ const testShrinkPathStrictlyDecreasing = function <U, T>(
 const testAlwaysGenerateCorrectValues = function <U, T>(
   argsForArbGenerator: fc.Arbitrary<U>,
   arbGenerator: (u: U) => Arbitrary<T>,
-  isValidValue: (g: T, seed: U) => boolean,
+  assertIsValidValue: (g: T, seed: U) => void,
   parameters: fc.Parameters<unknown> | undefined
 ) {
   it(`Should always generate correct values`, () =>
@@ -100,8 +100,7 @@ const testAlwaysGenerateCorrectValues = function <U, T>(
       fc.property(argsForArbGenerator, fc.integer().noShrink(), (params, seed) => {
         const arb = arbGenerator(params);
         const shrinkable = arb.generate(new Random(prand.xorshift128plus(seed)));
-        if (!isValidValue(shrinkable.value, params))
-          throw new Error(`Expect: ${fc.stringify(shrinkable.value_)} to be a valid value`);
+        assertIsValidValue(shrinkable.value, params);
       }),
       parameters
     ));
@@ -110,7 +109,7 @@ const testAlwaysGenerateCorrectValues = function <U, T>(
 const testAlwaysShrinkToCorrectValues = function <U, T>(
   argsForArbGenerator: fc.Arbitrary<U>,
   arbGenerator: (u: U) => Arbitrary<T>,
-  isValidValue: (g: T, seed: U) => boolean,
+  assertIsValidValue: (g: T, seed: U) => void,
   parameters: fc.Parameters<unknown> | undefined
 ) {
   it(`Should always shrink to correct values`, () =>
@@ -124,8 +123,7 @@ const testAlwaysShrinkToCorrectValues = function <U, T>(
           let shrinkable: Shrinkable<T> | null = arb.generate(new Random(prand.xorshift128plus(seed)));
           let id = 0;
           while (shrinkable !== null) {
-            if (!isValidValue(shrinkable.value, params))
-              throw new Error(`Invalid value: ${fc.stringify(shrinkable.value_)}`);
+            assertIsValidValue(shrinkable.value, params);
             shrinkable = shrinkable.shrink().getNthOrLast(id);
             id = (id + 1) % shrinkPath.length;
           }
@@ -151,11 +149,16 @@ export const isValidArbitrary = function <U, T>(
   const biasedArbitraryBuilder = ([biasedFactor, u]: [number | null, U]) => {
     return biasedFactor != null ? arbitraryBuilder(u).withBias(biasedFactor) : arbitraryBuilder(u);
   };
-  const biasedIsValidValue = (g: T, [_biasedFactor, u]: [number | null, U]) => {
-    return settings.isValidValue(g, u!);
+  const biasedAssertIsValidValue = (g: T, [_biasedFactor, u]: [number | null, U]): void => {
+    try {
+      const out = settings.isValidValue(g, u!);
+      expect(out).toBe(true);
+    } catch (err) {
+      throw new Error(`Expect: ${fc.stringify(g)} to be a valid value\n\nGot error: ${err}`);
+    }
   };
 
-  const assertEquality = (v1: T, v2: T, [, seed]: [number | null, U]) => {
+  const assertEquality = (v1: T, v2: T, [, seed]: [number | null, U]): void => {
     if (settings.isEqual) {
       if (!settings.isEqual(v1, v2, seed!)) {
         throw new Error(`Expect: ${fc.stringify(v1)} to be equal to ${fc.stringify(v2)}`);
@@ -176,8 +179,8 @@ export const isValidArbitrary = function <U, T>(
       parameters
     );
   }
-  testAlwaysGenerateCorrectValues(biasedSeedGenerator, biasedArbitraryBuilder, biasedIsValidValue, parameters);
-  testAlwaysShrinkToCorrectValues(biasedSeedGenerator, biasedArbitraryBuilder, biasedIsValidValue, parameters);
+  testAlwaysGenerateCorrectValues(biasedSeedGenerator, biasedArbitraryBuilder, biasedAssertIsValidValue, parameters);
+  testAlwaysShrinkToCorrectValues(biasedSeedGenerator, biasedArbitraryBuilder, biasedAssertIsValidValue, parameters);
 };
 
 export const minMax = <NType extends number | bigint>(