Skip to content

Commit

Permalink
💥 Error with cause by default (#5590)
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 -->

Until now, fast-check have been merging the content and stack of the
original Error that caused the property to fail into its own Error. With
the recent (2022) introduction of cause on Errors, this complex logic
can be dropped in favor of the native cause mechanism.

This PR makes cause mode the default. Before this PR, toggling it was
possible via `errorWithCause: true` on `fc.assert`.

Given not all test runners properly support causes attached to the
Error, we offer a fallback for users willing to preserve the old
behaviour. It can be toggled via `includeErrorInReport: true` on
`fc.assert`.

Related to #4416.

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

**💥 Breaking change**: The display of errors being reported by
fast-check will change. Many test runners do support that but some still
don't support it so a flag will have to be pass for them to display the
errors correctly.

<!-- [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 10, 2025
1 parent 46ce022 commit c806d57
Show file tree
Hide file tree
Showing 22 changed files with 104 additions and 796 deletions.
5 changes: 5 additions & 0 deletions .changeset/four-pens-happen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"fast-check": major
---

💥 Error with cause by default
14 changes: 8 additions & 6 deletions packages/fast-check/src/check/runner/configuration/Parameters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,15 @@ export interface Parameters<T = void> {
*/
asyncReporter?: (runDetails: RunDetails<T>) => Promise<void>;
/**
* Should the thrown Error include a cause leading to the original Error?
* By default the Error causing the failure of the predicate will not be directly exposed within the message
* of the Error thown by fast-check. It will be exposed by a cause field attached to the Error.
*
* In such case the original Error will disappear from the message of the Error thrown by fast-check
* and only appear within the cause part of it.
* The Error with cause has been supported by Node since 16.14.0 and is properly supported in many test runners.
*
* Remark: At the moment, only node (≥16.14.0) and vitest seem to properly display such errors.
* Others will just discard the cause at display time.
* But if the original Error fails to appear within your test runner,
* Or if you prefer the Error to be included directly as part of the message of the resulted Error,
* you can toggle this flag and the Error produced by fast-check in case of failure will expose the source Error
* as part of the message and not as a cause.
*/
errorWithCause?: boolean;
includeErrorInReport?: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class QualifiedParameters<T> {
ignoreEqualValues: boolean;
reporter: ((runDetails: RunDetails<T>) => void) | null;
asyncReporter: ((runDetails: RunDetails<T>) => Promise<void>) | null;
errorWithCause: boolean;
includeErrorInReport: boolean;

constructor(op?: Parameters<T>) {
const p = op || {};
Expand Down Expand Up @@ -66,7 +66,7 @@ export class QualifiedParameters<T> {
this.endOnFailure = QualifiedParameters.readBoolean(p, 'endOnFailure');
this.reporter = QualifiedParameters.readOrDefault(p, 'reporter', null);
this.asyncReporter = QualifiedParameters.readOrDefault(p, 'asyncReporter', null);
this.errorWithCause = QualifiedParameters.readBoolean(p, 'errorWithCause');
this.includeErrorInReport = QualifiedParameters.readBoolean(p, 'includeErrorInReport');
}

toParameters(): Parameters<T> {
Expand All @@ -90,7 +90,7 @@ export class QualifiedParameters<T> {
endOnFailure: this.endOnFailure,
reporter: orUndefined(this.reporter),
asyncReporter: orUndefined(this.asyncReporter),
errorWithCause: this.errorWithCause,
includeErrorInReport: this.includeErrorInReport,
};
return parameters;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,10 @@ function prettyError(errorInstance: unknown) {

/** @internal */
function preFormatFailure<Ts>(out: RunDetailsFailureProperty<Ts>, stringifyOne: (value: Ts) => string) {
const noErrorInMessage = out.runConfiguration.errorWithCause;
const messageErrorPart = noErrorInMessage
? ''
: `\nGot ${safeReplace(prettyError(out.errorInstance), /^Error: /, 'error: ')}`;
const includeErrorInReport = out.runConfiguration.includeErrorInReport;
const messageErrorPart = includeErrorInReport
? `\nGot ${safeReplace(prettyError(out.errorInstance), /^Error: /, 'error: ')}`
: '';
const message = `Property failed after ${out.numRuns} tests\n{ seed: ${out.seed}, path: "${
out.counterexamplePath
}", endOnFailure: true }\nCounterexample: ${stringifyOne(out.counterexample)}\nShrunk ${
Expand Down Expand Up @@ -288,7 +288,7 @@ async function asyncDefaultReportMessage<Ts>(out: RunDetails<Ts>): Promise<strin

/** @internal */
function buildError<Ts>(errorMessage: string | undefined, out: RunDetails<Ts> & { failed: true }) {
if (!out.runConfiguration.errorWithCause) {
if (out.runConfiguration.includeErrorInReport) {
throw new Error(errorMessage);
}
const ErrorWithCause: new (message: string | undefined, options: { cause: unknown }) => Error = Error;
Expand Down
12 changes: 6 additions & 6 deletions packages/fast-check/test/e2e/NoRegressionStack.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ describe(`NoRegressionStack`, () => {
fc.property(fc.nat(), fc.nat(), (a, b) => {
return a >= b;
}),
settings,
{ ...settings, includeErrorInReport: true },
),
),
).toThrowErrorMatchingSnapshot();
Expand All @@ -25,7 +25,7 @@ describe(`NoRegressionStack`, () => {
fc.property(fc.nat(), fc.nat(), (a, b) => {
return a >= b;
}),
{ ...settings, errorWithCause: true },
settings,
),
),
).toThrowErrorMatchingSnapshot();
Expand All @@ -39,7 +39,7 @@ describe(`NoRegressionStack`, () => {
throw new Error('a must be >= b');
}
}),
settings,
{ ...settings, includeErrorInReport: true },
),
),
).toThrowErrorMatchingSnapshot();
Expand All @@ -54,7 +54,7 @@ describe(`NoRegressionStack`, () => {
throw new Error('a must be >= b');
}
}),
{ ...settings, errorWithCause: true },
settings,
),
),
).toThrowErrorMatchingSnapshot();
Expand All @@ -67,7 +67,7 @@ describe(`NoRegressionStack`, () => {
fc.property(fc.nat(), (v) => {
(v as any)();
}),
settings,
{ ...settings, includeErrorInReport: true },
),
),
).toThrowErrorMatchingSnapshot();
Expand All @@ -80,7 +80,7 @@ describe(`NoRegressionStack`, () => {
fc.property(fc.nat(), (v) => {
(v as any)();
}),
{ ...settings, errorWithCause: true },
settings,
),
),
).toThrowErrorMatchingSnapshot();
Expand Down
Loading

0 comments on commit c806d57

Please sign in to comment.