Skip to content

Commit

Permalink
Merge pull request karma-runner#87 from matthewjh/stack-trace-fix
Browse files Browse the repository at this point in the history 
feat(adapter): do not filter out karma/jasmine frames from stack if the stack consists entirely of said frames
  • Loading branch information
@maksimr
maksimr committed May 12, 2015
2 parents 6ed9f5e + 616b1b9 commit dec1ef2
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 26 deletions.
36 changes: 24 additions & 12 deletions src/adapter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,16 @@
'use strict';

/**
* Decision maker for whether a stack entry is considered relevant.
* Decision maker for whether a stack entry is considered external to jasmine and karma.
* @param {String} entry Error stack entry.
* @return {Boolean} True if relevant, False otherwise.
* @return {Boolean} True if external, False otherwise.
*/
function isRelevantStackEntry(entry) {
// discard empty and falsy entries:
return (entry ? true : false) &&
// discard entries related to jasmine and karma-jasmine:
!/\/(jasmine-core|karma-jasmine)\//.test(entry) &&
// discard karma specifics, e.g. "at http://localhost:7018/karma.js:185"
!/\/(karma.js|context.html):/.test(entry);
function isExternalStackEntry(entry) {
return (entry ? true : false) &&
// entries related to jasmine and karma-jasmine:
!/\/(jasmine-core|karma-jasmine)\//.test(entry) &&
// karma specifics, e.g. "at http://localhost:7018/karma.js:185"
!/\/(karma.js|context.html):/.test(entry);
}

/**
Expand All @@ -21,13 +20,26 @@ function isRelevantStackEntry(entry) {
* @return {Array} A list of relevant stack entries.
*/
function getRelevantStackFrom(stack) {
var relevantStack = [];
var filteredStack = [],
relevantStack = [];

stack = stack.split('\n');

for (var i = 0; i < stack.length; i += 1) {
if (isRelevantStackEntry(stack[i])) {
relevantStack.push(stack[i]);
if (isExternalStackEntry(stack[i])) {
filteredStack.push(stack[i]);
}
}

// If the filtered stack is empty, i.e. the error originated entirely from within jasmine or karma, then the whole stack
// should be relevant.
if (filteredStack.length === 0) {
filteredStack = stack;
}

for (i = 0; i < filteredStack.length; i += 1) {
if (filteredStack[i]) {
relevantStack.push(filteredStack[i]);
}
}

Expand Down
69 changes: 55 additions & 14 deletions test/adapter.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ describe('jasmine adapter', function(){
});


it('should remove jasmine-specific frames from the exception stack traces', function(){
it('should remove jasmine-specific frames from the exception stack traces if the trace contains non-jasmine specific frames', function(){
var step = {};

step.message = 'Expected true to be false.';
Expand Down Expand Up @@ -138,6 +138,38 @@ describe('jasmine adapter', function(){

});

it('should not remove jasmine-specific frames from the exception stack traces if the trace contains no non-jasmine specific frames', function(){
var step = {};

step.message = 'Expected true to be false.';
step.stack = 'Error: Expected true to be false.\n' +
' at stack (/foo/bar/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1441:17)\n' +
' at buildExpectationResult (/foo/bar/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1411:14)\n' +
' at Spec.Env.expectationResultFactory (/foo/bar/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:533:18)\n' +
' at Spec.addExpectationResult (/foo/bar/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:293:34)\n' +
' at Expectation.addExpectationResult (/foo/bar/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:477:21)\n' +
' at Expectation.toBe (/foo/bar/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1365:12)\n';


karma.result.andCallFake(function(result){
expect(result.log).toEqual([
'Expected true to be false.\n' +
' at stack (/foo/bar/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1441:17)\n' +
' at buildExpectationResult (/foo/bar/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1411:14)\n' +
' at Spec.Env.expectationResultFactory (/foo/bar/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:533:18)\n' +
' at Spec.addExpectationResult (/foo/bar/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:293:34)\n' +
' at Expectation.addExpectationResult (/foo/bar/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:477:21)\n' +
' at Expectation.toBe (/foo/bar/node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1365:12)'
]);
});

spec.result.failedExpectations.push(step);
reporter.specDone(spec.result);

expect(karma.result).toHaveBeenCalled();

});

it('should remove special top level suite from result', function () {
karma.result.andCallFake(function(result){
expect(result.suite).toEqual([ 'Child Suite' ]);
Expand Down Expand Up @@ -233,27 +265,27 @@ describe('jasmine adapter', function(){
});


describe('isRelevantStackEntry', function(){
describe('isExternalStackEntry', function(){
it('should be a function', function(){
expect(typeof isRelevantStackEntry).toBe('function');
expect(typeof isExternalStackEntry).toBe('function');
});
it('should return false for empty strings', function(){
expect(isRelevantStackEntry('')).toBe(false);
expect(isExternalStackEntry('')).toBe(false);
});
it('should return false for strings with "jasmine-core"', function () {
expect(isRelevantStackEntry('/foo/jasmine-core/bar.js')).toBe(false);
expect(isExternalStackEntry('/foo/jasmine-core/bar.js')).toBe(false);
});
it('should return false for strings with "karma-jasmine"', function () {
expect(isRelevantStackEntry('/foo/karma-jasmine/bar.js')).toBe(false);
expect(isExternalStackEntry('/foo/karma-jasmine/bar.js')).toBe(false);
});
it('should return false for strings with "karma.js"', function () {
expect(isRelevantStackEntry('/foo/karma.js:183')).toBe(false);
expect(isExternalStackEntry('/foo/karma.js:183')).toBe(false);
});
it('should return false for strings with "context.html"', function () {
expect(isRelevantStackEntry('/foo/context.html:13')).toBe(false);
expect(isExternalStackEntry('/foo/context.html:13')).toBe(false);
});
it('should return true for all other strings', function(){
expect(isRelevantStackEntry('/foo/bar/baz.js:13:1')).toBe(true);
expect(isExternalStackEntry('/foo/bar/baz.js:13:1')).toBe(true);
});
});

Expand All @@ -262,13 +294,22 @@ describe('jasmine adapter', function(){
it('should be a function', function(){
expect(typeof getRelevantStackFrom).toBe('function');
});
it('should split by newline and return all values for which isRelevantStackEntry returns true', function () {
isRelevantStackEntry = jasmine.createSpy('isRelevantStackEntry').andReturn(true);

it('should split by newline and return all values for which isExternalStackEntry returns true', function () {
isExternalStackEntry = jasmine.createSpy('isExternalStackEntry').andReturn(true);
expect(getRelevantStackFrom('a\nb\nc')).toEqual(['a', 'b', 'c']);
});
it('should not return any values for which isRelevantStackEntry returns false', function () {
isRelevantStackEntry = jasmine.createSpy('isRelevantStackEntry').andReturn(false);
expect(getRelevantStackFrom('a\nb\nc')).toEqual([]);

it('should return the all stack entries if every entry is irrelevant', function () {
isExternalStackEntry = jasmine.createSpy('isExternalStackEntry').andReturn(false);
expect(getRelevantStackFrom('a\nb\nc')).toEqual(['a', 'b', 'c']);
});

it('should return only the relevant stack entries if the stack contains relevant entries', function () {
isExternalStackEntry = jasmine.createSpy('isExternalStackEntry').andCallFake(function (entry) {
return entry !== 'b';
});
expect(getRelevantStackFrom('a\nb\nc')).toEqual(['a', 'c']);
});
});

Expand Down

0 comments on commit dec1ef2

Please sign in to comment.