[Discussion] Expect only overrides error stack for built-in matchers

CHANGELOG.md

@@ -1,11 +1,11 @@
 ## master
 
-None for now
-
 ### Fixes
 
 * `[babel-jest]` moduleFileExtensions not passed to babel transformer.
   ([#4637](https://github.com/facebook/jest/issues/4637))
+* Do not override `Error` stack (with `Error.captureStackTrace`) for custom matchers.
+  ([#5162](https://github.com/facebook/jest/pull/5162))
 
 ### Features
 

packages/expect/src/__tests__/stacktrace.test.js

@@ -16,13 +16,25 @@ jestExpect.extend({
       pass: true,
     };
   },
+  toCustomMatch(callback, expectation) {
+    const actual = callback();
+
+    if (actual !== expectation) {
+      return {
+        pass: false,
+        message: () => `Expected "${expectation}" but got "${actual}"`
+      };
+    }
+
+    return {pass: true};
+  },
 });
 
 it('stack trace points to correct location when using matchers', () => {
   try {
     jestExpect(true).toBe(false);
   } catch (error) {
-    expect(error.stack).toContain('stacktrace.test.js:23');
+    expect(error.stack).toContain('stacktrace.test.js:35');
   }
 });
 
@@ -32,6 +44,22 @@ it('stack trace points to correct location when using nested matchers', () => {
       jestExpect(value).toBe(false);
     });
   } catch (error) {
-    expect(error.stack).toContain('stacktrace.test.js:32');
+    expect(error.stack).toContain('stacktrace.test.js:44');
+  }
+});
+
+it('stack trace points to correct location when throwing from a custom matcher', () => {
+  try {
+    jestExpect(() => {
+      const foo = () => bar();
+      const bar = () => baz();
+      const baz = () => {
+        throw new Error('Expected');
+      };
+
+      foo();
+    }).toCustomMatch('bar');
+  } catch (error) {
+    expect(error.stack).toContain('stacktrace.test.js:57');
   }
 });

packages/expect/src/index.js

@@ -214,6 +214,7 @@ const makeThrowingMatcher = (
       result = matcher.apply(matcherContext, [actual].concat(args));
     } catch (error) {
       if (
+        matcher.__jestInternal === true &&
         !(error instanceof JestAssertionError) &&
         error.name !== 'PrettyFormatPluginError' &&
         // Guard for some environments (browsers) that do not support this feature.
@@ -252,7 +253,8 @@ const makeThrowingMatcher = (
   };
 };
 
-expect.extend = (matchers: MatchersObject): void => setMatchers(matchers);
+expect.extend = (matchers: MatchersObject): void =>
+  setMatchers(matchers, false);
 
 expect.anything = anything;
 expect.any = any;
@@ -280,9 +282,9 @@ const _validateResult = result => {
 };
 
 // add default jest matchers
-expect.extend(matchers);
-expect.extend(spyMatchers);
-expect.extend(toThrowMatchers);
+setMatchers(matchers, true);
+setMatchers(spyMatchers, true);
+setMatchers(toThrowMatchers, true);
 
 expect.addSnapshotSerializer = () => void 0;
 expect.assertions = (expected: number) => {
@@ -296,4 +298,8 @@ expect.getState = getState;
 expect.setState = setState;
 expect.extractExpectedAssertionsErrors = extractExpectedAssertionsErrors;
 
+// Expose JestAssertionError for custom matchers
+// This enables them to preserve the stack for specific errors
+expect.JestAssertionError = JestAssertionError;
+
 module.exports = (expect: Expect);

packages/expect/src/jest_matchers_object.js

@@ -35,6 +35,13 @@ export const setState = (state: Object) => {
 
 export const getMatchers = () => global[JEST_MATCHERS_OBJECT].matchers;
 
-export const setMatchers = (matchers: MatchersObject) => {
+export const setMatchers = (matchers: MatchersObject, isInternal: boolean) => {
+  for (const key in matchers) {
+    const matcher = matchers[key];
+    Object.defineProperty(matcher, '__jestInternal', {
+      value: isInternal,
+    });
+  }
+
   Object.assign(global[JEST_MATCHERS_OBJECT].matchers, matchers);
 };

types/Matchers.js

@@ -19,6 +19,7 @@ export type RawMatcherFn = (
   expected: any,
   actual: any,
   options: any,
+  __jestInternal?: boolean,
 ) => ExpectationResult;
 
 export type ThrowingMatcherFn = (actual: any) => void;