Skip to content

Commit

Permalink
refactor(adapter): Simpilfy message+stack processing. (#239)
Browse files Browse the repository at this point in the history
Avoids duplicates with a clearer algorithm
  • Loading branch information
johnjbarton committed Oct 16, 2019
1 parent 646796e commit 9480804
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 47 deletions.
60 changes: 16 additions & 44 deletions src/adapter.js
Expand Up @@ -17,15 +17,13 @@ function isExternalStackEntry (entry) {

/**
* Returns relevant stack entries.
* @param {String} stack Complete error stack trace.
* @param {Array} stack frames
* @return {Array} A list of relevant stack entries.
*/
function getRelevantStackFrom (stack) {
var filteredStack = []
var relevantStack = []

stack = stack.split('\n')

for (var i = 0; i < stack.length; i += 1) {
if (isExternalStackEntry(stack[i])) {
filteredStack.push(stack[i])
Expand Down Expand Up @@ -68,7 +66,7 @@ function formatFailedStep (step) {
// construct a stacktrace out of filename and lineno:
if (!step.stack) {
if (step.filename) {
let stackframe = step.filename
var stackframe = step.filename
if (step.lineno) {
stackframe = stackframe + ':' + step.lineno
}
Expand All @@ -80,49 +78,23 @@ function formatFailedStep (step) {

// Remove the message prior to processing the stack to prevent issues like
// https://github.com/karma-runner/karma-jasmine/issues/79
var stack = step.stack.replace('Error: ' + step.message, '')
var prefix = (stack === step.stack) ? '' : 'Error: '

var dirtyRelevantStack = getRelevantStackFrom(stack)

// PhantomJS returns multiline error message for errors coming from specs
// (for example when calling a non-existing function). This error is present
// in both `step.message` and `step.stack` at the same time, but stack seems
// preferable, so we iterate relevant stack, compare it to message:
for (var i = 0; i < dirtyRelevantStack.length; i += 1) {
if (typeof step.message === 'string' && step.message.indexOf(dirtyRelevantStack[i]) === -1) {
// Stack entry is not in the message,
// we consider it to be a relevant stack:
relevantStack.push(dirtyRelevantStack[i])
} else {
// Stack entry is already in the message,
// we consider it to be a suitable message alternative:
relevantMessage.push(prefix + dirtyRelevantStack[i])
}
var stackframes = step.stack.split('\n')
var messageOnStack = null
if (stackframes[0].indexOf(step.message) !== -1) {
// Remove the message if it is in the stack string (eg Chrome)
messageOnStack = stackframes.shift()
}

// In most cases the above will leave us with an empty message...
if (relevantMessage.length === 0) {
// Let's reuse the original message:
relevantMessage.push(prefix + step.message)

// Now we probably have a repetition case where:
// relevantMessage: ["Expected true to be false."]
// relevantStack: ["Error: Expected true to be false.", ...]
if (relevantStack.length && relevantStack[0].indexOf(step.message) !== -1) {
// The message seems preferable, so we remove the first value from
// the stack to get rid of repetition :
relevantStack.shift()
}
// Filter frames
var relevantStackFrames = getRelevantStackFrom(stackframes)
if (messageOnStack) {
// Put the message back if we removed it.
relevantStackFrames.unshift(messageOnStack)
} else {
// The stack did not have the step.message so add it.
relevantStackFrames.unshift(step.message)
}

// Example output:
// --------------------
// Chrome 40.0.2214 (Mac OS X 10.9.5) xxx should return false 1 FAILED
// Expected true to be false
// at /foo/bar/baz.spec.js:22:13
// at /foo/bar/baz.js:18:29
return relevantMessage.concat(relevantStack).join('\n')
return relevantStackFrames.join('\n')
}

function SuiteNode (name, parent) {
Expand Down
6 changes: 3 additions & 3 deletions test/adapter.spec.js
Expand Up @@ -457,20 +457,20 @@ describe('jasmine adapter', function () {

it('should split by newline and return all values for which isExternalStackEntry returns true', function () {
spyOn(window, 'isExternalStackEntry').and.returnValue(true)
expect(getRelevantStackFrom('a\nb\nc')).toEqual(['a', 'b', 'c'])
expect(getRelevantStackFrom(['a', 'b', 'c'])).toEqual(['a', 'b', 'c'])
})

it('should return the all stack entries if every entry is irrelevant', function () {
// Check the case where the filteredStack is empty
spyOn(window, 'isExternalStackEntry').and.returnValue(false)
expect(getRelevantStackFrom('a\nb\nc')).toEqual(['a', 'b', 'c'])
expect(getRelevantStackFrom(['a', 'b', 'c'])).toEqual(['a', 'b', 'c'])
})

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

Expand Down

0 comments on commit 9480804

Please sign in to comment.