Repository: dubzzz/fast-check
Pull request #1581 (2021-03-04 07:15:08)

✨ Reject invalid weights on `fc.frequency`
<!-- 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

`fc.frequency` may reject invalid inputs earlier


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/1581/builds/106708) or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1b4d0f02eaa0c54fbc19328ec03e8d36c6518e1e:

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

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

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

```diff
@@           Coverage Diff           @@
##             main    #1581   +/-   ##
=======================================
  Coverage   97.42%   97.43%           
=======================================
  Files          92       92           
  Lines        3150     3158    +8     
  Branches      611      613    +2     
=======================================
+ Hits         3069     3077    +8     
  Misses         77       77           
  Partials        4        4           
```

| Flag | Coverage Δ | |
|---|---|---|
| unit-tests | `97.43% <100.00%> (+<0.01%)` | :arrow_up: |
| unit-tests-10.x-Linux | `?` | |
| unit-tests-12.x-Linux | `97.43% <100.00%> (+<0.01%)` | :arrow_up: |
| unit-tests-14.x-Linux | `97.43% <100.00%> (+<0.01%)` | :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/1581?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/check/arbitrary/FrequencyArbitrary.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1581/diff?src=pr&el=tree#diff-c3JjL2NoZWNrL2FyYml0cmFyeS9GcmVxdWVuY3lBcmJpdHJhcnkudHM=) | `96.77% <100.00%> (+1.12%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/dubzzz/fast-check/pull/1581?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/1581?src=pr&el=footer). Last update [bc12c14...1b4d0f0](https://codecov.io/gh/dubzzz/fast-check/pull/1581?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/check/arbitrary/FrequencyArbitrary.ts b/src/check/arbitrary/FrequencyArbitrary.ts
index 0be36142..e8818d8e 100644
--- a/src/check/arbitrary/FrequencyArbitrary.ts
+++ b/src/check/arbitrary/FrequencyArbitrary.ts
@@ -31,6 +31,17 @@ class FrequencyArbitrary<T> extends Arbitrary<T> {
     if (warbs.length === 0) {
       throw new Error('fc.frequency expects at least one weigthed arbitrary');
     }
+    let totalWeight = 0;
+    for (let idx = 0; idx !== warbs.length; ++idx) {
+      const currentWeight = warbs[idx].weight;
+      totalWeight += currentWeight;
+      if (currentWeight < 0) {
+        throw new Error('fc.frequency expects weights to be superior or equal to 0');
+      }
+    }
+    if (totalWeight <= 0) {
+      throw new Error('fc.frequency expects the sum of weights to be strictly superior to 0');
+    }
     return new FrequencyArbitrary(warbs);
   }
   private constructor(readonly warbs: WeightedArbitrary<T>[]) {


Test patch:
diff --git a/test/unit/check/arbitrary/FrequencyArbitrary.spec.ts b/test/unit/check/arbitrary/FrequencyArbitrary.spec.ts
index 2391b9c7..ba6d8ea4 100644
--- a/test/unit/check/arbitrary/FrequencyArbitrary.spec.ts
+++ b/test/unit/check/arbitrary/FrequencyArbitrary.spec.ts
@@ -38,5 +38,56 @@ describe('FrequencyArbitrary', () => {
     it('Should reject calls without any weighted arbitraries', () => {
       expect(() => frequency()).toThrowError();
     });
+    it('Should reject calls including at one strictly negative weight', () =>
+      fc.assert(
+        fc.property(
+          fc.integer({ max: -1 }),
+          fc.array(fc.nat()),
+          fc.array(fc.nat()),
+          (negativeWeight, headingWeights, traillingWeights) => {
+            expect(() =>
+              frequency(
+                ...[...headingWeights, negativeWeight, ...traillingWeights].map((weight) => ({
+                  weight,
+                  arbitrary: stubArb.single(0),
+                }))
+              )
+            ).toThrowError();
+          }
+        )
+      ));
+    it('Should reject calls having a total weight of zero', () =>
+      fc.assert(
+        fc.property(fc.nat({ max: 1000 }), (numEntries) => {
+          // Combined with: 'Should reject calls including at one strictly negative weight'
+          // it means that we have: 'Should reject calls having a total weight inferior or equal to zero'
+          expect(() =>
+            frequency(
+              ...[...Array(numEntries)].map(() => ({
+                weight: 0,
+                arbitrary: stubArb.single(0),
+              }))
+            )
+          ).toThrowError();
+        })
+      ));
+    it('Should not reject calls defining a strictly positive total weight without any negative weights', () =>
+      fc.assert(
+        fc.property(
+          fc.integer({ min: 1 }),
+          fc.array(fc.nat()),
+          fc.array(fc.nat()),
+          (negativeWeight, headingWeights, traillingWeights) => {
+            expect(() =>
+              frequency(
+                ...[...headingWeights, negativeWeight, ...traillingWeights].map((weight) => ({
+                  weight,
+                  arbitrary: stubArb.single(0),
+                }))
+              )
+            ).not.toThrowError();
+          }
+        )
+      ));
   });
 });