Repository: dubzzz/fast-check
Pull request #1611 (2021-03-14 20:43:32)

✨ Add recursive constraints on `fc.oneof`
<!-- 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: *please explain*

(✔️: 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/1611/builds/109555) or the icon next to each commit SHA.

Latest deployment of this branch, based on commit f9df4728b9e1be3ddfc9b85d0da683196348e255:

|Sandbox| Source |
|--|--|
|[Vanilla](https://codesandbox.io/s/vanilla-q43p2)| Configuration |
|[fast-check-examples](https://codesandbox.io/s/fast-check-examples-486o1)| Configuration |

codecov[bot]: # [Codecov](https://codecov.io/gh/dubzzz/fast-check/pull/1611?src=pr&el=h1) Report
> Merging [#1611](https://codecov.io/gh/dubzzz/fast-check/pull/1611?src=pr&el=desc) (f9df472) into [main](https://codecov.io/gh/dubzzz/fast-check/commit/6cd646780683ae5c70eaa2cd11e63e51d678459f?el=desc) (6cd6467) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/dubzzz/fast-check/pull/1611/graphs/tree.svg?width=650&height=150&src=pr&token=fRi2fJN730)](https://codecov.io/gh/dubzzz/fast-check/pull/1611?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##             main    #1611   +/-   ##
=======================================
  Coverage   97.44%   97.45%           
=======================================
  Files          93       93           
  Lines        3208     3216    +8     
  Branches      624      626    +2     
=======================================
+ Hits         3126     3134    +8     
  Misses         78       78           
  Partials        4        4           
```

| Flag | Coverage Δ | |
|---|---|---|
| unit-tests | `97.45% <100.00%> (+<0.01%)` | :arrow_up: |
| unit-tests-10.x-Linux | `?` | |
| unit-tests-12.x-Linux | `?` | |
| unit-tests-14.x-Linux | `97.45% <100.00%> (+0.03%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/dubzzz/fast-check/pull/1611?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/check/arbitrary/FrequencyArbitrary.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1611/diff?src=pr&el=tree#diff-c3JjL2NoZWNrL2FyYml0cmFyeS9GcmVxdWVuY3lBcmJpdHJhcnkudHM=) | `98.64% <ø> (ø)` | |
| [src/check/arbitrary/OneOfArbitrary.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1611/diff?src=pr&el=tree#diff-c3JjL2NoZWNrL2FyYml0cmFyeS9PbmVPZkFyYml0cmFyeS50cw==) | `100.00% <100.00%> (ø)` | |
| [src/check/runner/DecorateProperty.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1611/diff?src=pr&el=tree#diff-c3JjL2NoZWNrL3J1bm5lci9EZWNvcmF0ZVByb3BlcnR5LnRz) | `100.00% <0.00%> (ø)` | |
| [src/check/property/IgnoreEqualValuesProperty.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1611/diff?src=pr&el=tree#diff-c3JjL2NoZWNrL3Byb3BlcnR5L0lnbm9yZUVxdWFsVmFsdWVzUHJvcGVydHkudHM=) | `100.00% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/dubzzz/fast-check/pull/1611?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/dubzzz/fast-check/pull/1611?src=pr&el=footer). Last update [6cd6467...f9df472](https://codecov.io/gh/dubzzz/fast-check/pull/1611?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/documentation/Arbitraries.md b/documentation/Arbitraries.md
index 89019dff..ff7f48e9 100644
--- a/documentation/Arbitraries.md
+++ b/documentation/Arbitraries.md
@@ -2147,10 +2147,15 @@ fc.letrec(tie => ({
 *&#8195;Signatures*
 
 - `fc.oneof(...arbitraries)`
+- `fc.oneof({withCrossShrink?, maxDepth?, depthIdentifier?}, ...{ arbitrary, weight })`
 
 *&#8195;with:*
 
 - `...arbitraries` — _arbitraries that could be used to generate a value_
+- `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. 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_
+- `maxDepth?` — default: `Number.POSITIVE_INFINITY` — _when reaching maxDepth, the first arbitrary will be used to generate the value_
+- `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 a8f6c2f8..c37918af 100644
--- a/src/check/arbitrary/FrequencyArbitrary.ts
+++ b/src/check/arbitrary/FrequencyArbitrary.ts
@@ -168,7 +168,7 @@ export type FrequencyContraints = {
    */
   withCrossShrink?: boolean;
   /**
-   * While going deeper and deeper within a recursive structure,
+   * While going deeper and deeper within a recursive structure (see {@link letrec}),
    * this factor will be used to increase the probability to generate instances
    * of the first passed arbitrary.
    *
@@ -224,6 +224,7 @@ function frequency<Ts extends WeightedArbitrary<unknown>[]>(...warbs: Ts): Arbit
  *
  * **WARNING**: It expects at least one (Arbitrary, weight)
  *
+ * @param constraints - Constraints to be applied when generating the values
  * @param warbs - (Arbitrary, weight)s that might be called to produce a value
  *
  * @remarks Since 0.0.7
diff --git a/src/check/arbitrary/OneOfArbitrary.ts b/src/check/arbitrary/OneOfArbitrary.ts
index c092dfc9..da720d50 100644
--- a/src/check/arbitrary/OneOfArbitrary.ts
+++ b/src/check/arbitrary/OneOfArbitrary.ts
@@ -12,6 +12,57 @@ export type OneOfValue<Ts extends Arbitrary<unknown>[]> = {
   [K in keyof Ts]: Ts[K] extends Arbitrary<infer U> ? U : never;
 }[number];
 
+/**
+ * Constraints to be applied on {@link oneof}
+ * @remarks Since 2.14.0
+ * @public
+ */
+export type OneOfContraints = {
+  /**
+   * When set to true, the shrinker of oneof will try to check if the first arbitrary
+   * could have been used to discover an issue. It allows to shrink trees.
+   *
+   * Warning: First arbitrary must be the one resulting in the smallest structures
+   * for usages in deep tree-like structures.
+   *
+   * @remarks Since 2.14.0
+   */
+  withCrossShrink?: boolean;
+  /**
+   * While going deeper and deeper within a recursive structure (see {@link letrec}),
+   * this factor will be used to increase the probability to generate instances
+   * of the first passed arbitrary.
+   *
+   * Example of values: 0.1 (small impact as depth increases), 0.5, 1 (huge impact as depth increases).
+   *
+   * @remarks Since 2.14.0
+   */
+  depthFactor?: number;
+  /**
+   * Maximal authorized depth.
+   * Once this depth has been reached only the first arbitrary will be used.
+   *
+   * @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 oneof will have its own depth.
+   * In other words: you can have depth=1 in one while you have depth=100 in another one.
+   *
+   * @remarks Since 2.14.0
+   */
+  depthIdentifier?: string;
+};
+
+/**
+ * @internal
+ */
+function isOneOfContraints(param: OneOfContraints | Arbitrary<unknown> | undefined): param is OneOfContraints {
+  return param != null && typeof param === 'object' && !('generate' in param);
+}
+
 /**
  * For one of the values generated by `...arbs` - with all `...arbs` equiprobable
  *
@@ -22,8 +73,29 @@ export type OneOfValue<Ts extends Arbitrary<unknown>[]> = {
  * @remarks Since 0.0.1
  * @public
  */
-function oneof<Ts extends Arbitrary<unknown>[]>(...arbs: Ts): Arbitrary<OneOfValue<Ts>> {
-  const weightedArbs = (arbs as Arbitrary<OneOfValue<Ts>>[]).map((arbitrary) => ({ arbitrary, weight: 1 }));
+function oneof<Ts extends Arbitrary<unknown>[]>(...arbs: Ts): Arbitrary<OneOfValue<Ts>>;
+/**
+ * For one of the values generated by `...arbs` - with all `...arbs` equiprobable
+ *
+ * **WARNING**: It expects at least one arbitrary
+ *
+ * @param constraints - Constraints to be applied when generating the values
+ * @param arbs - Arbitraries that might be called to produce a value
+ *
+ * @remarks Since 2.14.0
+ * @public
+ */
+function oneof<Ts extends Arbitrary<unknown>[]>(constraints: OneOfContraints, ...arbs: Ts): Arbitrary<OneOfValue<Ts>>;
+function oneof<Ts extends Arbitrary<unknown>[]>(
+  ...args: [...Ts] | [OneOfContraints, ...Ts]
+): Arbitrary<OneOfValue<Ts>> {
+  // TODO With TypeScript 4.0 it will be possible to properly define typings for `oneof(...arbs, constraints)`
+  const constraints = args[0];
+  if (isOneOfContraints(constraints)) {
+    const weightedArbs = (args.slice(1) as Arbitrary<OneOfValue<Ts>>[]).map((arbitrary) => ({ arbitrary, weight: 1 }));
+    return FrequencyArbitrary.from(weightedArbs, constraints, 'fc.oneof');
+  }
+  const weightedArbs = (args as Arbitrary<OneOfValue<Ts>>[]).map((arbitrary) => ({ arbitrary, weight: 1 }));
   return FrequencyArbitrary.from(weightedArbs, {}, 'fc.oneof');
 }
 


Test patch:
diff --git a/test/e2e/RecursiveStructures.spec.ts b/test/e2e/RecursiveStructures.spec.ts
index 5a5d78a2..06a0a20c 100644
--- a/test/e2e/RecursiveStructures.spec.ts
+++ b/test/e2e/RecursiveStructures.spec.ts
@@ -25,6 +25,21 @@ describe(`RecursiveStructures (seed: ${seed})`, () => {
     expect(flat(out.counterexample![0])).toHaveLength(failingLength);
   });
 
+  it('Should shrink letrec/oneof towards the smallest case (on very simple scenario)', () => {
+    // Arrange
+    const failingLength = 2;
+    const dataArb = fc.letrec((tie) => ({
+      data: fc.oneof({ withCrossShrink: true, depthFactor: 0.5 }, fc.constant([0]), fc.tuple(tie('data'), tie('data'))),
+    })).data;
+
+    // Act
+    const out = fc.check(fc.property(dataArb, (data) => flat(data).length < failingLength));
+
+    // Assert
+    expect(out.failed).toBe(true);
+    expect(flat(out.counterexample![0])).toHaveLength(failingLength);
+  });
+
   it('Should shrink letrec/option towards the smallest case (on very simple scenario)', () => {
     // Arrange
     const failingLength = 2;
@@ -61,6 +76,22 @@ describe(`RecursiveStructures (seed: ${seed})`, () => {
     expect(flat(out.counterexample![0])).toHaveLength(failingLength);
   });
 
+  it('Should shrink memo/oneof towards the smallest case (on very simple scenario)', () => {
+    // Arrange
+    const failingLength = 2;
+    const dataArb: fc.Memo<unknown[]> = fc.memo((n) => {
+      if (n <= 1) return fc.constant([0]);
+      else return fc.oneof({ withCrossShrink: true }, fc.constant([0]), fc.tuple(dataArb(), dataArb()));
+    });
+
+    // Act
+    const out = fc.check(fc.property(dataArb(5), (data) => flat(data).length < failingLength));
+
+    // Assert
+    expect(out.failed).toBe(true);
+    expect(flat(out.counterexample![0])).toHaveLength(failingLength);
+  });
+
   it('Should shrink memo/option towards the smallest case (on very simple scenario)', () => {
     // Arrange
     const failingLength = 2;
diff --git a/test/type/main.ts b/test/type/main.ts
index a9113d59..fd06d151 100644
--- a/test/type/main.ts
+++ b/test/type/main.ts
@@ -249,6 +249,14 @@ expectType<fc.Arbitrary<string | number>>()(
   fc.oneof(fc.string(), fc.nat()),
   '"oneof" with a multiple arguments of different types'
 );
+expectType<fc.Arbitrary<string | number>>()(
+  fc.oneof({}, fc.string(), fc.nat()),
+  '"oneof" with different types and empty constraints'
+);
+expectType<fc.Arbitrary<string | number>>()(
+  fc.oneof({ withCrossShrink: true }, fc.string(), fc.nat()),
+  '"oneof" with different types and some constraints'
+);
 // @ts-expect-error - oneof expects arbitraries not raw values
 fc.oneof(fc.string(), '1');
 
diff --git a/test/unit/check/arbitrary/OneOfArbitrary.spec.ts b/test/unit/check/arbitrary/OneOfArbitrary.itest.spec.ts
similarity index 100%
rename from test/unit/check/arbitrary/OneOfArbitrary.spec.ts
rename to test/unit/check/arbitrary/OneOfArbitrary.itest.spec.ts
diff --git a/test/unit/check/arbitrary/OneOfArbitrary.utest.spec.ts b/test/unit/check/arbitrary/OneOfArbitrary.utest.spec.ts
new file mode 100644
index 00000000..6ae4aa0d
--- /dev/null
+++ b/test/unit/check/arbitrary/OneOfArbitrary.utest.spec.ts
@@ -0,0 +1,73 @@
+import { mocked } from 'ts-jest/utils';
+
+import { constant } from '../../../../src/check/arbitrary/ConstantArbitrary';
+import { oneof, OneOfContraints } from '../../../../src/check/arbitrary/OneOfArbitrary';
+
+import * as FrequencyArbitraryMock from '../../../../src/check/arbitrary/FrequencyArbitrary';
+jest.mock('../../../../src/check/arbitrary/FrequencyArbitrary');
+
+describe('OneOfArbitrary', () => {
+  describe('oneof', () => {
+    it('Should call frequency with empty constraints if no constraints have been passed', () => {
+      // Arrange
+      const from = mocked(FrequencyArbitraryMock.FrequencyArbitrary.from);
+      const arb1 = constant(1);
+      const arb2 = constant(2);
+
+      // Act
+      oneof(arb1, arb2);
+
+      // Assert
+      expect(from).toHaveBeenCalledWith(
+        [
+          { arbitrary: arb1, weight: 1 },
+          { arbitrary: arb2, weight: 1 },
+        ],
+        {}, // empty constraints
+        'fc.oneof'
+      );
+    });
+
+    it('Should pass received constraints to frequency', () => {
+      // Arrange
+      const from = mocked(FrequencyArbitraryMock.FrequencyArbitrary.from);
+      const constraints: OneOfContraints = { maxDepth: 10, depthIdentifier: 'hello' };
+      const arb1 = constant(1);
+      const arb2 = constant(2);
+
+      // Act
+      oneof(constraints, arb1, arb2);
+
+      // Assert
+      expect(from).toHaveBeenCalledWith(
+        [
+          { arbitrary: arb1, weight: 1 },
+          { arbitrary: arb2, weight: 1 },
+        ],
+        constraints,
+        'fc.oneof'
+      );
+    });
+
+    it('Should pass received constraints to frequency even if empty', () => {
+      // Arrange
+      const from = mocked(FrequencyArbitraryMock.FrequencyArbitrary.from);
+      const constraints: OneOfContraints = {};
+      const arb1 = constant(1);
+      const arb2 = constant(2);
+
+      // Act
+      oneof(constraints, arb1, arb2);
+
+      // Assert
+      expect(from).toHaveBeenCalledWith(
+        [
+          { arbitrary: arb1, weight: 1 },
+          { arbitrary: arb2, weight: 1 },
+        ],
+        constraints,
+        'fc.oneof'
+      );
+    });
+  });
+});