From 97867b2bf54259b49380da8f07097ff23dc5f500 Mon Sep 17 00:00:00 2001 From: Greg Cobb and Tim Jarratt Date: Tue, 26 Aug 2014 18:04:12 -0700 Subject: [PATCH] Reports expectation failures in afterAlls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes the specs green and appears to work for most cases. I have a number of concerns about the implementation and would appreciate ideas/feedback. - Suite#addExpecationResult infers if it is coming from an afterAll fn based on if the first child of the suite is finished. This assumes that the first child of the suite is a spec (this appears to be true as long as there is at least one spec in the suite) - Suites behave like unfinished specs. Because suites will propagate expectation failures to their children suites, the afterAll expectation reporting appears to work for suites without specs unless you have: 1) An otherwise empty suite with an afterAll 2) An afterAll'd suite whose first suite is empty (or whose first suite's first suite is empty (and so on)) - Changed afterAllError to afterAllEvent, so it can accommodate both errors and expectation failures. The reporter now receives a string instead of the actual error object. The loss of the object doesn't affect our reporters, but may be a nice-to-have for other reporters/ the future. - The gap between the expectations caught in Suite and QueueRunner (who triggers reporting via an injected callback) is an array injected into QR by the Suite. The array is then flushed at some point (currently after the attempt… functions). This works, but is a bit goofy. [#73741654] --- spec/console/ConsoleReporterSpec.js | 4 +- spec/core/QueueRunnerSpec.js | 2 +- spec/core/integration/EnvSpec.js | 57 +++++++++++++++++++++++------ spec/html/HtmlReporterSpec.js | 6 +-- src/console/ConsoleReporter.js | 6 +-- src/core/Env.js | 7 +++- src/core/QueueRunner.js | 18 +++++++-- src/core/Suite.js | 22 +++++++++-- src/html/HtmlReporter.js | 4 +- 9 files changed, 93 insertions(+), 33 deletions(-) diff --git a/spec/console/ConsoleReporterSpec.js b/spec/console/ConsoleReporterSpec.js index 2c1af8c1..cc407f97 100644 --- a/spec/console/ConsoleReporterSpec.js +++ b/spec/console/ConsoleReporterSpec.js @@ -229,8 +229,8 @@ describe("ConsoleReporter", function() { error = new Error('After All Exception'), anotherError = new Error('Some Other Exception'); - reporter.afterAllError(error); - reporter.afterAllError(anotherError); + reporter.afterAllEvent(error); + reporter.afterAllEvent(anotherError); reporter.jasmineDone(); expect(out.getOutput()).toMatch(/After All Exception/); diff --git a/spec/core/QueueRunnerSpec.js b/spec/core/QueueRunnerSpec.js index d715b7cb..217467e7 100644 --- a/spec/core/QueueRunnerSpec.js +++ b/spec/core/QueueRunnerSpec.js @@ -173,7 +173,7 @@ describe("QueueRunner", function() { queueRunner = new j$.QueueRunner({ queueableFns: [queueableFn], onComplete: onComplete, - reportException: reportException + reportException: reportException, onException: onException }); diff --git a/spec/core/integration/EnvSpec.js b/spec/core/integration/EnvSpec.js index 100b4091..f6871f1a 100644 --- a/spec/core/integration/EnvSpec.js +++ b/spec/core/integration/EnvSpec.js @@ -330,10 +330,11 @@ describe("Env integration", function() { it("reports when an afterAll fails an expectation", function(done) { var env = new j$.Env(), - reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','afterAllError']); + reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','afterAllEvent']); reporter.jasmineDone.and.callFake(function() { - expect(reporter.afterAllError).toHaveBeenCalled(); + expect(reporter.afterAllEvent).toHaveBeenCalledWith('Expectation failed: Expected 1 to equal 2.'); + expect(reporter.afterAllEvent).toHaveBeenCalledWith('Expectation failed: Expected 2 to equal 3.'); done(); }); @@ -343,6 +344,40 @@ describe("Env integration", function() { env.it('my spec', function() { }); + env.afterAll(function() { + env.expect(1).toEqual(2); + env.expect(2).toEqual(3); + }); + }); + + env.execute(); + }); + + + it("only reports afterAll expectation failures once, regardless of suite children", function(done) { + var env = new j$.Env(), + reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','afterAllEvent']); + + reporter.jasmineDone.and.callFake(function() { + expect(reporter.afterAllEvent.calls.count()).toEqual(1); + expect(reporter.afterAllEvent).toHaveBeenCalledWith('Expectation failed: Expected 1 to equal 2.'); + done(); + }); + + env.addReporter(reporter); + + env.describe('my suite', function() { + env.it('my spec', function() { + }); + + env.it('my spec2', function() { + }); + + env.describe('nested suite', function(){ + env.it('my spec3', function() { + }); + }); + env.afterAll(function() { env.expect(1).toEqual(2); }); @@ -354,11 +389,11 @@ describe("Env integration", function() { it("reports when afterAll throws an exception", function(done) { var env = new j$.Env(), error = new Error('After All Exception'), - reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','afterAllError']); - + reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','afterAllEvent']); reporter.jasmineDone.and.callFake(function() { - expect(reporter.afterAllError).toHaveBeenCalledWith(error); + expect(reporter.afterAllEvent.calls.count()).toEqual(1); + expect(reporter.afterAllEvent).toHaveBeenCalledWith('Error thrown: After All Exception'); done(); }); @@ -378,10 +413,10 @@ describe("Env integration", function() { it("reports when an async afterAll fails an expectation", function(done) { var env = new j$.Env(), - reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','afterAllError']); + reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','afterAllEvent']); reporter.jasmineDone.and.callFake(function() { - expect(reporter.afterAllError).toHaveBeenCalled(); + expect(reporter.afterAllEvent).toHaveBeenCalledWith('Expectation failed: Expected 1 to equal 2.'); done(); }); @@ -403,11 +438,11 @@ describe("Env integration", function() { it("reports when an async afterAll throws an exception", function(done) { var env = new j$.Env(), error = new Error('After All Exception'), - reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','afterAllError']); + reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','afterAllEvent']); reporter.jasmineDone.and.callFake(function() { - expect(reporter.afterAllError).toHaveBeenCalled(); + expect(reporter.afterAllEvent).toHaveBeenCalled(); done(); }); @@ -639,10 +674,10 @@ describe("Env integration", function() { it("should wait the specified interval before reporting an afterAll that fails to call done", function(done) { var env = new j$.Env(), - reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','afterAllError']); + reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone','afterAllEvent']); reporter.jasmineDone.and.callFake(function() { - expect(reporter.afterAllError).toHaveBeenCalledWith(jasmine.any(Error)); + expect(reporter.afterAllEvent).toHaveBeenCalledWith(jasmine.any(String)); done(); }); diff --git a/spec/html/HtmlReporterSpec.js b/spec/html/HtmlReporterSpec.js index 87cd80f0..ce3b526d 100644 --- a/spec/html/HtmlReporterSpec.js +++ b/spec/html/HtmlReporterSpec.js @@ -130,7 +130,7 @@ describe("New HtmlReporter", function() { }); }); - describe("when there are afterAllErrors", function () { + describe("when there are afterAllEvents", function () { it("displays the exceptions in their own alert bars", function(){ var env = new j$.Env(), container = document.createElement("div"), @@ -147,8 +147,8 @@ describe("New HtmlReporter", function() { reporter.initialize(); reporter.jasmineStarted({}); - reporter.afterAllError(error); - reporter.afterAllError(otherError); + reporter.afterAllEvent(error); + reporter.afterAllEvent(otherError); reporter.jasmineDone({}); var alertBars = container.querySelectorAll(".alert .bar"); diff --git a/src/console/ConsoleReporter.js b/src/console/ConsoleReporter.js index b40b86d5..a24f80ca 100644 --- a/src/console/ConsoleReporter.js +++ b/src/console/ConsoleReporter.js @@ -54,9 +54,7 @@ getJasmineRequireObj().ConsoleReporter = function() { for(i = 0; i < exceptionList.length; i++) { printNewline(); - print(colored('red', 'An error was thrown in an afterAll')); - printNewline(); - print(colored('red', (exceptionList[i].message || exceptionList[i].description))); + print(colored('red', 'AfterAll ' + exceptionList[i])); printNewline(); } @@ -84,7 +82,7 @@ getJasmineRequireObj().ConsoleReporter = function() { } }; - this.afterAllError = function(error) { + this.afterAllEvent = function(error) { exceptionList.push(error); }; diff --git a/src/core/Env.js b/src/core/Env.js index 165241ad..3fb56eee 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -35,7 +35,7 @@ getJasmineRequireObj().Env = function(j$) { 'suiteDone', 'specStarted', 'specDone', - 'afterAllError' + 'afterAllEvent' ]); this.specFilter = function() { @@ -170,9 +170,12 @@ getJasmineRequireObj().Env = function(j$) { options.timer = {setTimeout: realSetTimeout, clearTimeout: realClearTimeout}; options.reportException = function(e, type) { if (type === 'afterAll') { - reporter.afterAllError(e); + reporter.afterAllEvent('Error thrown: '+ (e.message || e.description)); } }; + options.reportExpectationFailure = function(message) { + reporter.afterAllEvent('Expectation failed: '+ message); + }; new j$.QueueRunner(options).execute(); }; diff --git a/src/core/QueueRunner.js b/src/core/QueueRunner.js index a0facf84..8a60969b 100644 --- a/src/core/QueueRunner.js +++ b/src/core/QueueRunner.js @@ -19,6 +19,8 @@ getJasmineRequireObj().QueueRunner = function(j$) { this.userContext = attrs.userContext || {}; this.timer = attrs.timeout || {setTimeout: setTimeout, clearTimeout: clearTimeout}; this.reportException = attrs.reportException || function() {}; + this.reportExpectationFailure = attrs.reportExpectationFailure || function() {}; + this.afterAllExpectationFailures = attrs.afterAllExpectationFailures || []; } QueueRunner.prototype.execute = function() { @@ -26,10 +28,10 @@ getJasmineRequireObj().QueueRunner = function(j$) { }; QueueRunner.prototype.run = function(queueableFns, recursiveIndex) { - var runner = this, - length = queueableFns.length, - self = this, - iterativeIndex; + var length = queueableFns.length, + self = this, + iterativeIndex; + for(iterativeIndex = recursiveIndex; iterativeIndex < length; iterativeIndex++) { var queueableFn = queueableFns[iterativeIndex]; @@ -37,6 +39,7 @@ getJasmineRequireObj().QueueRunner = function(j$) { return attemptAsync(queueableFn); } else { attemptSync(queueableFn); + flushAfterAllExpectationFailures(); } } @@ -60,6 +63,7 @@ getJasmineRequireObj().QueueRunner = function(j$) { }, next = once(function () { clearTimeout(timeoutId); + flushAfterAllExpectationFailures(); self.run(queueableFns, iterativeIndex + 1); }), timeoutId; @@ -93,6 +97,12 @@ getJasmineRequireObj().QueueRunner = function(j$) { throw e; } } + + function flushAfterAllExpectationFailures() { + while (self.afterAllExpectationFailures.length) { + self.reportExpectationFailure(self.afterAllExpectationFailures.pop()); + } + } }; return QueueRunner; diff --git a/src/core/Suite.js b/src/core/Suite.js index 43d02430..bdca8ad1 100644 --- a/src/core/Suite.js +++ b/src/core/Suite.js @@ -13,6 +13,7 @@ getJasmineRequireObj().Suite = function() { this.afterFns = []; this.beforeAllFns = []; this.afterAllFns = []; + this.afterAllExpectationFailures = []; this.queueRunner = attrs.queueRunner || function() {}; this.disabled = false; @@ -89,7 +90,8 @@ getJasmineRequireObj().Suite = function() { queueableFns: allFns, onComplete: complete, userContext: this.sharedUserContext(), - onException: function() { self.onException.apply(self, arguments); } + onException: function() { self.onException.apply(self, arguments); }, + afterAllExpectationFailures: this.afterAllExpectationFailures }); function complete() { @@ -136,9 +138,21 @@ getJasmineRequireObj().Suite = function() { }; Suite.prototype.addExpectationResult = function () { - for (var i = 0; i < this.children.length; i++) { - var child = this.children[i]; - child.addExpectationResult.apply(child, arguments); + if(isAfterAll(this.children) && isFailure(arguments)){ + this.afterAllExpectationFailures.push(arguments[1].message); + } else { + for (var i = 0; i < this.children.length; i++) { + var child = this.children[i]; + child.addExpectationResult.apply(child, arguments); + } + } + + function isAfterAll(children) { + return children && children[0].result.status; + } + + function isFailure(args) { + return !args[0]; } }; diff --git a/src/html/HtmlReporter.js b/src/html/HtmlReporter.js index 7ef6fb67..85da9c35 100644 --- a/src/html/HtmlReporter.js +++ b/src/html/HtmlReporter.js @@ -65,7 +65,7 @@ jasmineRequire.HtmlReporter = function(j$) { currentParent.addChild(result, 'spec'); }; - this.afterAllError = function(error) { + this.afterAllEvent = function(error) { exceptionList.push(error); }; @@ -142,7 +142,7 @@ jasmineRequire.HtmlReporter = function(j$) { alert.appendChild(createDom('span', {className: statusBarClassName}, statusBarMessage)); for(i = 0; i < exceptionList.length; i++) { - var errorBarMessage = 'An error was thrown in an afterAll: ' + (exceptionList[i].message || exceptionList[i].description); + var errorBarMessage = 'AfterAll ' + (exceptionList[i]); var errorBarClassName = 'bar errored'; alert.appendChild(createDom('span', {className: errorBarClassName}, errorBarMessage)); }