Call buildExpectationResult directly from Suite and Spec

This removes quite a bit of indirection from result processing, at the
cost of making a few of the tests more awkward.

lib/jasmine-core/jasmine.js

@@ -1501,7 +1501,7 @@ getJasmineRequireObj().Env = function(j$) {
         error.matcherName !== undefined && error.passed !== undefined;
       const result = isExpectationResult
         ? error
-        : expectationResultFactory({
+        : j$.buildExpectationResult({
             error,
             passed: false,
             matcherName: '',
@@ -1651,16 +1651,6 @@ getJasmineRequireObj().Env = function(j$) {
       return fullName.join(' ');
     };
 
-    // TODO: we may just be able to pass in the fn instead of wrapping here
-    var buildExpectationResult = j$.buildExpectationResult,
-      exceptionFormatter = new j$.ExceptionFormatter(),
-      expectationResultFactory = function(attrs) {
-        attrs.messageFormatter = exceptionFormatter.message;
-        attrs.stackFormatter = exceptionFormatter.stack;
-
-        return buildExpectationResult(attrs);
-      };
-
     /**
      * Causes a deprecation warning to be logged to the console and reported to
      * reporters.
@@ -1726,7 +1716,7 @@ getJasmineRequireObj().Env = function(j$) {
       description: 'Jasmine__TopLevel__Suite',
       expectationFactory: expectationFactory,
       asyncExpectationFactory: suiteAsyncExpectationFactory,
-      expectationResultFactory: expectationResultFactory,
+      expectationResultFactory: j$.buildExpectationResult,
       autoCleanClosures: config.autoCleanClosures,
       onLateError: recordLateError
     });
@@ -2185,7 +2175,7 @@ getJasmineRequireObj().Env = function(j$) {
         timer: new j$.Timer(),
         expectationFactory: expectationFactory,
         asyncExpectationFactory: suiteAsyncExpectationFactory,
-        expectationResultFactory: expectationResultFactory,
+        expectationResultFactory: j$.buildExpectationResult,
         throwOnExpectationFailure: config.stopSpecOnExpectationFailure,
         autoCleanClosures: config.autoCleanClosures,
         onLateError: recordLateError
@@ -2294,7 +2284,7 @@ getJasmineRequireObj().Env = function(j$) {
         },
         onStart: specStarted,
         description: description,
-        expectationResultFactory: expectationResultFactory,
+        expectationResultFactory: j$.buildExpectationResult,
         queueRunnerFactory: queueRunnerFactory,
         userContext: function() {
           return suite.clonedSharedUserContext();
@@ -3131,8 +3121,7 @@ getJasmineRequireObj().Truthy = function(j$) {
 //TODO: expectation result may make more sense as a presentation of an expectation.
 getJasmineRequireObj().buildExpectationResult = function(j$) {
   function buildExpectationResult(options) {
-    const messageFormatter = options.messageFormatter || function() {};
+    const exceptionFormatter = new j$.ExceptionFormatter();
-    const stackFormatter = options.stackFormatter || function() {};
 
     /**
      * @typedef Expectation
@@ -3182,7 +3171,7 @@ getJasmineRequireObj().buildExpectationResult = function(j$) {
       } else if (options.message) {
         return options.message;
       } else if (options.error) {
-        return messageFormatter(options.error);
+        return exceptionFormatter.message(options.error);
       }
       return '';
     }
@@ -3209,7 +3198,7 @@ getJasmineRequireObj().buildExpectationResult = function(j$) {
       }
       // Omit the message from the stack trace because it will be
       // included elsewhere.
-      return stackFormatter(error, { omitMessage: true });
+      return exceptionFormatter.stack(error, { omitMessage: true });
     }
   }
 

spec/core/SpecSpec.js

@@ -334,7 +334,7 @@ describe('Spec', function() {
     const spec = new jasmineUnderTest.Spec({
       queueableFn: { fn: jasmine.createSpy('spec body') }
     });
-    spec.addExpectationResult(true);
+    spec.addExpectationResult(true, {});
     expect(spec.status()).toBe('passed');
   });
 
@@ -342,8 +342,8 @@ describe('Spec', function() {
     const spec = new jasmineUnderTest.Spec({
       queueableFn: { fn: jasmine.createSpy('spec body') }
     });
-    spec.addExpectationResult(true);
+    spec.addExpectationResult(true, {});
-    spec.addExpectationResult(false);
+    spec.addExpectationResult(false, {});
     expect(spec.status()).toBe('failed');
   });
 
@@ -352,14 +352,11 @@ describe('Spec', function() {
       resultCallback = jasmine.createSpy('resultCallback'),
       spec = new jasmineUnderTest.Spec({
         queueableFn: { fn: jasmine.createSpy('spec body') },
-        expectationResultFactory: function(data) {
-          return data;
-        },
         queueRunnerFactory: fakeQueueRunner,
         resultCallback: resultCallback
       });
-    spec.addExpectationResult(true, 'expectation1');
+    spec.addExpectationResult(true, { message: 'expectation1' });
-    spec.addExpectationResult(false, 'expectation2');
+    spec.addExpectationResult(false, { message: 'expectation2' });
 
     spec.execute();
 
@@ -367,10 +364,10 @@ describe('Spec', function() {
     fns[fns.length - 1].fn();
 
     expect(resultCallback.calls.first().args[0].passedExpectations).toEqual([
-      'expectation1'
+      jasmine.objectContaining({ message: 'expectation1' })
     ]);
     expect(resultCallback.calls.first().args[0].failedExpectations).toEqual([
-      'expectation2'
+      jasmine.objectContaining({ message: 'expectation2' })
     ]);
   });
 
@@ -379,17 +376,14 @@ describe('Spec', function() {
       resultCallback = jasmine.createSpy('resultCallback'),
       spec = new jasmineUnderTest.Spec({
         queueableFn: { fn: function() {} },
-        expectationResultFactory: function(data) {
-          return data;
-        },
         queueRunnerFactory: fakeQueueRunner,
         resultCallback: resultCallback,
         throwOnExpectationFailure: true
       });
 
-    spec.addExpectationResult(true, 'passed');
+    spec.addExpectationResult(true, { message: 'passed' });
     expect(function() {
-      spec.addExpectationResult(false, 'failed');
+      spec.addExpectationResult(false, { message: 'failed' });
     }).toThrowError(jasmineUnderTest.errors.ExpectationFailed);
 
     spec.execute();
@@ -397,20 +391,16 @@ describe('Spec', function() {
     const fns = fakeQueueRunner.calls.mostRecent().args[0].queueableFns;
     fns[fns.length - 1].fn();
     expect(resultCallback.calls.first().args[0].passedExpectations).toEqual([
-      'passed'
+      jasmine.objectContaining({ message: 'passed' })
     ]);
     expect(resultCallback.calls.first().args[0].failedExpectations).toEqual([
-      'failed'
+      jasmine.objectContaining({ message: 'failed' })
     ]);
   });
 
   it('forwards late expectation failures to onLateError', function() {
     const onLateError = jasmine.createSpy('onLateError');
-    const expectationResultFactory = jasmine
-      .createSpy('expectationResultFactory')
-      .and.returnValue('built expectation result');
     const spec = new jasmineUnderTest.Spec({
-      expectationResultFactory,
       onLateError,
       queueableFn: { fn: function() {} }
     });
@@ -425,15 +415,17 @@ describe('Spec', function() {
     spec.reportedDone = true;
     spec.addExpectationResult(false, data, true);
 
-    expect(expectationResultFactory).toHaveBeenCalledWith(data);
+    expect(onLateError).toHaveBeenCalledWith(
-    expect(onLateError).toHaveBeenCalledWith('built expectation result');
+      jasmine.objectContaining({
+        message: jasmine.stringMatching(/^Error: nope/)
+      })
+    );
     expect(spec.result.failedExpectations).toEqual([]);
   });
 
   it('does not forward non-late expectation failures to onLateError', function() {
     const onLateError = jasmine.createSpy('onLateError');
     const spec = new jasmineUnderTest.Spec({
-      expectationResultFactory: r => r,
       onLateError,
       queueableFn: { fn: function() {} }
     });
@@ -452,34 +444,25 @@ describe('Spec', function() {
 
   it('forwards late handleException calls to onLateError', function() {
     const onLateError = jasmine.createSpy('onLateError');
-    const expectationResultFactory = jasmine
-      .createSpy('expectationResultFactory')
-      .and.returnValue('built expectation result');
     const spec = new jasmineUnderTest.Spec({
-      expectationResultFactory,
       onLateError,
       queueableFn: { fn: function() {} }
     });
-    const error = new Error('oops');
 
     spec.reportedDone = true;
-    spec.handleException(error);
+    spec.handleException(new Error('oops'));
 
-    expect(expectationResultFactory).toHaveBeenCalledWith({
+    expect(onLateError).toHaveBeenCalledWith(
-      matcherName: '',
+      jasmine.objectContaining({
-      passed: false,
+        message: jasmine.stringMatching(/^Error: oops/)
-      expected: '',
+      })
-      actual: '',
+    );
-      error
-    });
-    expect(onLateError).toHaveBeenCalledWith('built expectation result');
     expect(spec.result.failedExpectations).toEqual([]);
   });
 
   it('does not forward non-late handleException calls to onLateError', function() {
     const onLateError = jasmine.createSpy('onLateError');
     const spec = new jasmineUnderTest.Spec({
-      expectationResultFactory: r => r,
       onLateError,
       queueableFn: { fn: function() {} }
     });
@@ -493,7 +476,6 @@ describe('Spec', function() {
 
   it('clears the reportedDone flag when reset', function() {
     const spec = new jasmineUnderTest.Spec({
-      expectationResultFactory: r => r,
       queueableFn: { fn: function() {} }
     });
     spec.reportedDone = true;
@@ -507,9 +489,6 @@ describe('Spec', function() {
     const resultCallback = jasmine.createSpy('resultCallback'),
       spec = new jasmineUnderTest.Spec({
         queueableFn: { fn: function() {} },
-        expectationResultFactory: function(data) {
-          return data;
-        },
         queueRunnerFactory: function(attrs) {
           attrs.onComplete();
         },
@@ -582,9 +561,6 @@ describe('Spec', function() {
       resultCallback = jasmine.createSpy('resultCallback'),
       spec = new jasmineUnderTest.Spec({
         queueableFn: { fn: function() {} },
-        expectationResultFactory: function(data) {
-          return data;
-        },
         queueRunnerFactory: fakeQueueRunner,
         resultCallback: resultCallback
       });
@@ -596,11 +572,12 @@ describe('Spec', function() {
     args.queueableFns[args.queueableFns.length - 1].fn();
     expect(resultCallback.calls.first().args[0].failedExpectations).toEqual([
       {
-        error: 'foo',
+        message: 'foo thrown',
         matcherName: '',
         passed: false,
         expected: '',
-        actual: ''
+        actual: '',
+        stack: null
       }
     ]);
   });
@@ -610,9 +587,6 @@ describe('Spec', function() {
       resultCallback = jasmine.createSpy('resultCallback'),
       spec = new jasmineUnderTest.Spec({
         queueableFn: { fn: function() {} },
-        expectationResultFactory: function(data) {
-          return data;
-        },
         queueRunnerFactory: fakeQueueRunner,
         resultCallback: resultCallback
       });

spec/core/SuiteSpec.js

@@ -112,13 +112,9 @@ describe('Suite', function() {
   });
 
   it('has a status of failed if any expectations have failed', function() {
-    const suite = new jasmineUnderTest.Suite({
+    const suite = new jasmineUnderTest.Suite({});
-      expectationResultFactory: function() {
-        return 'hi';
-      }
-    });
 
-    suite.addExpectationResult(false);
+    suite.addExpectationResult(false, {});
     expect(suite.status()).toBe('failed');
   });
 
@@ -137,18 +133,17 @@ describe('Suite', function() {
 
   it('throws an ExpectationFailed when receiving a failed expectation when throwOnExpectationFailure is set', function() {
     const suite = new jasmineUnderTest.Suite({
-      expectationResultFactory: function(data) {
-        return data;
-      },
       throwOnExpectationFailure: true
     });
 
     expect(function() {
-      suite.addExpectationResult(false, 'failed');
+      suite.addExpectationResult(false, { message: 'failed' });
     }).toThrowError(jasmineUnderTest.errors.ExpectationFailed);
 
     expect(suite.status()).toBe('failed');
-    expect(suite.result.failedExpectations).toEqual(['failed']);
+    expect(suite.result.failedExpectations).toEqual([
+      jasmine.objectContaining({ message: 'failed' })
+    ]);
   });
 
   it('does not add an additional failure when an expectation fails', function() {
@@ -161,13 +156,7 @@ describe('Suite', function() {
 
   it('forwards late expectation failures to onLateError', function() {
     const onLateError = jasmine.createSpy('onLateError');
-    const expectationResultFactory = jasmine
+    const suite = new jasmineUnderTest.Suite({ onLateError });
-      .createSpy('expectationResultFactory')
-      .and.returnValue('built expectation result');
-    const suite = new jasmineUnderTest.Suite({
-      expectationResultFactory,
-      onLateError
-    });
     const data = {
       matcherName: '',
       passed: false,
@@ -179,15 +168,17 @@ describe('Suite', function() {
     suite.reportedDone = true;
     suite.addExpectationResult(false, data, true);
 
-    expect(expectationResultFactory).toHaveBeenCalledWith(data);
+    expect(onLateError).toHaveBeenCalledWith(
-    expect(onLateError).toHaveBeenCalledWith('built expectation result');
+      jasmine.objectContaining({
+        message: jasmine.stringMatching(/^Error: nope/)
+      })
+    );
     expect(suite.result.failedExpectations).toEqual([]);
   });
 
   it('does not forward non-late expectation failures to onLateError', function() {
     const onLateError = jasmine.createSpy('onLateError');
     const suite = new jasmineUnderTest.Suite({
-      expectationResultFactory: r => r,
       onLateError
     });
     const data = {
@@ -206,11 +197,7 @@ describe('Suite', function() {
 
   it('forwards late handleException calls to onLateError', function() {
     const onLateError = jasmine.createSpy('onLateError');
-    const expectationResultFactory = jasmine
-      .createSpy('expectationResultFactory')
-      .and.returnValue('built expectation result');
     const suite = new jasmineUnderTest.Suite({
-      expectationResultFactory,
       onLateError
     });
     const error = new Error('oops');
@@ -218,21 +205,17 @@ describe('Suite', function() {
     suite.reportedDone = true;
     suite.handleException(error);
 
-    expect(expectationResultFactory).toHaveBeenCalledWith({
+    expect(onLateError).toHaveBeenCalledWith(
-      matcherName: '',
+      jasmine.objectContaining({
-      passed: false,
+        message: jasmine.stringMatching(/^Error: oops/)
-      expected: '',
+      })
-      actual: '',
+    );
-      error
-    });
-    expect(onLateError).toHaveBeenCalledWith('built expectation result');
     expect(suite.result.failedExpectations).toEqual([]);
   });
 
   it('does not forward non-late handleException calls to onLateError', function() {
     const onLateError = jasmine.createSpy('onLateError');
     const suite = new jasmineUnderTest.Suite({
-      expectationResultFactory: r => r,
       onLateError
     });
     const error = new Error('oops');
@@ -245,7 +228,6 @@ describe('Suite', function() {
 
   it('clears the reportedDone flag when reset', function() {
     const suite = new jasmineUnderTest.Suite({
-      expectationResultFactory: r => r,
       queueableFn: { fn: function() {} }
     });
     suite.reportedDone = true;
@@ -346,11 +328,7 @@ describe('Suite', function() {
     });
 
     it('should reset the failedExpectations', function() {
-      const suite = new jasmineUnderTest.Suite({
+      const suite = new jasmineUnderTest.Suite({});
-        expectationResultFactory: function(error) {
-          return error;
-        }
-      });
       suite.handleException(new Error());
 
       suite.reset();

spec/core/buildExpectationResultSpec.js

@@ -22,56 +22,38 @@ describe('buildExpectationResult', function() {
     expect(result.message).toBe('some-value');
   });
 
-  it('delegates message formatting to the provided formatter if there was an Error', function() {
+  describe('When the error property is provided', function() {
-    const fakeError = { message: 'foo' },
+    it('sets the message to the formatted error', function() {
-      messageFormatter = jasmine
+      const result = jasmineUnderTest.buildExpectationResult({
-        .createSpy('exception message formatter')
+        passed: false,
-        .and.returnValue(fakeError.message);
+        error: { message: 'foo', fileName: 'somefile.js' }
+      });
 
-    const result = jasmineUnderTest.buildExpectationResult({
+      expect(result.message).toEqual('foo in somefile.js');
-      passed: false,
-      error: fakeError,
-      messageFormatter: messageFormatter
     });
 
-    expect(messageFormatter).toHaveBeenCalledWith(fakeError);
+    it('delegates stack formatting to the provided formatter', function() {
-    expect(result.message).toEqual('foo');
+      const result = jasmineUnderTest.buildExpectationResult({
+        passed: false,
+        error: { stack: 'foo', extra: 'wombat' }
+      });
+
+      expect(result.stack).toEqual(
+        "error properties: Object({ extra: 'wombat' })\nfoo"
+      );
+    });
   });
 
-  it('delegates stack formatting to the provided formatter if there was an Error', function() {
+  describe('When the errorForStack property is provided', function() {
-    const fakeError = { stack: 'foo' },
+    it('builds the stack trace using errorForStack instead of Error', function() {
-      stackFormatter = jasmine
+      const result = jasmineUnderTest.buildExpectationResult({
-        .createSpy('stack formatter')
+        passed: false,
-        .and.returnValue(fakeError.stack);
+        errorForStack: { stack: 'foo' },
+        error: { stack: 'bar' }
+      });
 
-    const result = jasmineUnderTest.buildExpectationResult({
+      expect(result.stack).toEqual('bar');
-      passed: false,
-      error: fakeError,
-      stackFormatter: stackFormatter
     });
-
-    expect(stackFormatter).toHaveBeenCalledWith(fakeError, {
-      omitMessage: true
-    });
-    expect(result.stack).toEqual('foo');
-  });
-
-  it('delegates stack formatting to the provided formatter if there was a provided errorForStack', function() {
-    const fakeError = { stack: 'foo' },
-      stackFormatter = jasmine
-        .createSpy('stack formatter')
-        .and.returnValue(fakeError.stack);
-
-    const result = jasmineUnderTest.buildExpectationResult({
-      passed: false,
-      errorForStack: fakeError,
-      stackFormatter: stackFormatter
-    });
-
-    expect(stackFormatter).toHaveBeenCalledWith(fakeError, {
-      omitMessage: true
-    });
-    expect(result.stack).toEqual('foo');
   });
 
   it('matcherName returns passed matcherName', function() {

src/core/Env.js

@@ -349,7 +349,7 @@ getJasmineRequireObj().Env = function(j$) {
         error.matcherName !== undefined && error.passed !== undefined;
       const result = isExpectationResult
         ? error
-        : expectationResultFactory({
+        : j$.buildExpectationResult({
             error,
             passed: false,
             matcherName: '',
@@ -499,16 +499,6 @@ getJasmineRequireObj().Env = function(j$) {
       return fullName.join(' ');
     };
 
-    // TODO: we may just be able to pass in the fn instead of wrapping here
-    var buildExpectationResult = j$.buildExpectationResult,
-      exceptionFormatter = new j$.ExceptionFormatter(),
-      expectationResultFactory = function(attrs) {
-        attrs.messageFormatter = exceptionFormatter.message;
-        attrs.stackFormatter = exceptionFormatter.stack;
-
-        return buildExpectationResult(attrs);
-      };
-
     /**
      * Causes a deprecation warning to be logged to the console and reported to
      * reporters.
@@ -574,7 +564,6 @@ getJasmineRequireObj().Env = function(j$) {
       description: 'Jasmine__TopLevel__Suite',
       expectationFactory: expectationFactory,
       asyncExpectationFactory: suiteAsyncExpectationFactory,
-      expectationResultFactory: expectationResultFactory,
       autoCleanClosures: config.autoCleanClosures,
       onLateError: recordLateError
     });
@@ -1033,7 +1022,6 @@ getJasmineRequireObj().Env = function(j$) {
         timer: new j$.Timer(),
         expectationFactory: expectationFactory,
         asyncExpectationFactory: suiteAsyncExpectationFactory,
-        expectationResultFactory: expectationResultFactory,
         throwOnExpectationFailure: config.stopSpecOnExpectationFailure,
         autoCleanClosures: config.autoCleanClosures,
         onLateError: recordLateError
@@ -1142,7 +1130,6 @@ getJasmineRequireObj().Env = function(j$) {
         },
         onStart: specStarted,
         description: description,
-        expectationResultFactory: expectationResultFactory,
         queueRunnerFactory: queueRunnerFactory,
         userContext: function() {
           return suite.clonedSharedUserContext();

src/core/Spec.js

@@ -43,8 +43,6 @@ getJasmineRequireObj().Spec = function(j$) {
       function() {
         return '';
       };
-    this.expectationResultFactory =
-      attrs.expectationResultFactory || function() {};
     this.onLateError = attrs.onLateError || function() {};
     this.queueRunnerFactory = attrs.queueRunnerFactory || function() {};
     this.catchingExceptions =
@@ -91,7 +89,7 @@ getJasmineRequireObj().Spec = function(j$) {
   }
 
   Spec.prototype.addExpectationResult = function(passed, data, isError) {
-    const expectationResult = this.expectationResultFactory(data);
+    const expectationResult = j$.buildExpectationResult(data);
     if (passed) {
       this.result.passedExpectations.push(expectationResult);
     } else {
@@ -296,7 +294,7 @@ getJasmineRequireObj().Spec = function(j$) {
       deprecation = { message: deprecation };
     }
     this.result.deprecationWarnings.push(
-      this.expectationResultFactory(deprecation)
+      j$.buildExpectationResult(deprecation)
     );
   };
 

src/core/Suite.js

@@ -25,7 +25,6 @@ getJasmineRequireObj().Suite = function(j$) {
     this.description = attrs.description;
     this.expectationFactory = attrs.expectationFactory;
     this.asyncExpectationFactory = attrs.asyncExpectationFactory;
-    this.expectationResultFactory = attrs.expectationResultFactory;
     this.throwOnExpectationFailure = !!attrs.throwOnExpectationFailure;
     this.autoCleanClosures =
       attrs.autoCleanClosures === undefined ? true : !!attrs.autoCleanClosures;
@@ -210,14 +209,14 @@ getJasmineRequireObj().Suite = function(j$) {
       return;
     }
 
-    var data = {
+    const data = {
       matcherName: '',
       passed: false,
       expected: '',
       actual: '',
       error: arguments[0]
     };
-    var failedExpectation = this.expectationResultFactory(data);
+    const failedExpectation = j$.buildExpectationResult(data);
 
     if (!this.parentSuite) {
       failedExpectation.globalErrorType = 'afterAll';
@@ -255,7 +254,7 @@ getJasmineRequireObj().Suite = function(j$) {
   Suite.prototype.addExpectationResult = function() {
     if (isFailure(arguments)) {
       const data = arguments[1];
-      const expectationResult = this.expectationResultFactory(data);
+      const expectationResult = j$.buildExpectationResult(data);
 
       if (this.reportedDone) {
         this.onLateError(expectationResult);
@@ -274,7 +273,7 @@ getJasmineRequireObj().Suite = function(j$) {
       deprecation = { message: deprecation };
     }
     this.result.deprecationWarnings.push(
-      this.expectationResultFactory(deprecation)
+      j$.buildExpectationResult(deprecation)
     );
   };
 

src/core/buildExpectationResult.js

@@ -1,8 +1,7 @@
 //TODO: expectation result may make more sense as a presentation of an expectation.
 getJasmineRequireObj().buildExpectationResult = function(j$) {
   function buildExpectationResult(options) {
-    const messageFormatter = options.messageFormatter || function() {};
+    const exceptionFormatter = new j$.ExceptionFormatter();
-    const stackFormatter = options.stackFormatter || function() {};
 
     /**
      * @typedef Expectation
@@ -52,7 +51,7 @@ getJasmineRequireObj().buildExpectationResult = function(j$) {
       } else if (options.message) {
         return options.message;
       } else if (options.error) {
-        return messageFormatter(options.error);
+        return exceptionFormatter.message(options.error);
       }
       return '';
     }
@@ -79,7 +78,7 @@ getJasmineRequireObj().buildExpectationResult = function(j$) {
       }
       // Omit the message from the stack trace because it will be
       // included elsewhere.
-      return stackFormatter(error, { omitMessage: true });
+      return exceptionFormatter.stack(error, { omitMessage: true });
     }
   }