From d6da13a8ddb07eb37e577bc2076bed486ba7b0d5 Mon Sep 17 00:00:00 2001 From: "Dan Hansen and Davis W. Frank" Date: Wed, 27 Feb 2013 16:37:31 -0800 Subject: [PATCH] Attempt at normalizing error stacks across browsers. Failed expectations now have a `stack` property, remove `trace.stack` --- lib/jasmine-core/jasmine-html.js | 3 +- lib/jasmine-core/jasmine.js | 126 ++++++++++++++++++---------- spec/core/ExceptionFormatterSpec.js | 62 ++++++++++---- spec/core/ExpectationResultSpec.js | 41 +++++---- spec/core/MatchersSpec.js | 7 -- spec/html/HtmlReporterSpec.js | 4 +- src/core/Env.js | 15 ++-- src/core/ExceptionFormatter.js | 31 ++++--- src/core/ExpectationResult.js | 45 ++++++++-- src/core/Matchers.js | 18 ++-- src/core/Spec.js | 14 ++-- src/html/HtmlReporter.js | 3 +- 12 files changed, 237 insertions(+), 132 deletions(-) diff --git a/lib/jasmine-core/jasmine-html.js b/lib/jasmine-core/jasmine-html.js index 02906847..10961d36 100644 --- a/lib/jasmine-core/jasmine-html.js +++ b/lib/jasmine-core/jasmine-html.js @@ -79,9 +79,8 @@ jasmine.HtmlReporter = function(options) { for (var i = 0; i < result.failedExpectations.length; i++) { var expectation = result.failedExpectations[i]; - var stack = (expectation.trace && expectation.trace.stack) || ""; messages.appendChild(createDom("div", {className: "result-message"}, expectation.message)); - messages.appendChild(createDom("div", {className: "stack-trace"}, stack)); + messages.appendChild(createDom("div", {className: "stack-trace"}, expectation.stack)); } failures.push(failure); diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index 0e4821ac..7d6c19e9 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -439,29 +439,66 @@ jasmine.util.argsToArray = function(args) { var arrayOfArgs = []; for (var i = 0; i < args.length; i++) arrayOfArgs.push(args[i]); return arrayOfArgs; -};jasmine.exceptionFormatter = function(e) { - var message = e.name - + ': ' - + e.message - + ' in ' - + (e.fileName || e.sourceURL || '') - + ' (line ' - + (e.line || e.lineNumber || '') - + ')'; +};jasmine.ExceptionFormatter = function() { + this.message = function(error) { + var message = error.name + + ': ' + + error.message; - return message; -}; -//TODO: expectation result may make more sense as a presentation of an expectation. -jasmine.buildExpectationResult = function(params) { - return { - type: 'expect', - matcherName: params.matcherName, - expected: params.expected, - actual: params.actual, - message: params.passed ? 'Passed.' : params.message, - trace: params.passed ? '' : (params.trace || new Error(this.message)), - passed: params.passed + if (error.fileName || error.sourceURL) { + message += " in " + (error.fileName || error.sourceURL); + } + + if (error.line || error.lineNumber) { + message += " (line " + (error.line || error.lineNumber) + ")" + } + + return message; }; + + this.stack = function(error) { + return error ? error.stack : null; + } +};//TODO: expectation result may make more sense as a presentation of an expectation. +jasmine.buildExpectationResult = function(options) { + var messageFormatter = options.messageFormatter || function() {}, + stackFormatter = options.stackFormatter || function() {}; + + return { + matcherName: options.matcherName, + expected: options.expected, + actual: options.actual, + message: message(), + stack: stack(), + passed: options.passed + }; + + function message() { + if (options.passed) { + return "Passed." + } else if (options.message) { + return options.message + } else if (options.error) { + return messageFormatter(options.error); + } + return "" + } + + function stack() { + if (options.passed) { + return ""; + } + + var error = options.error; + if (!error) { + try { + throw new Error(message()); + } catch (e) { + error = e; + } + } + return stackFormatter(error); + } }; (function() { jasmine.Env = function(options) { @@ -536,18 +573,21 @@ jasmine.buildExpectationResult = function(params) { } }; - var exceptionFormatter = jasmine.exceptionFormatter; - var specConstructor = jasmine.Spec; var getSpecName = function(spec, currentSuite) { return currentSuite.getFullName() + ' ' + spec.description + '.'; }; - var buildExpectationResult = jasmine.buildExpectationResult; - var expectationResultFactory = function(attrs) { - return buildExpectationResult(attrs); - }; + // TODO: we may just be able to pass in the fn instead of wrapping here + var buildExpectationResult = jasmine.buildExpectationResult, + exceptionFormatter = new jasmine.ExceptionFormatter(), + expectationResultFactory = function(attrs) { + attrs.messageFormatter = exceptionFormatter.message; + attrs.stackFormatter = exceptionFormatter.stack; + + return buildExpectationResult(attrs); + }; // TODO: fix this naming, and here's where the value comes in this.catchExceptions = function(value) { @@ -1043,21 +1083,19 @@ jasmine.Matchers.matcherFn_ = function(matcherName, matcherFunction) { message += "."; } } - var expectationResult = jasmine.buildExpectationResult({ + + this.spec.addExpectationResult(result, { matcherName: matcherName, passed: result, expected: matcherArgs.length > 1 ? matcherArgs : matcherArgs[0], actual: this.actual, message: message }); - this.spec.addExpectationResult(result, expectationResult); return jasmine.undefined; }; }; - - /** * toBe: compares the actual to the expected using === * @param expected @@ -1137,11 +1175,11 @@ jasmine.Matchers.prototype.toBeNull = function() { * Matcher that compares the actual to NaN. */ jasmine.Matchers.prototype.toBeNaN = function() { - this.message = function() { - return [ "Expected " + jasmine.pp(this.actual) + " to be NaN." ]; - }; + this.message = function() { + return [ "Expected " + jasmine.pp(this.actual) + " to be NaN." ]; + }; - return (this.actual !== this.actual); + return (this.actual !== this.actual); }; /** @@ -1356,7 +1394,7 @@ jasmine.Matchers.Any.prototype.jasmineToString = function() { return ''; }; -jasmine.Matchers.ObjectContaining = function (sample) { +jasmine.Matchers.ObjectContaining = function(sample) { this.sample = sample; }; @@ -1382,7 +1420,7 @@ jasmine.Matchers.ObjectContaining.prototype.jasmineMatches = function(other, mis return (mismatchKeys.length === 0 && mismatchValues.length === 0); }; -jasmine.Matchers.ObjectContaining.prototype.jasmineToString = function () { +jasmine.Matchers.ObjectContaining.prototype.jasmineToString = function() { return ""; }; /** @@ -1583,9 +1621,10 @@ jasmine.Spec = function(attrs) { jasmine.Spec.prototype.addExpectationResult = function(passed, data) { this.encounteredExpectations = true; - if (!passed) { - this.result.failedExpectations.push(data); + if (passed) { + return; } + this.result.failedExpectations.push(this.expectationResultFactory(data)); }; jasmine.Spec.prototype.expect = function(actual) { @@ -1608,14 +1647,13 @@ jasmine.Spec.prototype.execute = function(onComplete) { this.queueRunner({ fns: allFns, onException: function(e) { - self.addExpectationResult(false, self.expectationResultFactory({ + self.addExpectationResult(false, { matcherName: "", passed: false, expected: "", actual: "", - message: self.exceptionFormatter(e), - trace: e - })); + error: e + }); }, onComplete: complete }); @@ -1652,7 +1690,7 @@ jasmine.Spec.prototype.status = function() { jasmine.Spec.prototype.getFullName = function() { return this.getSpecName(this); -} +}; jasmine.Suite = function(attrs) { this.env = attrs.env; this.id = attrs.id; diff --git a/spec/core/ExceptionFormatterSpec.js b/spec/core/ExceptionFormatterSpec.js index c3052c8c..7ded6ac7 100644 --- a/spec/core/ExceptionFormatterSpec.js +++ b/spec/core/ExceptionFormatterSpec.js @@ -1,26 +1,52 @@ describe("ExceptionFormatter", function() { + describe("#message", function() { + it('formats Firefox exception messages', function() { + var sampleFirefoxException = { + fileName: 'foo.js', + lineNumber: '1978', + message: 'you got your foo in my bar', + name: 'A Classic Mistake' + }, + exceptionFormatter = new jasmine.ExceptionFormatter(), + message = exceptionFormatter.message(sampleFirefoxException); - it('formats Firefox exception messages', function() { - var sampleFirefoxException = { - fileName: 'foo.js', - line: '1978', - message: 'you got your foo in my bar', - name: 'A Classic Mistake' - }, - message = jasmine.exceptionMessageFor(sampleFirefoxException); + expect(message).toEqual('A Classic Mistake: you got your foo in my bar in foo.js (line 1978)'); + }); - expect(message).toEqual('A Classic Mistake: you got your foo in my bar in foo.js (line 1978)'); + it('formats Webkit exception messages', function() { + var sampleWebkitException = { + sourceURL: 'foo.js', + line: '1978', + message: 'you got your foo in my bar', + name: 'A Classic Mistake' + }, + exceptionFormatter = new jasmine.ExceptionFormatter(), + message = exceptionFormatter.message(sampleWebkitException); + + expect(message).toEqual('A Classic Mistake: you got your foo in my bar in foo.js (line 1978)'); + }); + + it('formats V8 exception messages', function() { + var sampleV8 = { + message: 'you got your foo in my bar', + name: 'A Classic Mistake' + }, + exceptionFormatter = new jasmine.ExceptionFormatter(), + message = exceptionFormatter.message(sampleV8); + + expect(message).toEqual('A Classic Mistake: you got your foo in my bar'); + + }); }); - it('formats Webkit exception messages', function() { - var sampleWebkitException = { - sourceURL: 'foo.js', - lineNumber: '1978', - message: 'you got your foo in my bar', - name: 'A Classic Mistake' - }, - message = jasmine.exceptionMessageFor(sampleWebkitException); + describe("#stack", function() { + it("formats stack traces from Webkit, Firefox or node.js", function() { + var error = new Error("an error"); + expect(new jasmine.ExceptionFormatter().stack(error)).toMatch(/ExceptionFormatterSpec\.js.*\d+/) + }); - expect(message).toEqual('A Classic Mistake: you got your foo in my bar in foo.js (line 1978)'); + it("returns null if no Error provided", function() { + expect(new jasmine.ExceptionFormatter().stack()).toBeNull(); + }); }); }); \ No newline at end of file diff --git a/spec/core/ExpectationResultSpec.js b/spec/core/ExpectationResultSpec.js index 36c4e49c..6586ab96 100644 --- a/spec/core/ExpectationResultSpec.js +++ b/spec/core/ExpectationResultSpec.js @@ -1,33 +1,47 @@ describe("buildExpectationResult", function() { - it("defaults to passed", function() { var result = jasmine.buildExpectationResult({passed: 'some-value'}); expect(result.passed).toBe('some-value'); }); - it("has a type of expect", function() { - var result = jasmine.buildExpectationResult({}); - expect(result.type).toBe('expect'); - }); - it("message defaults to Passed for passing specs", function() { var result = jasmine.buildExpectationResult({passed: true, message: 'some-value'}); expect(result.message).toBe('Passed.'); }); - it("message returns the message for failing specs", function() { + it("message returns the message for failing expecations", function() { var result = jasmine.buildExpectationResult({passed: false, message: 'some-value'}); expect(result.message).toBe('some-value'); }); - it("trace passes trace if exists", function() { - var result = jasmine.buildExpectationResult({trace: 'some-value'}); - expect(result.trace).toBe('some-value'); + it("delegates message formatting to the provided formatter if there was an Error", function() { + var fakeError = {message: 'foo'}, + messageFormatter = jasmine.createSpy("exception message formatter").andReturn(fakeError.message); + + var result = jasmine.buildExpectationResult( + { + passed: false, + error: fakeError, + messageFormatter: messageFormatter + }); + + expect(messageFormatter).toHaveBeenCalledWith(fakeError); + expect(result.message).toEqual('foo'); }); - it("trace returns a new error if trace is falsy", function() { - var result = jasmine.buildExpectationResult({trace: false}); - expect(result.trace).toEqual(jasmine.any(Error)); + it("delegates stack formatting to the provided formatter if there was an Error", function() { + var fakeError = {stack: 'foo'}, + stackFormatter = jasmine.createSpy("stack formatter").andReturn(fakeError.stack); + + var result = jasmine.buildExpectationResult( + { + passed: false, + error: fakeError, + stackFormatter: stackFormatter + }); + + expect(stackFormatter).toHaveBeenCalledWith(fakeError); + expect(result.stack).toEqual('foo'); }); it("matcherName returns passed matcherName", function() { @@ -44,5 +58,4 @@ describe("buildExpectationResult", function() { var result = jasmine.buildExpectationResult({actual: 'some-value'}); expect(result.actual).toBe('some-value'); }); - }); diff --git a/spec/core/MatchersSpec.js b/spec/core/MatchersSpec.js index fe501f7a..011c4aae 100644 --- a/spec/core/MatchersSpec.js +++ b/spec/core/MatchersSpec.js @@ -660,9 +660,6 @@ describe("jasmine.Matchers", function() { }); it("should provide an inverted default message", function() { - match(37).not.toBeGreaterThan(42); - expect(lastResult().message).toEqual("Passed."); - match(42).not.toBeGreaterThan(37); expect(lastResult().message).toEqual("Expected 42 not to be greater than 37."); }); @@ -677,14 +674,10 @@ describe("jasmine.Matchers", function() { } }); - match(true).custom(); - expect(lastResult().message).toEqual("Passed."); match(false).custom(); expect(lastResult().message).toEqual("Expected it was called."); match(true).not.custom(); expect(lastResult().message).toEqual("Expected it wasn't called."); - match(false).not.custom(); - expect(lastResult().message).toEqual("Passed."); }); }); diff --git a/spec/html/HtmlReporterSpec.js b/spec/html/HtmlReporterSpec.js index b897259f..c87f5286 100644 --- a/spec/html/HtmlReporterSpec.js +++ b/spec/html/HtmlReporterSpec.js @@ -369,9 +369,7 @@ describe("New HtmlReporter", function() { failedExpectations: [ { message: "a failure message", - trace: { - stack: "a stack trace" - } + stack: "a stack trace" } ] }; diff --git a/src/core/Env.js b/src/core/Env.js index c23bd6da..ff743ca3 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -71,18 +71,21 @@ } }; - var exceptionFormatter = jasmine.exceptionFormatter; - var specConstructor = jasmine.Spec; var getSpecName = function(spec, currentSuite) { return currentSuite.getFullName() + ' ' + spec.description + '.'; }; - var buildExpectationResult = jasmine.buildExpectationResult; - var expectationResultFactory = function(attrs) { - return buildExpectationResult(attrs); - }; + // TODO: we may just be able to pass in the fn instead of wrapping here + var buildExpectationResult = jasmine.buildExpectationResult, + exceptionFormatter = new jasmine.ExceptionFormatter(), + expectationResultFactory = function(attrs) { + attrs.messageFormatter = exceptionFormatter.message; + attrs.stackFormatter = exceptionFormatter.stack; + + return buildExpectationResult(attrs); + }; // TODO: fix this naming, and here's where the value comes in this.catchExceptions = function(value) { diff --git a/src/core/ExceptionFormatter.js b/src/core/ExceptionFormatter.js index 7cb6032c..d0fb644f 100644 --- a/src/core/ExceptionFormatter.js +++ b/src/core/ExceptionFormatter.js @@ -1,12 +1,21 @@ -jasmine.exceptionMessageFor = function(e) { - var message = e.name - + ': ' - + e.message - + ' in ' - + (e.fileName || e.sourceURL || '') - + ' (line ' - + (e.line || e.lineNumber || '') - + ')'; +jasmine.ExceptionFormatter = function() { + this.message = function(error) { + var message = error.name + + ': ' + + error.message; - return message; -}; + if (error.fileName || error.sourceURL) { + message += " in " + (error.fileName || error.sourceURL); + } + + if (error.line || error.lineNumber) { + message += " (line " + (error.line || error.lineNumber) + ")" + } + + return message; + }; + + this.stack = function(error) { + return error ? error.stack : null; + } +}; \ No newline at end of file diff --git a/src/core/ExpectationResult.js b/src/core/ExpectationResult.js index bc783ede..2d3c3d57 100644 --- a/src/core/ExpectationResult.js +++ b/src/core/ExpectationResult.js @@ -1,12 +1,41 @@ //TODO: expectation result may make more sense as a presentation of an expectation. -jasmine.buildExpectationResult = function(params) { +jasmine.buildExpectationResult = function(options) { + var messageFormatter = options.messageFormatter || function() {}, + stackFormatter = options.stackFormatter || function() {}; + return { - type: 'expect', - matcherName: params.matcherName, - expected: params.expected, - actual: params.actual, - message: params.passed ? 'Passed.' : params.message, - trace: params.passed ? '' : (params.trace || new Error(this.message)), - passed: params.passed + matcherName: options.matcherName, + expected: options.expected, + actual: options.actual, + message: message(), + stack: stack(), + passed: options.passed }; + + function message() { + if (options.passed) { + return "Passed." + } else if (options.message) { + return options.message + } else if (options.error) { + return messageFormatter(options.error); + } + return "" + } + + function stack() { + if (options.passed) { + return ""; + } + + var error = options.error; + if (!error) { + try { + throw new Error(message()); + } catch (e) { + error = e; + } + } + return stackFormatter(error); + } }; diff --git a/src/core/Matchers.js b/src/core/Matchers.js index 468c2a7e..d7de131b 100644 --- a/src/core/Matchers.js +++ b/src/core/Matchers.js @@ -53,21 +53,19 @@ jasmine.Matchers.matcherFn_ = function(matcherName, matcherFunction) { message += "."; } } - var expectationResult = jasmine.buildExpectationResult({ + + this.spec.addExpectationResult(result, { matcherName: matcherName, passed: result, expected: matcherArgs.length > 1 ? matcherArgs : matcherArgs[0], actual: this.actual, message: message }); - this.spec.addExpectationResult(result, expectationResult); return jasmine.undefined; }; }; - - /** * toBe: compares the actual to the expected using === * @param expected @@ -147,11 +145,11 @@ jasmine.Matchers.prototype.toBeNull = function() { * Matcher that compares the actual to NaN. */ jasmine.Matchers.prototype.toBeNaN = function() { - this.message = function() { - return [ "Expected " + jasmine.pp(this.actual) + " to be NaN." ]; - }; + this.message = function() { + return [ "Expected " + jasmine.pp(this.actual) + " to be NaN." ]; + }; - return (this.actual !== this.actual); + return (this.actual !== this.actual); }; /** @@ -366,7 +364,7 @@ jasmine.Matchers.Any.prototype.jasmineToString = function() { return ''; }; -jasmine.Matchers.ObjectContaining = function (sample) { +jasmine.Matchers.ObjectContaining = function(sample) { this.sample = sample; }; @@ -392,6 +390,6 @@ jasmine.Matchers.ObjectContaining.prototype.jasmineMatches = function(other, mis return (mismatchKeys.length === 0 && mismatchValues.length === 0); }; -jasmine.Matchers.ObjectContaining.prototype.jasmineToString = function () { +jasmine.Matchers.ObjectContaining.prototype.jasmineToString = function() { return ""; }; diff --git a/src/core/Spec.js b/src/core/Spec.js index bc39cad3..5b6db399 100644 --- a/src/core/Spec.js +++ b/src/core/Spec.js @@ -26,9 +26,10 @@ jasmine.Spec = function(attrs) { jasmine.Spec.prototype.addExpectationResult = function(passed, data) { this.encounteredExpectations = true; - if (!passed) { - this.result.failedExpectations.push(data); + if (passed) { + return; } + this.result.failedExpectations.push(this.expectationResultFactory(data)); }; jasmine.Spec.prototype.expect = function(actual) { @@ -51,14 +52,13 @@ jasmine.Spec.prototype.execute = function(onComplete) { this.queueRunner({ fns: allFns, onException: function(e) { - self.addExpectationResult(false, self.expectationResultFactory({ + self.addExpectationResult(false, { matcherName: "", passed: false, expected: "", actual: "", - message: self.exceptionFormatter(e), - trace: e - })); + error: e + }); }, onComplete: complete }); @@ -95,4 +95,4 @@ jasmine.Spec.prototype.status = function() { jasmine.Spec.prototype.getFullName = function() { return this.getSpecName(this); -} +}; diff --git a/src/html/HtmlReporter.js b/src/html/HtmlReporter.js index 2c6ec006..09bcb8d3 100644 --- a/src/html/HtmlReporter.js +++ b/src/html/HtmlReporter.js @@ -79,9 +79,8 @@ jasmine.HtmlReporter = function(options) { for (var i = 0; i < result.failedExpectations.length; i++) { var expectation = result.failedExpectations[i]; - var stack = (expectation.trace && expectation.trace.stack) || ""; messages.appendChild(createDom("div", {className: "result-message"}, expectation.message)); - messages.appendChild(createDom("div", {className: "stack-trace"}, stack)); + messages.appendChild(createDom("div", {className: "stack-trace"}, expectation.stack)); } failures.push(failure);