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

CHANGELOG.md

@@ -4,6 +4,8 @@
 
 * `[jest-config]` fix unexpected condition to avoid infinite recursion in
   Windows platform. ([#5161](https://github.com/facebook/jest/pull/5161))
+* Do not override `Error` stack (with `Error.captureStackTrace`) for custom matchers.
+  ([#5162](https://github.com/facebook/jest/pull/5162))
 
 ### Features
 
@@ -1347,4 +1349,4 @@ See https://facebook.github.io/jest/blog/2016/12/15/2016-in-jest.html
 
 ## <=0.4.0
 
-* See commit history for changes in previous versions of jest.
+* See commit history for changes in previous versions of jest.

integration_tests/__tests__/__snapshots__/stack_trace.test.js.snap

@@ -62,3 +62,31 @@ Time:        <<REPLACED>>
 Ran all test suites matching /runtime_error.test.js/i.
 "
 `;
+
+exports[`works with custom matchers 1`] = `
+"FAIL __tests__/custom_matcher.test.js
+  Custom matcher
+    ✓ passes
+    ✓ fails
+    ✕ preserves error stack
+
+  ● Custom matcher › preserves error stack
+
+    qux
+
+      44 |     const bar = () => baz();
+      45 |     const baz = () => {
+    > 46 |       throw Error('qux');
+      47 |     };
+      48 | 
+      49 |     expect(() => {
+
+      at __tests__/custom_matcher.test.js:46:13
+      at __tests__/custom_matcher.test.js:44:23
+      at __tests__/custom_matcher.test.js:43:23
+      at __tests__/custom_matcher.test.js:50:7
+      at __tests__/custom_matcher.test.js:11:18
+      at __tests__/custom_matcher.test.js:51:8
+
+"
+`;

integration_tests/__tests__/stack_trace.test.js

@@ -122,3 +122,9 @@ describe('Stack Trace', () => {
     );
   });
 });
+
+test('works with custom matchers', () => {
+  const {stderr} = runJest('stack_trace', ['custom_matcher.test.js']);
+
+  expect(extractSummary(stderr).rest).toMatchSnapshot();
+});

integration_tests/stack_trace/__tests__/custom_matcher.test.js

@@ -0,0 +1,53 @@
+/**
+ * Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ */
+'use strict';
+
+function toCustomMatch(callback, expectation) {
+  const actual = callback();
+
+  if (actual !== expectation) {
+    return {
+      message: () => `Expected "${expectation}" but got "${actual}"`,
+      pass: false,
+    };
+  } else {
+    return {pass: true};
+  }
+}
+
+expect.extend({
+  toCustomMatch,
+});
+
+describe('Custom matcher', () => {
+  // This test is expected to pass
+  it('passes', () => {
+    expect(() => 'foo').toCustomMatch('foo');
+  });
+
+  // This test is expected to fail
+  it('fails', () => {
+    expect(() => {
+      expect(() => 'foo').toCustomMatch('bar');
+    }).toThrow();
+  });
+
+  // This test fails due to an unrelated/unexpected error
+  // It will show a helpful stack trace though
+  it('preserves error stack', () => {
+    const foo = () => bar();
+    const bar = () => baz();
+    const baz = () => {
+      throw Error('qux');
+    };
+
+    expect(() => {
+      foo();
+    }).toCustomMatch('test');
+  });
+});

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

@@ -9,6 +9,18 @@
 const jestExpect = require('../');
 
 jestExpect.extend({
+  toCustomMatch(callback, expectation) {
+    const actual = callback();
+
+    if (actual !== expectation) {
+      return {
+        message: () => `Expected "${expectation}" but got "${actual}"`,
+        pass: false,
+      };
+    }
+
+    return {pass: true};
+  },
   toMatchPredicate(received, argument) {
     argument(received);
     return {
@@ -22,7 +34,7 @@ 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) => {

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) => {
+  Object.keys(matchers).forEach(key => {
+    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;