✨ Stop repeating the error twice in reports (#3638)

* ✨ Stop repeating the error twice in reports While the change could have been considered as a bug fix, I prefer including it as a fresh new feature to avoid risks of breaking people relying on this bug. Fixes #3634 * Drop Error: from report * versions * unfify stacks cross os * safer poisoned * format * Update packages/fast-check/test/e2e/NoRegressionStack.spec.ts * Update packages/fast-check/test/e2e/__snapshots__/NoRegressionStack.spec.ts.snap * Update packages/fast-check/test/e2e/__snapshots__/NoRegressionStack.spec.ts.snap * Update packages/fast-check/test/e2e/__snapshots__/NoRegressionStack.spec.ts.snap * Update packages/fast-check/test/e2e/__snapshots__/NoRegressionStack.spec.ts.snap * Update packages/fast-check/test/e2e/__snapshots__/NoRegressionStack.spec.ts.snap * fix no reg * fix unit
dubzzz · Feb 23, 2023 · df98c64 · df98c64
1 parent c2e609d
commit df98c64
Show file tree

Hide file tree

Showing 10 changed files with 133 additions and 9 deletions.
diff --git a/.yarn/versions/8d95bfec.yml b/.yarn/versions/8d95bfec.yml
@@ -0,0 +1,8 @@
+releases:
+  fast-check: minor
+
+declined:
+  - "@fast-check/ava"
+  - "@fast-check/jest"
+  - "@fast-check/vitest"
+  - "@fast-check/worker"
diff --git a/packages/fast-check/src/check/property/AsyncProperty.generic.ts b/packages/fast-check/src/check/property/AsyncProperty.generic.ts
@@ -119,14 +119,14 @@ export class AsyncProperty<Ts> implements IAsyncPropertyWithHooks<Ts> {
         ? null
         : {
             error: new Error('Property failed by returning false'),
-            errorMessage: 'Property failed by returning false',
+            errorMessage: 'Error: Property failed by returning false',
           };
     } catch (err) {
       // precondition failure considered as success for the first version
       if (PreconditionFailure.isFailure(err)) return err;
       // exception as PropertyFailure in case of real failure
       if (err instanceof Error && err.stack) {
-        return { error: err, errorMessage: `${err}\n\nStack trace: ${err.stack}` };
+        return { error: err, errorMessage: err.stack }; // stack includes the message
       }
       return { error: err, errorMessage: String(err) };
     } finally {

diff --git a/packages/fast-check/src/check/property/Property.generic.ts b/packages/fast-check/src/check/property/Property.generic.ts
@@ -135,14 +135,14 @@ export class Property<Ts> implements IProperty<Ts>, IPropertyWithHooks<Ts> {
         ? null
         : {
             error: new Error('Property failed by returning false'),
-            errorMessage: 'Property failed by returning false',
+            errorMessage: 'Error: Property failed by returning false',
           };
     } catch (err) {
       // precondition failure considered as success for the first version
       if (PreconditionFailure.isFailure(err)) return err;
       // exception as PropertyFailure in case of real failure
       if (err instanceof Error && err.stack) {
-        return { error: err, errorMessage: `${err}\n\nStack trace: ${err.stack}` };
+        return { error: err, errorMessage: err.stack }; // stack includes the message
       }
       return { error: err, errorMessage: String(err) };
     } finally {

diff --git a/packages/fast-check/src/check/runner/utils/RunDetailsFormatter.ts b/packages/fast-check/src/check/runner/utils/RunDetailsFormatter.ts
@@ -1,4 +1,4 @@
-import { Error, safePush } from '../../../utils/globals';
+import { Error, safePush, safeReplace } from '../../../utils/globals';
 import { stringify, possiblyAsyncStringify } from '../../../utils/stringify';
 import { VerbosityLevel } from '../configuration/VerbosityLevel';
 import { ExecutionStatus } from '../reporter/ExecutionStatus';
@@ -82,7 +82,7 @@ function preFormatTooManySkipped<Ts>(out: RunDetailsFailureTooManySkips<Ts>, str
 /** @internal */
 function preFormatFailure<Ts>(out: RunDetailsFailureProperty<Ts>, stringifyOne: (value: Ts) => string) {
   const noErrorInMessage = out.runConfiguration.errorWithCause;
-  const messageErrorPart = noErrorInMessage ? '' : `\nGot error: ${out.error}`;
+  const messageErrorPart = noErrorInMessage ? '' : `\nGot ${safeReplace(out.error, /^Error: /, 'error: ')}`;
   const message = `Property failed after ${out.numRuns} tests\n{ seed: ${out.seed}, path: "${
     out.counterexamplePath
   }", endOnFailure: true }\nCounterexample: ${stringifyOne(out.counterexample)}\nShrunk ${

diff --git a/packages/fast-check/src/utils/globals.ts b/packages/fast-check/src/utils/globals.ts
@@ -278,6 +278,7 @@ const untouchedToLowerCase = String.prototype.toLowerCase;
 const untouchedToUpperCase = String.prototype.toUpperCase;
 const untouchedPadStart = String.prototype.padStart;
 const untouchedCharCodeAt = String.prototype.charCodeAt;
+const untouchedReplace: (pattern: RegExp | string, replacement: string) => string = String.prototype.replace;
 function extractSplit(instance: string) {
   try {
     return instance.split;
@@ -334,6 +335,13 @@ function extractCharCodeAt(instance: string) {
     return undefined;
   }
 }
+function extractReplace(instance: string) {
+  try {
+    return instance.replace;
+  } catch (err) {
+    return undefined;
+  }
+}
 export function safeSplit(
   instance: string,
   ...args: [separator: string | RegExp, limit?: number | undefined]
@@ -391,6 +399,12 @@ export function safeCharCodeAt(instance: string, index: number): number {
   }
   return safeApply(untouchedCharCodeAt, instance, [index]);
 }
+export function safeReplace(instance: string, pattern: RegExp | string, replacement: string): string {
+  if (extractReplace(instance) === untouchedReplace) {
+    return instance.replace(pattern, replacement);
+  }
+  return safeApply(untouchedReplace, instance, [pattern, replacement]);
+}
 
 // Number
 

diff --git a/packages/fast-check/test/e2e/NoRegressionStack.spec.ts b/packages/fast-check/test/e2e/NoRegressionStack.spec.ts
@@ -0,0 +1,61 @@
+import fc from '../../src/fast-check';
+
+const settings = { seed: 42, verbose: 0 };
+
+describe(`NoRegressionStack`, () => {
+  it('throw', () => {
+    expect(
+      sanitize(() =>
+        fc.assert(
+          fc.property(fc.nat(), fc.nat(), (a, b) => {
+            if (a < b) {
+              throw new Error('a must be >= b');
+            }
+          }),
+          settings
+        )
+      )
+    ).toThrowErrorMatchingSnapshot();
+  });
+
+  it('not a function', () => {
+    expect(
+      sanitize(() =>
+        fc.assert(
+          fc.property(fc.nat(), (v) => {
+            (v as any)();
+          }),
+          settings
+        )
+      )
+    ).toThrowErrorMatchingSnapshot();
+  });
+});
+
+// Helpers
+
+function sanitize(run: () => void) {
+  return () => {
+    try {
+      run();
+    } catch (err) {
+      const initialMessage = (err as Error).message;
+      const lines = initialMessage
+        .replace(/\\/g, '/')
+        .replace(/at [^(]*fast-check\/(packages|node_modules)(.*)/g, 'at $1$2')
+        .replace(/at (.*) \(.*fast-check\/(packages|node_modules)(.*)\)/g, 'at $1 ($2$3)')
+        .replace(/at (.*) \((node_modules\/.*):\d+:\d+\)/g, 'at $1 ($2:?:?)') // reducing risks of changes on bumps
+        .split('\n');
+      throw new Error(
+        lines
+          .slice(
+            0,
+            // internals of jest, subject to regular changes
+            // and OS dependent
+            lines.findIndex((line) => line.includes('node_modules/jest-circus'))
+          )
+          .join('\n')
+      );
+    }
+  };
+}
diff --git a/packages/fast-check/test/e2e/__snapshots__/NoRegressionStack.spec.ts.snap b/packages/fast-check/test/e2e/__snapshots__/NoRegressionStack.spec.ts.snap
@@ -0,0 +1,41 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`NoRegressionStack not a function 1`] = `
+"Property failed after 1 tests
+{ seed: 42, path: "0:0", endOnFailure: true }
+Counterexample: [0]
+Shrunk 1 time(s)
+Got TypeError: v is not a function
+    at packages/fast-check/test/e2e/NoRegressionStack.spec.ts:26:14
+    at Property.predicate (packages/fast-check/src/check/property/Property.ts:23:56)
+    at Property.predicate [as run] (packages/fast-check/src/check/property/Property.generic.ts:133:27)
+    at run (packages/fast-check/src/check/runner/Runner.ts:37:26)
+    at runIt (packages/fast-check/src/check/runner/Runner.ts:154:7)
+    at Object.check (packages/fast-check/src/check/runner/Runner.ts:206:15)
+    at packages/fast-check/test/e2e/NoRegressionStack.spec.ts:40:7
+    at _toThrowErrorMatchingSnapshot (node_modules/jest-snapshot/build/index.js:?:?)
+    at Object.toThrowErrorMatchingSnapshot (node_modules/jest-snapshot/build/index.js:?:?)
+    at __EXTERNAL_MATCHER_TRAP__ (node_modules/expect/build/index.js:?:?)
+    at Object.throwingMatcher [as toThrowErrorMatchingSnapshot] (node_modules/expect/build/index.js:?:?)
+    at Object.<anonymous> (packages/fast-check/test/e2e/NoRegressionStack.spec.ts:31:7)"
+`;
+
+exports[`NoRegressionStack throw 1`] = `
+"Property failed after 2 tests
+{ seed: 42, path: "1:0:1:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0", endOnFailure: true }
+Counterexample: [0,1]
+Shrunk 32 time(s)
+Got error: a must be >= b
+    at packages/fast-check/test/e2e/NoRegressionStack.spec.ts:12:21
+    at Property.predicate (packages/fast-check/src/check/property/Property.ts:23:56)
+    at Property.predicate [as run] (packages/fast-check/src/check/property/Property.generic.ts:133:27)
+    at run (packages/fast-check/src/check/runner/Runner.ts:37:26)
+    at runIt (packages/fast-check/src/check/runner/Runner.ts:154:7)
+    at Object.check (packages/fast-check/src/check/runner/Runner.ts:206:15)
+    at packages/fast-check/test/e2e/NoRegressionStack.spec.ts:40:7
+    at _toThrowErrorMatchingSnapshot (node_modules/jest-snapshot/build/index.js:?:?)
+    at Object.toThrowErrorMatchingSnapshot (node_modules/jest-snapshot/build/index.js:?:?)
+    at __EXTERNAL_MATCHER_TRAP__ (node_modules/expect/build/index.js:?:?)
+    at Object.throwingMatcher [as toThrowErrorMatchingSnapshot] (node_modules/expect/build/index.js:?:?)
+    at Object.<anonymous> (packages/fast-check/test/e2e/NoRegressionStack.spec.ts:18:7)"
+`;
diff --git a/packages/fast-check/test/unit/check/property/AsyncProperty.spec.ts b/packages/fast-check/test/unit/check/property/AsyncProperty.spec.ts
@@ -46,7 +46,7 @@ describe.each([[true], [false]])('AsyncProperty (dontRunHook: %p)', (dontRunHook
 
     // Assert
     expect((out as PropertyFailure).errorMessage).toContain('predicate throws');
-    expect((out as PropertyFailure).errorMessage).toContain('\n\nStack trace:');
+    expect((out as PropertyFailure).errorMessage).toContain('at ');
     expect((out as PropertyFailure).error).toBe(originalError);
   });
   it('Should fail if predicate throws a raw string', async () => {

diff --git a/packages/fast-check/test/unit/check/property/Property.spec.ts b/packages/fast-check/test/unit/check/property/Property.spec.ts
@@ -46,7 +46,7 @@ describe.each([[true], [false]])('Property (dontRunHook: %p)', (dontRunHook) =>
 
     // Assert
     expect((out as PropertyFailure).errorMessage).toContain('predicate throws');
-    expect((out as PropertyFailure).errorMessage).toContain('\n\nStack trace:');
+    expect((out as PropertyFailure).errorMessage).toContain('at ');
     expect((out as PropertyFailure).error).toBe(originalError);
   });
   it('Should fail if predicate throws a raw string', () => {

diff --git a/packages/fast-check/test/unit/check/runner/Runner.spec.ts b/packages/fast-check/test/unit/check/runner/Runner.spec.ts
@@ -565,7 +565,7 @@ describe('Runner', () => {
       );
     });
     it('Should put the orginal error in error message', () => {
-      expect(() => rAssert(failingProperty, { seed: 42 })).toThrowError(`Got error: error in failingProperty`);
+      expect(() => rAssert(failingProperty, { seed: 42 })).toThrowError(`Got error in failingProperty`);
     });
     describe('Impact of VerbosityLevel in case of failure', () => {
       const baseErrorMessage = '';