Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(adapter): Simplify message+stack processing. #239

Merged
merged 1 commit into from Oct 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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