Suite level errors all report the same way (on suiteDone)

- For `beforeAll`, `afterAll`, and declaration errors

[#150118881] #1409

Signed-off-by: Gregg Van Hove <gvanhove@pivotal.io>

spec/core/SuiteSpec.js

@@ -67,11 +67,10 @@ describe("Suite", function() {
     expect(suite.afterFns).toEqual([innerAfter, outerAfter]);
   });
 
-  it('has a status of failed if any afterAll expectations have failed', function() {
+  it('has a status of failed if any expectations have failed', function() {
     var suite = new jasmineUnderTest.Suite({
       expectationResultFactory: function() { return 'hi'; }
     });
-    suite.addChild({ result: { status: 'done' } });
 
     suite.addExpectationResult(false);
     expect(suite.status()).toBe('failed');
@@ -103,28 +102,11 @@ describe("Suite", function() {
     expect(suite.isExecutable()).toBe(false);
   });
 
-  it("tells all children about expectation failures, even if one throws", function() {
+  it("throws an ExpectationFailed when receiving a failed expectation when throwOnExpectationFailure is set", function() {
-    var suite = new jasmineUnderTest.Suite({}),
-      child1 = { addExpectationResult: jasmine.createSpy('child1#expectationResult'), result: {} },
-      child2 = { addExpectationResult: jasmine.createSpy('child2#expectationResult'), result: {} };
-
-    suite.addChild(child1);
-    suite.addChild(child2);
-
-    child1.addExpectationResult.and.throwError('foo');
-
-    suite.addExpectationResult('stuff');
-
-    expect(child1.addExpectationResult).toHaveBeenCalledWith('stuff');
-    expect(child2.addExpectationResult).toHaveBeenCalledWith('stuff');
-  });
-
-  it("throws an ExpectationFailed when receiving a failed expectation in an afterAll when throwOnExpectationFailure is set", function() {
     var suite = new jasmineUnderTest.Suite({
       expectationResultFactory: function(data) { return data; },
       throwOnExpectationFailure: true
     });
-    suite.addChild({ result: { status: 'done' } });
 
     expect(function() {
       suite.addExpectationResult(false, 'failed');
@@ -134,9 +116,8 @@ describe("Suite", function() {
     expect(suite.result.failedExpectations).toEqual(['failed']);
   });
 
-  it("does not add an additional failure when an expectation fails in an afterAll", function(){
+  it("does not add an additional failure when an expectation fails", function(){
     var suite = new jasmineUnderTest.Suite({});
-    suite.addChild({ result: { status: 'done' } });
 
     suite.onException(new jasmineUnderTest.errors.ExpectationFailed());
 

spec/core/integration/EnvSpec.js

@@ -1,43 +1,6 @@
 describe("Env integration", function() {
   beforeEach(function() {
-    jasmine.addMatchers({
+    jasmine.getEnv().registerIntegrationMatchers();
-      toHaveFailedExpecationsForRunnable: function(util, customeEqualityTesters) {
-        return {
-          compare: function(actual, fullName, expectedFailures) {
-            var foundRunnable = false, expectations = true, foundFailures = [];
-            for (var i = 0; i < actual.calls.count(); i++) {
-              var args = actual.calls.argsFor(i)[0];
-
-              if (args.fullName === fullName) {
-                foundRunnable = true;
-
-                for (var j = 0; j < args.failedExpectations.length; j++) {
-                  foundFailures.push(args.failedExpectations[j].message);
-                }
-
-                for (var j = 0; j < expectedFailures.length; j++) {
-                  var failure = foundFailures[j];
-                  var expectedFailure = expectedFailures[j];
-
-                  if (Object.prototype.toString.call(expectedFailure) === '[object RegExp]') {
-                    expectations = expectations && expectedFailure.test(failure);
-                  } else {
-                    expectations = expectations && failure === expectedFailure;
-                  }
-                }
-                break;
-              }
-            }
-
-            return {
-              pass: foundRunnable && expectations,
-              message: !foundRunnable ? 'The runnable "' + fullName + '" never finished' :
-                'Expected runnable "' + fullName + '" to have failures ' + jasmine.pp(expectedFailures) + ' but it had ' + jasmine.pp(foundFailures)
-            };
-          }
-        };
-      }
-    });
   });
 
   it("Suites execute as expected (no nesting)", function(done) {
@@ -464,22 +427,16 @@ describe("Env integration", function() {
     env.execute();
   });
 
-  it("fails all underlying specs when the beforeAll fails", function (done) {
+  it("when the beforeAll fails, error at suite level", function (done) {
     var env = new jasmineUnderTest.Env(),
-      reporter = jasmine.createSpyObj('fakeReporter', [ "specDone", "jasmineDone" ]);
+      reporter = jasmine.createSpyObj('fakeReporter', [ "specDone", "suiteDone", "jasmineDone" ]);
 
     reporter.jasmineDone.and.callFake(function() {
       expect(reporter.specDone.calls.count()).toEqual(2);
+      expect(reporter.specDone).toHaveFailedExpectationsForRunnable('A suite spec that will pass', []);
+      expect(reporter.specDone).toHaveFailedExpectationsForRunnable('A suite nesting another spec to pass', []);
+      expect(reporter.suiteDone).toHaveFailedExpectationsForRunnable('A suite', ['Expected 1 to be 2.']);
 
-      expect(reporter.specDone.calls.argsFor(0)[0])
-        .toEqual(jasmine.objectContaining({status: 'failed'}));
-      expect(reporter.specDone.calls.argsFor(0)[0].failedExpectations[0].message)
-        .toEqual("Expected 1 to be 2.");
-
-      expect(reporter.specDone.calls.argsFor(1)[0])
-        .toEqual(jasmine.objectContaining({status: 'failed'}));
-      expect(reporter.specDone.calls.argsFor(1)[0].failedExpectations[0].message)
-        .toEqual("Expected 1 to be 2.");
       done();
     });
 
@@ -490,11 +447,11 @@ describe("Env integration", function() {
         env.expect(1).toBe(2);
       });
 
-      env.it("spec that will be failed", function() {
+      env.it("spec that will pass", function() {
       });
 
       env.describe("nesting", function() {
-        env.it("another spec to fail", function() {
+        env.it("another spec to pass", function() {
         });
       });
     });
@@ -512,8 +469,8 @@ describe("Env integration", function() {
       reporter = jasmine.createSpyObj('fakeReporter', [ "specDone", "jasmineDone", "suiteDone" ]);
 
     reporter.jasmineDone.and.callFake(function() {
-      expect(reporter.specDone).not.toHaveFailedExpecationsForRunnable('A suite fails', ['fail thrown']);
+      expect(reporter.specDone).not.toHaveFailedExpectationsForRunnable('A suite fails', ['fail thrown']);
-      expect(reporter.suiteDone).toHaveFailedExpecationsForRunnable('A suite', ['fail thrown']);
+      expect(reporter.suiteDone).toHaveFailedExpectationsForRunnable('A suite', ['fail thrown']);
       done();
     });
 
@@ -543,7 +500,7 @@ describe("Env integration", function() {
         reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','suiteDone']);
 
       reporter.jasmineDone.and.callFake(function() {
-        expect(reporter.suiteDone).toHaveFailedExpecationsForRunnable('my suite', [
+        expect(reporter.suiteDone).toHaveFailedExpectationsForRunnable('my suite', [
           'Expected 1 to equal 2.',
           'Expected 2 to equal 3.'
         ]);
@@ -570,7 +527,7 @@ describe("Env integration", function() {
         reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','suiteDone']);
 
       reporter.jasmineDone.and.callFake(function() {
-        expect(reporter.suiteDone).toHaveFailedExpecationsForRunnable('outer suite', [
+        expect(reporter.suiteDone).toHaveFailedExpectationsForRunnable('outer suite', [
           'Expected 1 to equal 2.',
           'Expected 2 to equal 3.'
         ]);
@@ -599,7 +556,7 @@ describe("Env integration", function() {
         reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','suiteDone']);
 
       reporter.jasmineDone.and.callFake(function() {
-        expect(reporter.suiteDone).toHaveFailedExpecationsForRunnable('my suite', [
+        expect(reporter.suiteDone).toHaveFailedExpectationsForRunnable('my suite', [
           (/^Error: After All Exception/)
         ]);
         done();
@@ -624,7 +581,7 @@ describe("Env integration", function() {
         reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','suiteDone']);
 
       reporter.jasmineDone.and.callFake(function() {
-        expect(reporter.suiteDone).toHaveFailedExpecationsForRunnable('my suite', [
+        expect(reporter.suiteDone).toHaveFailedExpectationsForRunnable('my suite', [
           'Expected 1 to equal 2.'
         ]);
         done();
@@ -652,7 +609,7 @@ describe("Env integration", function() {
 
 
       reporter.jasmineDone.and.callFake(function() {
-        expect(reporter.suiteDone).toHaveFailedExpecationsForRunnable('my suite', [
+        expect(reporter.suiteDone).toHaveFailedExpectationsForRunnable('my suite', [
           (/^Error: After All Exception/)
         ]);
         done();
@@ -673,15 +630,13 @@ describe("Env integration", function() {
     });
   });
 
-  it('cascades expecatation failures in global beforeAll down to children', function(done) {
+  it('reports expectation failures in global beforeAll', function(done) {
     var env = new jasmineUnderTest.Env(),
       reporter = jasmine.createSpyObj(['specDone', 'jasmineDone']);
 
     reporter.jasmineDone.and.callFake(function(results) {
-      expect(results.failedExpectations).toEqual([]);
+      expect(results.failedExpectations).toEqual([jasmine.objectContaining({ message: 'Expected 1 to be 0.' })]);
-      expect(reporter.specDone).toHaveFailedExpecationsForRunnable('is a spec', [
+      expect(reporter.specDone).toHaveFailedExpectationsForRunnable('is a spec', []);
-        'Expected 1 to be 0.'
-      ]);
       done();
     });
 
@@ -1041,35 +996,6 @@ describe("Env integration", function() {
       env.execute();
     });
 
-    it("should wait a specified interval before failing beforeAll's and their associated specs that haven't called done", function(done) {
-      var env = new jasmineUnderTest.Env(),
-        reporter = jasmine.createSpyObj('fakeReporter', [ "specDone", "jasmineDone" ]);
-
-      reporter.jasmineDone.and.callFake(function() {
-        expect(reporter.specDone.calls.count()).toEqual(2);
-        expect(reporter.specDone.calls.argsFor(0)[0]).toEqual(jasmine.objectContaining({status: 'failed'}));
-        expect(reporter.specDone.calls.argsFor(1)[0]).toEqual(jasmine.objectContaining({status: 'failed'}));
-        done();
-      });
-
-      env.addReporter(reporter);
-      jasmineUnderTest.DEFAULT_TIMEOUT_INTERVAL = 1290;
-
-      env.beforeAll(function(innerDone) {
-        jasmine.clock().tick(1291);
-      });
-
-      env.it("spec that will be failed", function() {
-      });
-
-      env.describe("nesting", function() {
-        env.it("another spec to fail", function() {
-        });
-      });
-
-      env.execute();
-    });
-
     it("should not use the mock clock for asynchronous timeouts", function(done){
       var env = new jasmineUnderTest.Env(),
         reporter = jasmine.createSpyObj('fakeReporter', [ "specDone", "jasmineDone" ]),
@@ -1102,59 +1028,18 @@ describe("Env integration", function() {
       env.execute();
     });
 
-    it("should wait the specified interval before reporting an afterAll that fails to call done", function(done) {
-      var env = new jasmineUnderTest.Env(),
-      reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','suiteDone']);
-
-      reporter.jasmineDone.and.callFake(function() {
-        expect(reporter.suiteDone).toHaveFailedExpecationsForRunnable('my suite', [
-          (/^Error: Timeout - Async callback was not invoked within timeout specified by jasmine\.DEFAULT_TIMEOUT_INTERVAL\./)
-        ]);
-        done();
-      });
-
-      env.addReporter(reporter);
-      jasmineUnderTest.DEFAULT_TIMEOUT_INTERVAL = 3000;
-
-      env.describe('my suite', function() {
-        env.it('my spec', function() {
-        });
-
-        env.afterAll(function(innerDone) {
-          jasmine.clock().tick(3001);
-          innerDone();
-        });
-      });
-
-      env.execute();
-      jasmine.clock().tick(1);
-    });
-
     it('should wait a custom interval before reporting async functions that fail to call done', function(done) {
       var env = new jasmineUnderTest.Env(),
           reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone', 'suiteDone', 'specDone']),
-          realSetTimeout = this.realSetTimeout;
+          realSetTimeout = this.realSetTimeout,
+          timeoutFailure = (/^Error: Timeout - Async callback was not invoked within timeout specified by jasmine\.DEFAULT_TIMEOUT_INTERVAL\./);
 
       reporter.jasmineDone.and.callFake(function() {
-        expect(reporter.specDone).toHaveFailedExpecationsForRunnable('suite beforeAll times out', [
+        expect(reporter.suiteDone).toHaveFailedExpectationsForRunnable('suite beforeAll', [ timeoutFailure ]);
-          (/^Error: Timeout - Async callback was not invoked within timeout specified by jasmine\.DEFAULT_TIMEOUT_INTERVAL\./)
+        expect(reporter.suiteDone).toHaveFailedExpectationsForRunnable('suite afterAll', [ timeoutFailure ]);
-        ]);
+        expect(reporter.specDone).toHaveFailedExpectationsForRunnable('suite beforeEach times out', [ timeoutFailure ]);
-
+        expect(reporter.specDone).toHaveFailedExpectationsForRunnable('suite afterEach times out', [ timeoutFailure ]);
-        expect(reporter.suiteDone).toHaveFailedExpecationsForRunnable('suite afterAll', [
+        expect(reporter.specDone).toHaveFailedExpectationsForRunnable('suite it times out', [ timeoutFailure ]);
-          (/^Error: Timeout - Async callback was not invoked within timeout specified by jasmine\.DEFAULT_TIMEOUT_INTERVAL\./)
-        ]);
-
-        expect(reporter.specDone).toHaveFailedExpecationsForRunnable('suite beforeEach times out', [
-          (/^Error: Timeout - Async callback was not invoked within timeout specified by jasmine\.DEFAULT_TIMEOUT_INTERVAL\./)
-        ]);
-
-        expect(reporter.specDone).toHaveFailedExpecationsForRunnable('suite afterEach times out', [
-          (/^Error: Timeout - Async callback was not invoked within timeout specified by jasmine\.DEFAULT_TIMEOUT_INTERVAL\./)
-        ]);
-
-        expect(reporter.specDone).toHaveFailedExpecationsForRunnable('suite it times out', [
-          (/^Error: Timeout - Async callback was not invoked within timeout specified by jasmine\.DEFAULT_TIMEOUT_INTERVAL\./)
-        ]);
 
         done();
       });
@@ -1965,10 +1850,10 @@ describe("Env integration", function() {
         reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','suiteDone','specDone']);
 
     reporter.jasmineDone.and.callFake(function() {
-      expect(reporter.suiteDone).toHaveFailedExpecationsForRunnable('async suite', [
+      expect(reporter.suiteDone).toHaveFailedExpectationsForRunnable('async suite', [
         /^(((Uncaught )?Error: suite( thrown)?)|(suite thrown))$/
       ]);
-      expect(reporter.specDone).toHaveFailedExpecationsForRunnable('suite async spec', [
+      expect(reporter.specDone).toHaveFailedExpectationsForRunnable('suite async spec', [
         /^(((Uncaught )?Error: spec( thrown)?)|(spec thrown))$/
       ]);
       done();
@@ -2000,19 +1885,19 @@ describe("Env integration", function() {
     reporter.jasmineDone.and.callFake(function() {
       var msg = /\'.*\' should only be used in \'describe\' function/;
 
-      expect(reporter.specDone).toHaveFailedExpecationsForRunnable('suite describe', [msg]);
+      expect(reporter.specDone).toHaveFailedExpectationsForRunnable('suite describe', [msg]);
-      expect(reporter.specDone).toHaveFailedExpecationsForRunnable('suite xdescribe', [msg]);
+      expect(reporter.specDone).toHaveFailedExpectationsForRunnable('suite xdescribe', [msg]);
-      expect(reporter.specDone).toHaveFailedExpecationsForRunnable('suite fdescribe', [msg]);
+      expect(reporter.specDone).toHaveFailedExpectationsForRunnable('suite fdescribe', [msg]);
 
-      expect(reporter.specDone).toHaveFailedExpecationsForRunnable('spec it', [msg]);
+      expect(reporter.specDone).toHaveFailedExpectationsForRunnable('spec it', [msg]);
-      expect(reporter.specDone).toHaveFailedExpecationsForRunnable('spec xit', [msg]);
+      expect(reporter.specDone).toHaveFailedExpectationsForRunnable('spec xit', [msg]);
-      expect(reporter.specDone).toHaveFailedExpecationsForRunnable('spec fit', [msg]);
+      expect(reporter.specDone).toHaveFailedExpectationsForRunnable('spec fit', [msg]);
 
-      expect(reporter.specDone).toHaveFailedExpecationsForRunnable('beforeAll spec', [msg]);
+      expect(reporter.suiteDone).toHaveFailedExpectationsForRunnable('beforeAll', [msg]);
-      expect(reporter.specDone).toHaveFailedExpecationsForRunnable('beforeEach spec', [msg]);
+      expect(reporter.specDone).toHaveFailedExpectationsForRunnable('beforeEach spec', [msg]);
 
-      expect(reporter.suiteDone).toHaveFailedExpecationsForRunnable('afterAll', [msg]);
+      expect(reporter.suiteDone).toHaveFailedExpectationsForRunnable('afterAll', [msg]);
-      expect(reporter.specDone).toHaveFailedExpecationsForRunnable('afterEach spec', [msg]);
+      expect(reporter.specDone).toHaveFailedExpectationsForRunnable('afterEach spec', [msg]);
 
       done();
     });

spec/core/integration/SpecRunningSpec.js

@@ -2,6 +2,7 @@ describe("spec running", function () {
   var env;
 
   beforeEach(function() {
+    jasmine.getEnv().registerIntegrationMatchers();
     env = new jasmineUnderTest.Env();
     env.randomizeTests(false);
   });
@@ -601,18 +602,16 @@ describe("spec running", function () {
 
   it("should recover gracefully when there are errors in describe functions", function(done) {
     var specs = [],
-      reporter = jasmine.createSpyObj(['specDone', 'jasmineDone']);
+      reporter = jasmine.createSpyObj(['specDone', 'suiteDone', 'jasmineDone']);
 
     reporter.specDone.and.callFake(function(result) {
       specs.push(result.fullName);
     });
 
     reporter.jasmineDone.and.callFake(function() {
-      expect(specs).toContain('outer1 inner1 should thingy');
+      expect(specs).toEqual(['outer1 inner1 should thingy', 'outer1 inner2 should other thingy', 'outer2 should xxx']);
-      expect(specs).toContain('outer1 inner1 encountered a declaration exception');
+      expect(reporter.suiteDone).toHaveFailedExpectationsForRunnable('outer1 inner1', [/inner error/]);
-      expect(specs).toContain('outer1 inner2 should other thingy');
+      expect(reporter.suiteDone).toHaveFailedExpectationsForRunnable('outer1', [/outer error/]);
-      expect(specs).toContain('outer1 encountered a declaration exception');
-      expect(specs).toContain('outer2 should xxx');
       done();
     });
 
@@ -623,7 +622,7 @@ describe("spec running", function () {
             this.expect(true).toEqual(true);
           });
 
-          throw new Error("fake error");
+          throw new Error("inner error");
         });
 
         env.describe("inner2", function() {
@@ -632,7 +631,7 @@ describe("spec running", function () {
           });
         });
 
-        throw new Error("fake error");
+        throw new Error("outer error");
 
       });
     }).not.toThrow();

spec/helpers/integrationMatchers.js

@@ -0,0 +1,43 @@
+(function(env) {
+  env.registerIntegrationMatchers = function() {
+    jasmine.addMatchers({
+      toHaveFailedExpectationsForRunnable: function (util, customeEqualityTesters) {
+        return {
+          compare: function (actual, fullName, expectedFailures) {
+            var foundRunnable = false, expectations = true, foundFailures = [];
+            for (var i = 0; i < actual.calls.count(); i++) {
+              var args = actual.calls.argsFor(i)[0];
+
+              if (args.fullName === fullName) {
+                foundRunnable = true;
+
+                for (var j = 0; j < args.failedExpectations.length; j++) {
+                  foundFailures.push(args.failedExpectations[j].message);
+                }
+
+                for (var j = 0; j < expectedFailures.length; j++) {
+                  var failure = foundFailures[j];
+                  var expectedFailure = expectedFailures[j];
+
+                  if (Object.prototype.toString.call(expectedFailure) === '[object RegExp]') {
+                    expectations = expectations && expectedFailure.test(failure);
+                  } else {
+                    expectations = expectations && failure === expectedFailure;
+                  }
+                }
+                break;
+              }
+            }
+
+            return {
+              pass: foundRunnable && expectations,
+              message: !foundRunnable ? 'The runnable "' + fullName + '" never finished' :
+                'Expected runnable "' + fullName + '" to have failures ' + jasmine.pp(expectedFailures) + ' but it had ' + jasmine.pp(foundFailures)
+            };
+          }
+        };
+      }
+    });
+  };
+})(jasmine.getEnv());
+

spec/support/jasmine.json

@@ -10,6 +10,7 @@
     "helpers/checkForSet.js",
     "helpers/checkForSymbol.js",
     "helpers/checkForTypedArrays.js",
+    "helpers/integrationMatchers.js",
     "helpers/nodeDefineJasmineUnderTest.js"
   ],
   "random": true

spec/support/jasmine.yml

@@ -23,6 +23,7 @@ helpers:
   - 'helpers/checkForSet.js'
   - 'helpers/checkForSymbol.js'
   - 'helpers/checkForTypedArrays.js'
+  - 'helpers/integrationMatchers.js'
   - 'helpers/defineJasmineUnderTest.js'
 spec_files:
   - '**/*[Ss]pec.js'

src/core/Env.js

@@ -526,9 +526,7 @@ getJasmineRequireObj().Env = function(j$) {
       }
 
       if (declarationError) {
-        self.it('encountered a declaration exception', function() {
+        suite.onException(declarationError);
-          throw declarationError;
-        });
       }
 
       currentDeclarationSuite = parentSuite;

src/core/Suite.js

@@ -124,52 +124,32 @@ getJasmineRequireObj().Suite = function(j$) {
       return;
     }
 
-    if(isAfterAll(this.children)) {
+    var data = {
-      var data = {
+      matcherName: '',
-        matcherName: '',
+      passed: false,
-        passed: false,
+      expected: '',
-        expected: '',
+      actual: '',
-        actual: '',
+      error: arguments[0]
-        error: arguments[0]
+    };
-      };
+    var failedExpectation = this.expectationResultFactory(data);
-      var failedExpectation = this.expectationResultFactory(data);
 
-      if (!this.parentSuite) {
+    if (!this.parentSuite) {
-        failedExpectation.globalErrorType = 'afterAll';
+      failedExpectation.globalErrorType = 'afterAll';
-      }
-
-      this.result.failedExpectations.push(failedExpectation);
-    } else {
-      for (var i = 0; i < this.children.length; i++) {
-        var child = this.children[i];
-        child.onException.apply(child, arguments);
-      }
     }
+
+    this.result.failedExpectations.push(failedExpectation);
   };
 
   Suite.prototype.addExpectationResult = function () {
-    if(isAfterAll(this.children) && isFailure(arguments)){
+    if(isFailure(arguments)) {
       var data = arguments[1];
       this.result.failedExpectations.push(this.expectationResultFactory(data));
       if(this.throwOnExpectationFailure) {
         throw new j$.errors.ExpectationFailed();
       }
-    } else {
-      for (var i = 0; i < this.children.length; i++) {
-        var child = this.children[i];
-        try {
-          child.addExpectationResult.apply(child, arguments);
-        } catch(e) {
-          // keep going
-        }
-      }
     }
   };
 
-  function isAfterAll(children) {
-    return children && children[0].result.status;
-  }
-
   function isFailure(args) {
     return !args[0];
   }