Repository: dubzzz/fast-check
Pull request #1776 (2021-04-20 07:34:33)

🐛 `ConstantArbitrary` must not shrink towards the first if it is already the first
<!-- 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

Shrink values impact: very very rare, only for NaN or -0, 0.


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

Latest deployment of this branch, based on commit 0305ac02e763880efb5ee117983a176782e2c0f5:

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

codecov[bot]: # [Codecov](https://codecov.io/gh/dubzzz/fast-check/pull/1776?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nicolas+DUBIEN) Report
> Merging [#1776](https://codecov.io/gh/dubzzz/fast-check/pull/1776?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nicolas+DUBIEN) (0305ac0) into [main](https://codecov.io/gh/dubzzz/fast-check/commit/a4815c51d13ac8e7c564a4f92085d0d0a8c3cfca?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nicolas+DUBIEN) (a4815c5) will **not change** coverage.
> The diff coverage is `100.00%`.

> :exclamation: Current head 0305ac0 differs from pull request most recent head f26a718. Consider uploading reports for the commit f26a718 to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/dubzzz/fast-check/pull/1776/graphs/tree.svg?width=650&height=150&src=pr&token=fRi2fJN730&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nicolas+DUBIEN)](https://codecov.io/gh/dubzzz/fast-check/pull/1776?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nicolas+DUBIEN)

```diff
@@           Coverage Diff           @@
##             main    #1776   +/-   ##
=======================================
  Coverage   96.62%   96.62%           
=======================================
  Files         117      117           
  Lines        3647     3647           
  Branches      716      716           
=======================================
  Hits         3524     3524           
  Misses        119      119           
  Partials        4        4           
```

| Flag | Coverage Δ | |
|---|---|---|
| unit-tests | `96.62% <100.00%> (ø)` | |
| unit-tests-10.x-Linux | `96.62% <100.00%> (ø)` | |
| unit-tests-12.x-Linux | `96.62% <100.00%> (ø)` | |
| unit-tests-14.x-Linux | `96.62% <100.00%> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nicolas+DUBIEN#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/dubzzz/fast-check/pull/1776?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nicolas+DUBIEN) | Coverage Δ | |
|---|---|---|
| [src/arbitrary/\_internals/ConstantArbitrary.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1776/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nicolas+DUBIEN#diff-c3JjL2FyYml0cmFyeS9faW50ZXJuYWxzL0NvbnN0YW50QXJiaXRyYXJ5LnRz) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/dubzzz/fast-check/pull/1776?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nicolas+DUBIEN).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nicolas+DUBIEN)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/dubzzz/fast-check/pull/1776?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nicolas+DUBIEN). Last update [a4815c5...f26a718](https://codecov.io/gh/dubzzz/fast-check/pull/1776?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nicolas+DUBIEN). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Nicolas+DUBIEN).


Patch:
diff --git a/src/arbitrary/_internals/ConstantArbitrary.ts b/src/arbitrary/_internals/ConstantArbitrary.ts
index 397d2d97..f39b35ae 100644
--- a/src/arbitrary/_internals/ConstantArbitrary.ts
+++ b/src/arbitrary/_internals/ConstantArbitrary.ts
@@ -25,7 +25,7 @@ export class ConstantArbitrary<T> extends NextArbitrary<T> {
     return false;
   }
   shrink(value: T, context?: unknown): Stream<NextValue<T>> {
-    if (context === 0 || value === this.values[0]) {
+    if (context === 0 || Object.is(value, this.values[0])) {
       return Stream.nil();
     }
     return Stream.of(new NextValue(this.values[0], 0));


Test patch:
diff --git a/test/unit/arbitrary/_internals/ConstantArbitrary.spec.ts b/test/unit/arbitrary/_internals/ConstantArbitrary.spec.ts
index 8989a45c..4156b22b 100644
--- a/test/unit/arbitrary/_internals/ConstantArbitrary.spec.ts
+++ b/test/unit/arbitrary/_internals/ConstantArbitrary.spec.ts
@@ -156,7 +156,7 @@ describe('ConstantArbitrary', () => {
           const shrinks = [...arb.shrink(value.value, value.context)];
 
           // Assert
-          if (value.value === values[0]) {
+          if (Object.is(value.value, values[0])) {
             expect(shrinks.map((v) => v.value)).toEqual([]);
           } else {
             expect(shrinks.map((v) => v.value)).toEqual([values[0]]);
@@ -177,34 +177,41 @@ describe('ConstantArbitrary', () => {
           const shrinks = [...arb.shrink(value.value, undefined)];
 
           // Assert
-          if (value.value === values[0]) {
+          if (Object.is(value.value, values[0])) {
             expect(shrinks.map((v) => v.value)).toEqual([]);
           } else {
             expect(shrinks.map((v) => v.value)).toEqual([values[0]]);
           }
         })
       ));
+
+    it('should not shrink towards the first value if generated value is equal to the first one regarding `Object.is`', () => {
+      // Arrange
+      const { instance: mrng, nextInt } = fakeRandom();
+
+      // Act / Assert
+      const arb = new ConstantArbitrary([Number.NaN, Number.NaN, Number.NaN]);
+      for (let idx = 0; idx !== 3; ++idx) {
+        nextInt.mockReturnValue(idx);
+        const value = arb.generate(mrng, undefined);
+        expect(value.value).toBe(Number.NaN);
+        const shrinks = [...arb.shrink(value.value, value.context)];
+        expect(shrinks).toHaveLength(0);
+      }
+    });
   });
 });
 
 describe('ConstantArbitrary (integration)', () => {
   type Extra = unknown[];
-  // We use a `set` for our parameters
-  // Because when shrinking `ConstantArbitrary` shrinks towards the item [0] that can also be the one it shrunk from
-  // Such case being non-expected* we prefer not to treat it (in such case it shrinks to the same value).
-  // *In usual scenario, we expect `ConstantArbitrary` to only be called with distinct values.
-  //  Any other case may fall into imperfect but working shrinker.
-  const extraParameters: fc.Arbitrary<Extra> = fc.set(fc.anything(), {
-    minLength: 1,
-    // We use `Object.is` as compare function for our parameters
-    // In order to properly deal with -0, 0 and Number.NaN
-    compare: (a, b) => Object.is(a, b),
-  });
+  const extraParameters: fc.Arbitrary<Extra> = fc.array(fc.anything(), { minLength: 1 });
 
   // In other words: extra.includes(value)                   --but with Object.is
   const isCorrect = (value: unknown, extra: Extra) => extra.findIndex((v) => Object.is(value, v)) !== -1;
 
   // In other words: extra.indexOf(v1) < extra.indexOf(v2)   --but with Object.is
+  // If the same value has been declared twice in the `extra` once for `extra[0]` and the other for `extra[n]` (with n > 0)
+  // Shrinker should never shrink `extra[n]` into `extra[0]` if they are equal regarding `Object.is`
   const isStrictlySmaller = (v1: unknown, v2: unknown, extra: Extra) =>
     extra.findIndex((v) => Object.is(v1, v)) < extra.findIndex((v) => Object.is(v2, v));