Skip to content

Commit

Permalink
✨ Add ability to limit shrink path (#5006)
Browse files Browse the repository at this point in the history
**Description**

We want to offer a way for our users to limit the depth and overall size
of the shrinking capabilities. At the moment, they can only either apply
shrink or drop it totally. We want to offer a finer way to control the
shrinks.

This proposal is related to #4162.

At the moment, we added an extra method for all our arbitrary but I feel
that it will not be scalable. I believe we should go for yet another
utility arbitrary that will be responsible to offer such capabilities.
Regarding offering these capabilities at assert level itself, we should
probably (option 1) do as we used to do for timeouts..., or (option 2)
start thinking into some kind of plugin system. Option 1 seems to be a
valid option for version 3 of fast-check, we could also start offering a
noShrink utility either hidden behind limitShrink one (probably better)
or directly.

**NOTE:** Not ready yet, we should move from methods to a dedicated
utility.

<!-- Please provide a short description and potentially linked issues
hustifying the need for this PR -->

<!-- * Your PR is fixing a bug or regression? Check for existing issues
related to this bug and link them -->
<!-- * Your PR is adding a new feature? Make sure there is a related
issue or discussion attached to it -->

<!-- You can provide any additional context to help into understanding
what's this PR is attempting to solve: reproduction of a bug, code
snippets... -->

**Checklist** — _Don't delete this checklist and make sure you do the
following before opening the PR_

- [x] The name of my PR follows [gitmoji](https://gitmoji.dev/)
specification
- [x] My PR references one of several related issues (if any)
- [x] New features or breaking changes must come with an associated
Issue or Discussion
- [x] My PR does not add any new dependency without an associated Issue
or Discussion
- [x] My PR includes bumps details, please run `yarn bump` and flag the
impacts properly
- [x] My PR adds relevant tests and they would have failed without my PR
(when applicable)

<!-- More about contributing at
https://github.com/dubzzz/fast-check/blob/main/CONTRIBUTING.md -->

**Advanced**

<!-- How to fill the advanced section is detailed below! -->

- [x] Category: ✨ Introduce new features
- [x] Impacts: New arbitrary

<!-- [Category] Please use one of the categories below, it will help us
into better understanding the urgency of the PR -->
<!-- * ✨ Introduce new features -->
<!-- * 📝 Add or update documentation -->
<!-- * ✅ Add or update tests -->
<!-- * 🐛 Fix a bug -->
<!-- * 🏷️ Add or update types -->
<!-- * ⚡️ Improve performance -->
<!-- * _Other(s):_ ... -->

<!-- [Impacts] Please provide a comma separated list of the potential
impacts that might be introduced by this change -->
<!-- * Generated values: Can your change impact any of the existing
generators in terms of generated values, if so which ones? when? -->
<!-- * Shrink values: Can your change impact any of the existing
generators in terms of shrink values, if so which ones? when? -->
<!-- * Performance: Can it require some typings changes on user side?
Please give more details -->
<!-- * Typings: Is there a potential performance impact? In which cases?
-->
  • Loading branch information
dubzzz authored Jul 5, 2024
1 parent cd5df93 commit 56a3468
Show file tree
Hide file tree
Showing 12 changed files with 565 additions and 0 deletions.
8 changes: 8 additions & 0 deletions .yarn/versions/b4e668ef.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
releases:
fast-check: minor

declined:
- "@fast-check/ava"
- "@fast-check/jest"
- "@fast-check/vitest"
- "@fast-check/worker"
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { Arbitrary } from '../../check/arbitrary/definition/Arbitrary';
import { Value } from '../../check/arbitrary/definition/Value';
import type { Random } from '../../random/generator/Random';
import { Stream } from '../../stream/Stream';
import { zipIterableIterators } from './helpers/ZipIterableIterators';

/** @internal */
function* iotaFrom(startValue: number) {
let value = startValue;
while (true) {
yield value;
++value;
}
}

/** @internal */
export class LimitedShrinkArbitrary<T> extends Arbitrary<T> {
constructor(
readonly arb: Arbitrary<T>,
readonly maxShrinks: number,
) {
super();
}
generate(mrng: Random, biasFactor: number | undefined): Value<T> {
const value = this.arb.generate(mrng, biasFactor);
return this.valueMapper(value, 0);
}
canShrinkWithoutContext(value: unknown): value is T {
return this.arb.canShrinkWithoutContext(value);
}
shrink(value: T, context?: unknown): Stream<Value<T>> {
if (this.isSafeContext(context)) {
return this.safeShrink(value, context.originalContext, context.length);
}
return this.safeShrink(value, undefined, 0);
}
private safeShrink(value: T, originalContext: unknown, currentLength: number): Stream<Value<T>> {
const remaining = this.maxShrinks - currentLength;
if (remaining <= 0) {
return Stream.nil(); // early-exit to avoid potentially expensive computations in .shrink
}
return new Stream(zipIterableIterators(this.arb.shrink(value, originalContext), iotaFrom(currentLength + 1)))
.take(remaining)
.map((valueAndLength) => this.valueMapper(valueAndLength[0], valueAndLength[1]));
}
private valueMapper(v: Value<T>, newLength: number): Value<T> {
const context: LimitedShrinkArbitraryContext = { originalContext: v.context, length: newLength };
return new Value(v.value, context);
}
private isSafeContext(context: unknown): context is LimitedShrinkArbitraryContext {
return (
context != null &&
typeof context === 'object' &&
'originalContext' in (context as any) &&
'length' in (context as any)
);
}
}

/** @internal */
type LimitedShrinkArbitraryContext = {
originalContext: unknown;
length: number;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/** @internal */
function initZippedValues<ITs extends IterableIterator<unknown>[]>(its: ITs) {
const vs: IteratorResult<unknown, any>[] = [];
for (let index = 0; index !== its.length; ++index) {
vs.push(its[index].next());
}
return vs;
}

/** @internal */
function nextZippedValues<ITs extends IterableIterator<unknown>[]>(its: ITs, vs: IteratorResult<unknown, any>[]) {
for (let index = 0; index !== its.length; ++index) {
vs[index] = its[index].next();
}
}

/** @internal */
function isDoneZippedValues(vs: IteratorResult<unknown, any>[]): boolean {
for (let index = 0; index !== vs.length; ++index) {
if (vs[index].done) {
return true;
}
}
return false;
}
/** @internal */
export function* zipIterableIterators<ITs extends IterableIterator<unknown>[]>(
...its: ITs
): ZippedIterableIterator<ITs> {
const vs = initZippedValues(its);
while (!isDoneZippedValues(vs)) {
yield vs.map((v) => v.value) as unknown as ZippedIterableIteratorValues<ITs>;
nextZippedValues(its, vs);
}
}

/** @internal */
type ZippedIterableIteratorValues<ITs extends IterableIterator<unknown>[]> = {
[K in keyof ITs]: ITs[K] extends IterableIterator<infer IT> ? IT : unknown;
};

/** @internal */
type ZippedIterableIterator<ITs extends IterableIterator<unknown>[]> = IterableIterator<
ZippedIterableIteratorValues<ITs>
>;
36 changes: 36 additions & 0 deletions packages/fast-check/src/arbitrary/limitShrink.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import type { Arbitrary } from '../check/arbitrary/definition/Arbitrary';
import { LimitedShrinkArbitrary } from './_internals/LimitedShrinkArbitrary';

/**
* Constraints to be applied on {@link limitShrink}
* @remarks Since 3.20.0
* @public
*/
export type LimitShrinkConstraints = {
/**
* Define the maximal number of shrink values that can be pulled from this arbitrary
* @remarks Since 3.20.0
*/
maxShrinks: number;
};

/**
* Create another Arbitrary with a limited (or capped) number of shrink values
*
* @example
* ```typescript
* const dataGenerator: Arbitrary<string> = ...;
* const limitedShrinkableDataGenerator: Arbitrary<string> = fc.limitShrink(dataGenerator, { maxShrinks: 10 });
* // up to 10 shrunk values could be extracted from the resulting arbitrary
* ```
*
* NOTE: Although limiting the shrinking capabilities can speed up your CI when failures occur, we do not recommend this approach.
* Instead, if you want to reduce the shrinking time for automated jobs or local runs, consider using `endOnFailure` or `interruptAfterTimeLimit`.
*
* @returns Create another arbitrary with limited number of shrink values
* @remarks Since 3.20.0
* @public
*/
export function limitShrink<T>(arbitrary: Arbitrary<T>, constraints: LimitShrinkConstraints): Arbitrary<T> {
return new LimitedShrinkArbitrary(arbitrary, constraints.maxShrinks);
}
4 changes: 4 additions & 0 deletions packages/fast-check/src/fast-check-default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,8 @@ import type { StringMatchingConstraints } from './arbitrary/stringMatching';
import { stringMatching } from './arbitrary/stringMatching';
import { noShrink } from './arbitrary/noShrink';
import { noBias } from './arbitrary/noBias';
import { limitShrink } from './arbitrary/limitShrink';
import type { LimitShrinkConstraints } from './arbitrary/limitShrink';

// Explicit cast into string to avoid to have __type: "__PACKAGE_TYPE__"
/**
Expand Down Expand Up @@ -281,6 +283,7 @@ export type {
IntegerConstraints,
JsonSharedConstraints,
UnicodeJsonSharedConstraints,
LimitShrinkConstraints,
LoremConstraints,
MixedCaseConstraints,
NatConstraints,
Expand Down Expand Up @@ -378,6 +381,7 @@ export {
hexaString,
base64String,
stringMatching,
limitShrink,
lorem,
constant,
constantFrom,
Expand Down
10 changes: 10 additions & 0 deletions packages/fast-check/test/e2e/NoRegression.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,16 @@ describe(`NoRegression`, () => {
),
).toThrowErrorMatchingSnapshot();
});
it('limitShrink', () => {
expect(
runWithSanitizedStack(() =>
fc.assert(
fc.property(fc.limitShrink(fc.nat(), { maxShrinks: 4 }), (v) => testFunc(v)),
settings,
),
),
).toThrowErrorMatchingSnapshot();
});
it('int8Array', () => {
expect(
runWithSanitizedStack(() =>
Expand Down
19 changes: 19 additions & 0 deletions packages/fast-check/test/e2e/StateFullArbitraries.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ describe(`StateFullArbitraries (seed: ${seed})`, () => {
fc.assert(fc.property(fc.array(cloneableWithCount(data)), () => {}));
expect(data.counter).toEqual(0);
});
it('fc.limitShrink', () => {
const data = { counter: 0 };
fc.assert(fc.property(fc.limitShrink(cloneableWithCount(data), { maxShrinks: 10 }), () => {}));
expect(data.counter).toEqual(0);
});
});
describe('Never call with non-cloned instance and correct counterexample', () => {
it('normal property', () => {
Expand Down Expand Up @@ -230,5 +235,19 @@ describe(`StateFullArbitraries (seed: ${seed})`, () => {
expect(nonClonedDetected).toBe(false);
expect(alwaysWithElements).toBe(true);
});
it('fc.limitShrink', () => {
let nonClonedDetected = false;
const status = fc.check(
fc.property(fc.integer(), fc.limitShrink(fc.context(), { maxShrinks: 10 }), fc.integer(), (a, ctx, b) => {
nonClonedDetected = nonClonedDetected || ctx.size() !== 0;
ctx.log('logging stuff');
return a < b;
}),
{ seed },
);
expect(status.failed).toBe(true);
expect(nonClonedDetected).toBe(false);
expect(status.counterexample![1].size()).toEqual(1);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3051,6 +3051,22 @@ Execution summary:
. . . . . . . . √ [{"left":{"left":0,"right":{"left":0,"right":{"left":0,"right":0}}},"right":10}]]
`;

exports[`NoRegression > limitShrink 1`] = `
[Error: Property failed after 2 tests
{ seed: 42, path: "1:2", endOnFailure: true }
Counterexample: [864744596]
Shrunk 1 time(s)
Got error: Property failed by returning false

Execution summary:
√ [2147483626]
× [1152992794]
. √ [0]
. √ [576496397]
. × [864744596]
. . √ [720620497]]
`;

exports[`NoRegression > lorem 1`] = `
[Error: Property failed after 2 tests
{ seed: 42, path: "1:1:0:0:0", endOnFailure: true }
Expand Down
Loading

0 comments on commit 56a3468

Please sign in to comment.