Skip to content

Commit

Permalink
🔥 Enforce run{Before/After}Each on property (#5581)
Browse files Browse the repository at this point in the history
**Description**

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

The API of property has been updated in a non-breaking manner during v3.
Now, let's seal that move to drop unneeded code complexity, drop useless
tests... but also make the API clearer and way simpler to use and
manage.

The implementations of `IRawProperty` now have to implement
`runBeforeEach` and `runAfterEach`. If they themselves wrap another
`IRawProperty`, they probably should at least call these methods when
called on their side of them. The migration from v3, consists into
implementing the two methods.

<!-- * 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! -->

**:boom: Breaking change**: Change the execution flow of properties.
Only impacts on advanced users building their own custom runners or
their own property type.

<!-- [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 Jan 8, 2025
1 parent bb46b92 commit ee62923
Show file tree
Hide file tree
Showing 21 changed files with 359 additions and 557 deletions.
5 changes: 5 additions & 0 deletions .changeset/wise-pears-lay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"fast-check": major
---

🔥 Enforce `run{Before/After}Each` on property
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,7 @@ export class AsyncProperty<Ts> implements IAsyncPropertyWithHooks<Ts> {
await this.afterEachHook();
}

async run(v: Ts, dontRunHook?: boolean): Promise<PreconditionFailure | PropertyFailure | null> {
if (!dontRunHook) {
await this.beforeEachHook();
}
async run(v: Ts): Promise<PreconditionFailure | PropertyFailure | null> {
try {
const output = await this.predicate(v);
return output == null || output === true
Expand All @@ -135,10 +132,6 @@ export class AsyncProperty<Ts> implements IAsyncPropertyWithHooks<Ts> {
return { error: err, errorMessage: err.stack }; // stack includes the message
}
return { error: err, errorMessage: String(err) };
} finally {
if (!dontRunHook) {
await this.afterEachHook();
}
}
}

Expand Down
6 changes: 2 additions & 4 deletions packages/fast-check/src/check/property/IRawProperty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,10 @@ export interface IRawProperty<Ts, IsAsync extends boolean = boolean> {
/**
* Check the predicate for v
* @param v - Value of which we want to check the predicate
* @param dontRunHook - Do not run beforeEach and afterEach hooks within run
* @remarks Since 0.0.7
*/
run(
v: Ts,
dontRunHook?: boolean,
):
| (IsAsync extends true ? Promise<PreconditionFailure | PropertyFailure | null> : never)
| (IsAsync extends false ? PreconditionFailure | PropertyFailure | null : never);
Expand All @@ -79,13 +77,13 @@ export interface IRawProperty<Ts, IsAsync extends boolean = boolean> {
* Run before each hook
* @remarks Since 3.4.0
*/
runBeforeEach?: () => (IsAsync extends true ? Promise<void> : never) | (IsAsync extends false ? void : never);
runBeforeEach: () => (IsAsync extends true ? Promise<void> : never) | (IsAsync extends false ? void : never);

/**
* Run after each hook
* @remarks Since 3.4.0
*/
runAfterEach?: () => (IsAsync extends true ? Promise<void> : never) | (IsAsync extends false ? void : never);
runAfterEach: () => (IsAsync extends true ? Promise<void> : never) | (IsAsync extends false ? void : never);
}

/**
Expand Down
23 changes: 11 additions & 12 deletions packages/fast-check/src/check/property/IgnoreEqualValuesProperty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,12 @@ function fromCachedUnsafe<Ts, IsAsync extends boolean>(

/** @internal */
export class IgnoreEqualValuesProperty<Ts, IsAsync extends boolean> implements IRawProperty<Ts, IsAsync> {
runBeforeEach?: () => (IsAsync extends true ? Promise<void> : never) | (IsAsync extends false ? void : never);
runAfterEach?: () => (IsAsync extends true ? Promise<void> : never) | (IsAsync extends false ? void : never);
private coveredCases: Map<string, ReturnType<IRawProperty<Ts, IsAsync>['run']>> = new Map();

constructor(
readonly property: IRawProperty<Ts, IsAsync>,
readonly skipRuns: boolean,
) {
if (this.property.runBeforeEach !== undefined && this.property.runAfterEach !== undefined) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this.runBeforeEach = () => this.property.runBeforeEach!();
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this.runAfterEach = () => this.property.runAfterEach!();
}
}
) {}

isAsync(): IsAsync {
return this.property.isAsync();
Expand All @@ -67,7 +58,7 @@ export class IgnoreEqualValuesProperty<Ts, IsAsync extends boolean> implements I
return this.property.shrink(value);
}

run(v: Ts, dontRunHook: boolean): ReturnType<IRawProperty<Ts, IsAsync>['run']> {
run(v: Ts): ReturnType<IRawProperty<Ts, IsAsync>['run']> {
const stringifiedValue = stringify(v);
if (this.coveredCases.has(stringifiedValue)) {
const lastOutput = this.coveredCases.get(stringifiedValue) as ReturnType<IRawProperty<Ts, IsAsync>['run']>;
Expand All @@ -76,8 +67,16 @@ export class IgnoreEqualValuesProperty<Ts, IsAsync extends boolean> implements I
}
return fromCachedUnsafe(lastOutput, this.property.isAsync());
}
const out = this.property.run(v, dontRunHook);
const out = this.property.run(v);
this.coveredCases.set(stringifiedValue, out);
return out;
}

runBeforeEach(): ReturnType<IRawProperty<Ts, IsAsync>['runBeforeEach']> {
return this.property.runBeforeEach();
}

runAfterEach(): ReturnType<IRawProperty<Ts, IsAsync>['runAfterEach']> {
return this.property.runAfterEach();
}
}
9 changes: 1 addition & 8 deletions packages/fast-check/src/check/property/Property.generic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,7 @@ export class Property<Ts> implements IProperty<Ts>, IPropertyWithHooks<Ts> {
this.afterEachHook();
}

run(v: Ts, dontRunHook?: boolean): PreconditionFailure | PropertyFailure | null {
if (!dontRunHook) {
this.beforeEachHook();
}
run(v: Ts): PreconditionFailure | PropertyFailure | null {
try {
const output = this.predicate(v);
return output == null || output === true
Expand All @@ -151,10 +148,6 @@ export class Property<Ts> implements IProperty<Ts>, IPropertyWithHooks<Ts> {
return { error: err, errorMessage: err.stack }; // stack includes the message
}
return { error: err, errorMessage: String(err) };
} finally {
if (!dontRunHook) {
this.afterEachHook();
}
}
}

Expand Down
22 changes: 11 additions & 11 deletions packages/fast-check/src/check/property/SkipAfterProperty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ function interruptAfter(timeMs: number, setTimeoutSafe: typeof setTimeout, clear

/** @internal */
export class SkipAfterProperty<Ts, IsAsync extends boolean> implements IRawProperty<Ts, IsAsync> {
runBeforeEach?: () => (IsAsync extends true ? Promise<void> : never) | (IsAsync extends false ? void : never);
runAfterEach?: () => (IsAsync extends true ? Promise<void> : never) | (IsAsync extends false ? void : never);
private skipAfterTime: number;

constructor(
Expand All @@ -36,12 +34,6 @@ export class SkipAfterProperty<Ts, IsAsync extends boolean> implements IRawPrope
readonly clearTimeoutSafe: typeof clearTimeout,
) {
this.skipAfterTime = this.getTime() + timeLimit;
if (this.property.runBeforeEach !== undefined && this.property.runAfterEach !== undefined) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this.runBeforeEach = () => this.property.runBeforeEach!();
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this.runAfterEach = () => this.property.runAfterEach!();
}
}

isAsync(): IsAsync {
Expand All @@ -56,7 +48,7 @@ export class SkipAfterProperty<Ts, IsAsync extends boolean> implements IRawPrope
return this.property.shrink(value);
}

run(v: Ts, dontRunHook: boolean): ReturnType<IRawProperty<Ts, IsAsync>['run']> {
run(v: Ts): ReturnType<IRawProperty<Ts, IsAsync>['run']> {
const remainingTime = this.skipAfterTime - this.getTime();
if (remainingTime <= 0) {
const preconditionFailure = new PreconditionFailure(this.interruptExecution);
Expand All @@ -68,10 +60,18 @@ export class SkipAfterProperty<Ts, IsAsync extends boolean> implements IRawPrope
}
if (this.interruptExecution && this.isAsync()) {
const t = interruptAfter(remainingTime, this.setTimeoutSafe, this.clearTimeoutSafe);
const propRun = Promise.race([this.property.run(v, dontRunHook), t.promise]);
const propRun = Promise.race([this.property.run(v), t.promise]);
propRun.then(t.clear, t.clear); // always clear timeout handle - catch should never occur
return propRun as any; // IsAsync => Promise<PreconditionFailure | string | null>
}
return this.property.run(v, dontRunHook);
return this.property.run(v);
}

runBeforeEach(): ReturnType<IRawProperty<Ts, IsAsync>['runBeforeEach']> {
return this.property.runBeforeEach();
}

runAfterEach(): ReturnType<IRawProperty<Ts, IsAsync>['runAfterEach']> {
return this.property.runAfterEach();
}
}
24 changes: 11 additions & 13 deletions packages/fast-check/src/check/property/TimeoutProperty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,12 @@ const timeoutAfter = (timeMs: number, setTimeoutSafe: typeof setTimeout, clearTi

/** @internal */
export class TimeoutProperty<Ts> implements IRawProperty<Ts, true> {
runBeforeEach?: () => Promise<void>;
runAfterEach?: () => Promise<void>;

constructor(
readonly property: IRawProperty<Ts>,
readonly timeMs: number,
readonly setTimeoutSafe: typeof setTimeout,
readonly clearTimeoutSafe: typeof clearTimeout,
) {
if (this.property.runBeforeEach !== undefined && this.property.runAfterEach !== undefined) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this.runBeforeEach = () => Promise.resolve(this.property.runBeforeEach!());
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this.runAfterEach = () => Promise.resolve(this.property.runAfterEach!());
}
}
) {}

isAsync(): true {
return true;
Expand All @@ -55,10 +45,18 @@ export class TimeoutProperty<Ts> implements IRawProperty<Ts, true> {
return this.property.shrink(value);
}

async run(v: Ts, dontRunHook: boolean): Promise<PreconditionFailure | PropertyFailure | null> {
async run(v: Ts): Promise<PreconditionFailure | PropertyFailure | null> {
const t = timeoutAfter(this.timeMs, this.setTimeoutSafe, this.clearTimeoutSafe);
const propRun = Promise.race([this.property.run(v, dontRunHook), t.promise]);
const propRun = Promise.race([this.property.run(v), t.promise]);
propRun.then(t.clear, t.clear); // always clear timeout handle - catch should never occur
return propRun;
}

runBeforeEach(): ReturnType<IRawProperty<Ts, true>['runBeforeEach']> {
return Promise.resolve(this.property.runBeforeEach());
}

runAfterEach(): ReturnType<IRawProperty<Ts, true>['runAfterEach']> {
return Promise.resolve(this.property.runAfterEach());
}
}
24 changes: 11 additions & 13 deletions packages/fast-check/src/check/property/UnbiasedProperty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,7 @@ import type { IRawProperty } from './IRawProperty';

/** @internal */
export class UnbiasedProperty<Ts, IsAsync extends boolean> implements IRawProperty<Ts, IsAsync> {
runBeforeEach?: () => (IsAsync extends true ? Promise<void> : never) | (IsAsync extends false ? void : never);
runAfterEach?: () => (IsAsync extends true ? Promise<void> : never) | (IsAsync extends false ? void : never);

constructor(readonly property: IRawProperty<Ts, IsAsync>) {
if (this.property.runBeforeEach !== undefined && this.property.runAfterEach !== undefined) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this.runBeforeEach = () => this.property.runBeforeEach!();
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this.runAfterEach = () => this.property.runAfterEach!();
}
}
constructor(readonly property: IRawProperty<Ts, IsAsync>) {}

isAsync(): IsAsync {
return this.property.isAsync();
Expand All @@ -29,7 +19,15 @@ export class UnbiasedProperty<Ts, IsAsync extends boolean> implements IRawProper
return this.property.shrink(value);
}

run(v: Ts, dontRunHook: boolean): ReturnType<IRawProperty<Ts, IsAsync>['run']> {
return this.property.run(v, dontRunHook);
run(v: Ts): ReturnType<IRawProperty<Ts, IsAsync>['run']> {
return this.property.run(v);
}

runBeforeEach(): ReturnType<IRawProperty<Ts, IsAsync>['runBeforeEach']> {
return this.property.runBeforeEach();
}

runAfterEach(): ReturnType<IRawProperty<Ts, IsAsync>['runAfterEach']> {
return this.property.runAfterEach();
}
}
24 changes: 6 additions & 18 deletions packages/fast-check/src/check/runner/Runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,11 @@ function runIt<Ts>(
verbose: VerbosityLevel,
interruptedAsFailure: boolean,
): RunExecution<Ts> {
const isModernProperty = property.runBeforeEach !== undefined && property.runAfterEach !== undefined;
const runner = new RunnerIterator(sourceValues, shrink, verbose, interruptedAsFailure);
for (const v of runner) {
if (isModernProperty) {
(property.runBeforeEach as () => void)();
}
const out = property.run(v, isModernProperty) as PreconditionFailure | PropertyFailure | null;
if (isModernProperty) {
(property.runAfterEach as () => void)();
}
(property.runBeforeEach as () => void)();
const out = property.run(v) as PreconditionFailure | PropertyFailure | null;
(property.runAfterEach as () => void)();
runner.handleResult(out);
}
return runner.runExecution;
Expand All @@ -50,18 +45,11 @@ async function asyncRunIt<Ts>(
verbose: VerbosityLevel,
interruptedAsFailure: boolean,
): Promise<RunExecution<Ts>> {
const isModernProperty = property.runBeforeEach !== undefined && property.runAfterEach !== undefined;
const runner = new RunnerIterator(sourceValues, shrink, verbose, interruptedAsFailure);
for (const v of runner) {
if (isModernProperty) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await property.runBeforeEach!();
}
const out = await property.run(v, isModernProperty);
if (isModernProperty) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await property.runAfterEach!();
}
await property.runBeforeEach();
const out = await property.run(v);
await property.runAfterEach();
runner.handleResult(out);
}
return runner.runExecution;
Expand Down
Loading

0 comments on commit ee62923

Please sign in to comment.