[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__/custom_matcher.test.js.snap

@@ -0,0 +1,15 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Custom matcher preserves error stack 1`] = `
+"Error: qux
+    at baz (/Users/bvaughn/Documents/git/jest/integration_tests/__tests__/custom_matcher.test.js:49:13)
+    at bar (/Users/bvaughn/Documents/git/jest/integration_tests/__tests__/custom_matcher.test.js:47:23)
+    at foo (/Users/bvaughn/Documents/git/jest/integration_tests/__tests__/custom_matcher.test.js:46:23)
+    at expect (/Users/bvaughn/Documents/git/jest/integration_tests/__tests__/custom_matcher.test.js:54:9)
+    at Object.toCustomMatch (/Users/bvaughn/Documents/git/jest/integration_tests/__tests__/custom_matcher.test.js:12:18)
+    at Object.throwingMatcher [as toCustomMatch] (/Users/bvaughn/Documents/git/jest/packages/expect/build/index.js:214:24)
+    at Object.it (/Users/bvaughn/Documents/git/jest/integration_tests/__tests__/custom_matcher.test.js:55:10)
+    at Object.asyncFn (/Users/bvaughn/Documents/git/jest/packages/jest-jasmine2/build/jasmine_async.js:129:432)
+    at resolve (/Users/bvaughn/Documents/git/jest/packages/jest-jasmine2/build/queue_runner.js:51:12)
+    at Promise (<anonymous>)"
+`;

integration_tests/__tests__/custom_matcher.test.js

@@ -0,0 +1,60 @@
+/**
+ * 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.
+ *
+ * @flow
+ */
+'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,
+});
+
+declare var expect: (func: Function) => any;
+
+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');
+    };
+
+    try {
+      expect(() => {
+        foo();
+      }).toCustomMatch('test');
+    } catch (error) {
+      expect(error.stack).toMatchSnapshot();
+    }
+  });
+});

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;