jest-jasmine2: pretty-print non-Error errors (#5980)

Author: wmertens

CHANGELOG.md

@@ -54,6 +54,8 @@
   ([#5864](https://github.com/facebook/jest/pull/5864))
 * `[jest-editor-support]` Add `no-color` option to runner
   ([#5909](https://github.com/facebook/jest/pull/5909))
+* Pretty-print non-Error object errors
+  ([#5980](https://github.com/facebook/jest/pull/5980))
 
 ### Fixes
 

integration-tests/__tests__/__snapshots__/failures.test.js.snap

@@ -68,6 +68,96 @@ exports[`not throwing Error objects 4`] = `
 "
 `;
 
+exports[`not throwing Error objects 5`] = `
+"FAIL __tests__/during_tests.test.js
+  ✕ Promise thrown during test
+  ✕ Boolean thrown during test
+  ✕ undefined thrown during test
+  ✕ Object thrown during test
+  ✕ Error during test
+  ✕ done(Error)
+  ✕ done(non-error)
+
+  ● Promise thrown during test
+
+    thrown: Promise {}
+      
+      at packages/jest-jasmine2/build/expectation_result_factory.js:54:47
+
+  ● Boolean thrown during test
+
+    thrown: false
+      
+      at packages/jest-jasmine2/build/expectation_result_factory.js:54:47
+
+  ● undefined thrown during test
+
+    thrown: undefined
+      
+      at packages/jest-jasmine2/build/expectation_result_factory.js:54:47
+
+  ● Object thrown during test
+
+    thrown: Object {
+      \\"notAnError\\": Array [
+        Object {
+          \\"hello\\": true,
+          \\"tooDeep\\": [Object],
+        },
+      ],
+    }
+      
+      at packages/jest-jasmine2/build/expectation_result_factory.js:54:47
+
+  ● Error during test
+
+    ReferenceError: doesNotExist is not defined
+
+      26 | test('Error during test', () => {
+      27 |   // eslint-disable-next-line no-undef
+    > 28 |   doesNotExist.alsoThisNot;
+      29 | });
+      30 | 
+      31 | test('done(Error)', done => {
+      
+      at __tests__/during_tests.test.js:28:3
+
+  ● done(Error)
+
+    this is an error
+
+      30 | 
+      31 | test('done(Error)', done => {
+    > 32 |   done(new Error('this is an error'));
+      33 | });
+      34 | 
+      35 | test('done(non-error)', done => {
+      
+      at __tests__/during_tests.test.js:32:8
+
+  ● done(non-error)
+
+    Failed: Object {
+      \\"notAnError\\": Array [
+        Object {
+          \\"hello\\": true,
+          \\"tooDeep\\": [Object],
+        },
+      ],
+    }
+
+      34 | 
+      35 | test('done(non-error)', done => {
+    > 36 |   done(deepObject);
+      37 | });
+      38 | 
+      
+      at packages/jest-jasmine2/build/expectation_result_factory.js:54:47
+      at __tests__/during_tests.test.js:36:3
+
+"
+`;
+
 exports[`works with assertions in separate files 1`] = `
 "FAIL __tests__/test_macro.test.js
   ✕ use some imported macro to make assertion
@@ -442,8 +532,9 @@ exports[`works with node assert 1`] = `
 
   ● assert.ifError
 
-    Error
-      1 thrown
+    thrown: 1
+      
+      at packages/jest-jasmine2/build/expectation_result_factory.js:54:47
 
   ● assert.doesNotThrow
 

integration-tests/__tests__/failures.test.js

@@ -52,6 +52,8 @@ test('not throwing Error objects', () => {
   expect(extractSummary(stderr).rest).toMatchSnapshot();
   stderr = runJest(dir, ['assertion_count.test.js']).stderr;
   expect(extractSummary(cleanupStackTrace(stderr)).rest).toMatchSnapshot();
+  stderr = runJest(dir, ['during_tests.test.js']).stderr;
+  expect(extractSummary(stderr).rest).toMatchSnapshot();
 });
 
 test('works with node assert', () => {

integration-tests/__tests__/stack_trace.test.js

@@ -78,7 +78,7 @@ describe('Stack Trace', () => {
     expect(result.status).toBe(1);
 
     expect(stderr).toMatch(/this is unexpected\./);
-    expect(stderr).toMatch(/this is a string\. thrown/);
+    expect(stderr).toMatch(/this is a string\./);
 
     expect(stderr).toMatch(/\s+at\s(?:.+?)\s\(__tests__\/test_error.test\.js/);
 

integration-tests/failures/__tests__/during_tests.test.js

@@ -0,0 +1,37 @@
+'use strict';
+
+const deepObject = {
+  notAnError: [{hello: true, tooDeep: {notVisible: true}}],
+};
+
+test('Promise thrown during test', () => {
+  throw Promise.resolve(5);
+});
+
+test('Boolean thrown during test', () => {
+  // eslint-disable-next-line no-throw-literal
+  throw false;
+});
+
+test('undefined thrown during test', () => {
+  // eslint-disable-next-line no-throw-literal
+  throw undefined;
+});
+
+test('Object thrown during test', () => {
+  // eslint-disable-next-line no-throw-literal
+  throw deepObject;
+});
+
+test('Error during test', () => {
+  // eslint-disable-next-line no-undef
+  doesNotExist.alsoThisNot;
+});
+
+test('done(Error)', done => {
+  done(new Error('this is an error'));
+});
+
+test('done(non-error)', done => {
+  done(deepObject);
+});

packages/jest-jasmine2/package.json

@@ -18,7 +18,8 @@
     "jest-message-util": "^22.4.0",
     "jest-snapshot": "^22.4.0",
     "jest-util": "^22.4.1",
-    "source-map-support": "^0.5.0"
+    "source-map-support": "^0.5.0",
+    "pretty-format": "^22.4.0"
   },
   "devDependencies": {
     "jest-runtime": "^22.4.2"

packages/jest-jasmine2/src/__tests__/expectation_result_factory.test.js

@@ -28,7 +28,7 @@ describe('expectationResultFactory', () => {
       passed: false,
     };
     const result = expectationResultFactory(options);
-    expect(result.message).toEqual('');
+    expect(result.message).toEqual('thrown: undefined');
   });
 
   it('returns the result if failed (with `message`).', () => {
@@ -66,6 +66,6 @@ describe('expectationResultFactory', () => {
       passed: false,
     };
     const result = expectationResultFactory(options);
-    expect(result.message).toEqual('Expected `Pass`, received `Fail`. thrown');
+    expect(result.message).toEqual('Expected `Pass`, received `Fail`.');
   });
 });

packages/jest-jasmine2/src/expectation_result_factory.js

@@ -7,26 +7,36 @@
  * @flow
  */
 
+import prettyFormat from 'pretty-format';
+
 function messageFormatter({error, message, passed}) {
   if (passed) {
     return 'Passed.';
   }
   if (message) {
     return message;
   }
-  if (!error) {
-    return '';
+  if (typeof error === 'string') {
+    return error;
+  }
+  if (
+    // duck-type Error, see #2549
+    error &&
+    typeof error === 'object' &&
+    typeof error.message === 'string' &&
+    typeof error.name === 'string'
+  ) {
+    return `${error.name}: ${error.message}`;
   }
-  return error.message && error.name
-    ? `${error.name}: ${error.message}`
-    : `${error.toString()} thrown`;
+  return `thrown: ${prettyFormat(error, {maxDepth: 3})}`;
 }
 
 function stackFormatter(options, errorMessage) {
   if (options.passed) {
     return '';
   }
-  const {stack} = options.error || new Error(errorMessage);
+  const stack =
+    (options.error && options.error.stack) || new Error(errorMessage).stack;
   return stack;
 }
 

packages/jest-jasmine2/src/jasmine/Env.js

@@ -32,6 +32,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 import queueRunner from '../queue_runner';
 import treeProcessor from '../tree_processor';
+import prettyFormat from 'pretty-format';
 
 // Try getting the real promise object from the context, if available. Someone
 // could have overridden it in a test. Async functions return it implicitly.
@@ -547,19 +548,20 @@ export default function(j$) {
     };
 
     this.fail = function(error) {
-      let message = 'Failed';
-      if (error) {
-        message += ': ';
-        message += error.message || error;
-      }
+      // duck-type Error, see #2549
+      const isError =
+        typeof error === 'object' &&
+        typeof error.message === 'string' &&
+        typeof error.name === 'string';
+      const message = `Failed: ${prettyFormat(error, {maxDepth: 3})}`;
 
       currentRunnable().addExpectationResult(false, {
         matcherName: '',
         passed: false,
         expected: '',
         actual: '',
         message,
-        error: error && error.message ? error : null,
+        error: isError ? error : null,
       });
     };
   }