Repository: dubzzz/fast-check
Pull request #1693 (2021-04-05 14:43:56)

🐛 Apply cloneMethod on map/filter instances of Shrinkable
<!-- 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

Potential impacts on:
- Generated values impact,
- Shrink values impact,

Only if one of the generators produces cloneable values.

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

Latest deployment of this branch, based on commit f700b80672a5f4d46ed00829ce5d60171afb6f1e:

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

codecov[bot]: # [Codecov](https://codecov.io/gh/dubzzz/fast-check/pull/1693?src=pr&el=h1) Report
> Merging [#1693](https://codecov.io/gh/dubzzz/fast-check/pull/1693?src=pr&el=desc) (f700b80) into [main](https://codecov.io/gh/dubzzz/fast-check/commit/e72376e4c1df74b27004a3ee302431629cce680e?el=desc) (e72376e) will **not change** coverage.
> The diff coverage is `100.00%`.

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

```diff
@@           Coverage Diff           @@
##             main    #1693   +/-   ##
=======================================
  Coverage   97.01%   97.01%           
=======================================
  Files          98       98           
  Lines        3488     3488           
  Branches      681      681           
=======================================
  Hits         3384     3384           
  Misses        100      100           
  Partials        4        4           
```

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

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/1693?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/check/arbitrary/definition/Shrinkable.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1693/diff?src=pr&el=tree#diff-c3JjL2NoZWNrL2FyYml0cmFyeS9kZWZpbml0aW9uL1Nocmlua2FibGUudHM=) | `100.00% <100.00%> (ø)` | |

------

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


Patch:
diff --git a/src/check/arbitrary/definition/Shrinkable.ts b/src/check/arbitrary/definition/Shrinkable.ts
index e85bbd8e..33a4947f 100644
--- a/src/check/arbitrary/definition/Shrinkable.ts
+++ b/src/check/arbitrary/definition/Shrinkable.ts
@@ -72,7 +72,7 @@ export class Shrinkable<T, TShrink extends T = T> {
     if (this.hasToBeCloned) {
       const out = mapper(this.value);
       if (out instanceof Object) {
-        (out as any)[cloneMethod] = () => mapper(this.value);
+        (out as any)[cloneMethod] = () => this.applyMapper(mapper);
       }
       return out;
     }
@@ -121,12 +121,12 @@ export class Shrinkable<T, TShrink extends T = T> {
   filter(predicate: (t: TShrink) => boolean): Shrinkable<T, TShrink>;
   filter<U extends TShrink>(refinement: (t: TShrink) => t is U): Shrinkable<T, U> {
     const refinementOnShrinkable = (s: Shrinkable<TShrink, TShrink>): s is Shrinkable<U, U> => {
-      return refinement(s.value);
+      return refinement(s.value_);
     };
     return new Shrinkable<T, U>(this.value, () =>
       this.shrink()
         .filter(refinementOnShrinkable)
-        .map((v) => v.filter(refinement))
+        .map((v) => v.filter(refinement) /* touch .value */)
     );
   }
 }


Test patch:
diff --git a/test/unit/check/arbitrary/definition/Shrinkable.spec.ts b/test/unit/check/arbitrary/definition/Shrinkable.spec.ts
index 46d190ca..f9e0f06d 100644
--- a/test/unit/check/arbitrary/definition/Shrinkable.spec.ts
+++ b/test/unit/check/arbitrary/definition/Shrinkable.spec.ts
@@ -1,5 +1,6 @@
 import { Shrinkable } from '../../../../../src/check/arbitrary/definition/Shrinkable';
-import { cloneMethod } from '../../../../../src/check/symbols';
+import { cloneMethod, hasCloneMethod } from '../../../../../src/check/symbols';
+import { Stream } from '../../../../../src/stream/Stream';
 
 describe('Shrinkable', () => {
   it('Should detect absence of [cloneMethod] method', () => {
@@ -51,4 +52,73 @@ describe('Shrinkable', () => {
     expect(s.value_).toBe(cloneable);
     expect(numCalls).toEqual(0);
   });
+  it('Should produce mapped values with [cloneMethod] if value was cloneable', () => {
+    const cloneMethodImplem = jest.fn();
+    const cloneable = { [cloneMethod]: cloneMethodImplem };
+    const s = new Shrinkable(cloneable);
+    const mapped = s.map(() => []);
+    expect(mapped.hasToBeCloned).toBe(true);
+    expect(hasCloneMethod(mapped.value_)).toBe(true);
+    expect(cloneMethodImplem).not.toHaveBeenCalled();
+  });
+  it('Should produce mapped values with [cloneMethod] returning a clone with [cloneMethod] too', () => {
+    const cloneMethodImplem = jest.fn();
+    const cloneable = { [cloneMethod]: cloneMethodImplem };
+    const s = new Shrinkable(cloneable);
+    const mapped = s.map(() => []);
+    const access1 = mapped.value;
+    const access2 = mapped.value;
+    const access3 = (access1 as any)[cloneMethod]();
+    expect(access2).not.toBe(access1);
+    expect(access3).not.toBe(access1);
+    expect(hasCloneMethod(access1)).toBe(true);
+    expect(hasCloneMethod(access2)).toBe(true);
+    expect(hasCloneMethod(access3)).toBe(true);
+    expect(cloneMethodImplem).toHaveBeenCalledTimes(2);
+  });
+  it('Should produce shrunk mapped values with [cloneMethod] if value was cloneable', () => {
+    const cloneMethodImplem = jest.fn();
+    const cloneable = { [cloneMethod]: cloneMethodImplem };
+    const s = new Shrinkable(cloneable, () => Stream.of(new Shrinkable(cloneable)));
+    const mapped = s.map(() => []);
+    const shrunkMapped = mapped.shrink().getNthOrLast(0)!;
+    expect(shrunkMapped.hasToBeCloned).toBe(true);
+    expect(hasCloneMethod(shrunkMapped.value_)).toBe(true);
+    expect(cloneMethodImplem).not.toHaveBeenCalled();
+  });
+  it('Should produce filtered values with [cloneMethod] if value was cloneable', () => {
+    const cloneMethodImplem = jest.fn();
+    const cloneable = { [cloneMethod]: cloneMethodImplem };
+    const s = new Shrinkable(cloneable);
+    const filtered = s.filter(() => true);
+    expect(filtered.hasToBeCloned).toBe(true);
+    expect(hasCloneMethod(filtered.value_)).toBe(true);
+    expect(cloneMethodImplem).not.toHaveBeenCalled();
+  });
+  it('Should produce filtered values with [cloneMethod] returning a clone with [cloneMethod] too', () => {
+    const cloneMethodImplem = jest.fn();
+    const cloneable = { [cloneMethod]: cloneMethodImplem };
+    cloneMethodImplem.mockReturnValue({ [cloneMethod]: cloneMethodImplem });
+    const s = new Shrinkable(cloneable);
+    const filtered = s.filter(() => true);
+    const access1 = filtered.value;
+    const access2 = filtered.value;
+    const access3 = (access1 as any)[cloneMethod]();
+    expect(access2).not.toBe(access1);
+    expect(access3).not.toBe(access1);
+    expect(hasCloneMethod(access1)).toBe(true);
+    expect(hasCloneMethod(access2)).toBe(true);
+    expect(hasCloneMethod(access3)).toBe(true);
+    expect(cloneMethodImplem).toHaveBeenCalledTimes(2);
+  });
+  it('Should produce shrunk filtered values with [cloneMethod] if value was cloneable', () => {
+    const cloneMethodImplem = jest.fn();
+    const cloneable = { [cloneMethod]: cloneMethodImplem };
+    const s = new Shrinkable(cloneable, () => Stream.of(new Shrinkable(cloneable)));
+    const mapped = s.filter(() => true);
+    const shrunkFiltered = mapped.shrink().getNthOrLast(0)!;
+    expect(shrunkFiltered.hasToBeCloned).toBe(true);
+    expect(hasCloneMethod(shrunkFiltered.value_)).toBe(true);
+    expect(cloneMethodImplem).not.toHaveBeenCalled();
+  });
 });