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

Redesign and improve reporter error display #6467

Merged
merged 60 commits into from
Mar 13, 2020
Merged
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
4cbcd0e
redesign and improve reporter error display
chrisbreiding Feb 14, 2020
515033b
$utils -> $errUtils
brian-mann Feb 14, 2020
1b3b113
derp
brian-mann Feb 14, 2020
8f0a3a3
serializeError -> wrapErr
brian-mann Feb 14, 2020
ef3ec79
cloneErr -> makeErrFromObj
brian-mann Feb 14, 2020
4f9379f
yarn.lock
chrisbreiding Feb 18, 2020
479b56d
fix unit tests
chrisbreiding Feb 18, 2020
25883f6
move err-model
chrisbreiding Feb 18, 2020
ad4575a
Merge branch 'extract-backticks' of github.com:cypress-io/cypress int…
chrisbreiding Feb 18, 2020
9cfe040
fix styles
chrisbreiding Feb 18, 2020
8793a62
fix/improve error logging
chrisbreiding Feb 18, 2020
47b42e1
fix non-converted bits
chrisbreiding Feb 18, 2020
7669533
transfer missed changes
chrisbreiding Feb 18, 2020
e4bfb7a
fix issues
chrisbreiding Feb 18, 2020
14b0840
remove obselete spec
chrisbreiding Feb 18, 2020
854ccc2
make type test more reliable
chrisbreiding Feb 18, 2020
b65f707
Merge branch 'develop' into extract-backticks
jennifer-shehane Feb 19, 2020
c58e01b
use should, get retries
chrisbreiding Feb 19, 2020
d71c201
update snapshots
chrisbreiding Feb 19, 2020
56ce403
Merge branch 'extract-backticks' of github.com:cypress-io/cypress int…
chrisbreiding Feb 19, 2020
55ee7e8
update e2e network error test
chrisbreiding Feb 19, 2020
ffd3aa3
update more snapshots
chrisbreiding Feb 19, 2020
ea41e3c
update error whitespace
chrisbreiding Feb 19, 2020
cd1cf55
update snapshot
chrisbreiding Feb 19, 2020
4860abf
try something out
chrisbreiding Feb 19, 2020
28340d1
nevermind
chrisbreiding Feb 19, 2020
91e0d32
Merge branch 'develop' into extract-backticks
chrisbreiding Feb 20, 2020
6001f8f
fix tooltip
chrisbreiding Feb 24, 2020
272d883
add some logging
chrisbreiding Feb 24, 2020
1626f64
remove whitespace
chrisbreiding Feb 24, 2020
4910548
remove spying on window
brian-mann Feb 24, 2020
52ab0e0
update snapshot
chrisbreiding Feb 24, 2020
ae5d8d9
Merge branch 'develop' into extract-backticks
chrisbreiding Feb 24, 2020
cb8a965
Merge branch 'extract-backticks' of github.com:cypress-io/cypress int…
chrisbreiding Feb 24, 2020
94f37dc
fix test
chrisbreiding Feb 24, 2020
1d62031
update snapshot
chrisbreiding Feb 24, 2020
3a83947
fix merge: snapshot stacktraces
kuceb Feb 24, 2020
ec6cb03
fix noStackTrace and update snapshot
chrisbreiding Feb 25, 2020
1f890ad
Merge branch 'extract-backticks' of github.com:cypress-io/cypress int…
chrisbreiding Feb 25, 2020
479c40f
update snapshot
chrisbreiding Feb 26, 2020
5dfc72a
Merge branch 'error-improvements-approved' into extract-backticks
chrisbreiding Mar 6, 2020
fd2c434
fix yarn.lock
chrisbreiding Mar 6, 2020
d888936
don't show diff if retrying an existence error
chrisbreiding Mar 6, 2020
6b5ded1
url -> URL
chrisbreiding Mar 6, 2020
752a7dc
don't add newline after docs url and update a few snapshots
chrisbreiding Mar 6, 2020
388a286
keep opening stack trace from collapsing test
chrisbreiding Mar 6, 2020
48df542
remove unnecessary global cy reference
chrisbreiding Mar 6, 2020
b065a51
fix tests
chrisbreiding Mar 6, 2020
a3fe64b
put e2e timeout increase back in the right spot for exit: false
chrisbreiding Mar 10, 2020
e375034
don't show diff when assertion contains an element
chrisbreiding Mar 10, 2020
e831bc3
use backticks for hook error title
chrisbreiding Mar 10, 2020
9080c7e
fix appending error message when original message is falsy
chrisbreiding Mar 10, 2020
98699fb
don't show diff on existence failures
chrisbreiding Mar 12, 2020
414490f
update snapshots
chrisbreiding Mar 12, 2020
d66f4a6
Merge branch 'error-improvements-approved' into extract-backticks
chrisbreiding Mar 12, 2020
36ac6b3
fix finish/done being called twice due to not returning
chrisbreiding Mar 13, 2020
c021e6e
prevent error print button click from propagating
chrisbreiding Mar 13, 2020
d651707
use correct error methods and remove need for workaround
chrisbreiding Mar 13, 2020
7c10479
create better abstraction around creating cypress error from path, re…
chrisbreiding Mar 13, 2020
b6c9631
fix throwErr and tests
chrisbreiding Mar 13, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/driver/src/cy/chai.js
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ chai.use((chai, u) => {
return `Not enough elements found. Found '${len1}', expected '${len2}'.`
}

e1.displayMessage = getLongLengthMessage(obj.length, length)
e1.message = getLongLengthMessage(obj.length, length)
throw e1
}

Expand Down Expand Up @@ -397,10 +397,10 @@ chai.use((chai, u) => {
return `Expected ${node} not to exist in the DOM, but it was continuously found.`
}

return `Expected to find element: '${obj.selector}', but never found it.`
return `Expected to find element: \`${obj.selector}\`, but never found it.`
}

e1.displayMessage = getLongExistsMessage(obj)
e1.message = getLongExistsMessage(obj)
throw e1
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/driver/src/cy/commands/actions/check.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ checkOrUncheck = (type, subject, values = [], options = {}) ->
if not isAcceptableElement($el)
node = $dom.stringify($el)
word = $utils.plural(options.$el, "contains", "is")
phrase = if type is "check" then " and :radio" else ""
phrase = if type is "check" then " and `:radio`" else ""
$errUtils.throwErrByPath("check_uncheck.invalid_element", {
onFail: options._log
args: { node, word, phrase, cmd: type }
Expand Down
2 changes: 1 addition & 1 deletion packages/driver/src/cy/commands/actions/scroll.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->
## ensure the subject is not window itself
## cause how are you gonna scroll the window into view...
if subject is state("window")
$utils.throwErrByPath("scrollIntoView.subject_is_window")
$errUtils.throwErrByPath("scrollIntoView.subject_is_window")

## throw if we're trying to scroll to multiple elements
if subject.length > 1
Expand Down
2 changes: 1 addition & 1 deletion packages/driver/src/cy/commands/angular.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->
cy.verifyUpcomingAssertions(getEl($elements), options, {
onRetry: resolveElements
onFail: (err) ->
err.displayMessage = "Could not find element for binding: '#{binding}'."
err.message = "Could not find element for binding: '#{binding}'."
})

findByNgAttr = (name, attr, el, options) ->
Expand Down
4 changes: 2 additions & 2 deletions packages/driver/src/cy/commands/commands.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ $errUtils = require("../../cypress/error_utils")

command = (ctx, name, args...) ->
if not ctx[name]
cmds = _.keys($Chainer.prototype).join(", ")
cmds = "\`#{_.keys($Chainer.prototype).join("`, `")}\`"
$errUtils.throwErrByPath("miscellaneous.invalid_command", {
args: { name, cmds }
})

ctx[name].apply(window, args)
ctx[name].apply(ctx, args)

module.exports = (Commands, Cypress, cy, state, config) ->
Commands.addChainer({
Expand Down
22 changes: 12 additions & 10 deletions packages/driver/src/cy/commands/connectors.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Promise = require("bluebird")
$dom = require("../../dom")
$utils = require("../../cypress/utils")
$errUtils = require("../../cypress/error_utils")
$errMessages = require("../../cypress/error_messages")

returnFalseIfThenable = (key, args...) ->
if key is "then" and _.isFunction(args[0]) and _.isFunction(args[1])
Expand Down Expand Up @@ -211,11 +212,9 @@ module.exports = (Commands, Cypress, cy, state, config) ->
args: { cmd: name }
})

## TODO: use the new error utils that are part of
## the error message enhancements PR
propertyNotOnSubjectErr = (prop) ->
$errUtils.cypressErr(
$errUtils.errMsgByPath("invoke_its.nonexistent_prop", {
$errUtils.cypressErrObj(
$errUtils.errObjByPath($errMessages, "invoke_its.nonexistent_prop", {
prop,
cmd: name
})
Expand All @@ -224,8 +223,8 @@ module.exports = (Commands, Cypress, cy, state, config) ->
propertyValueNullOrUndefinedErr = (prop, value) ->
errMessagePath = if isCmdIts then "its" else "invoke"

$errUtils.cypressErr(
$errUtils.errMsgByPath("#{errMessagePath}.null_or_undefined_prop_value", {
$errUtils.cypressErrObj(
$errUtils.errObjByPath($errMessages, "#{errMessagePath}.null_or_undefined_prop_value", {
prop,
value,
cmd: name
Expand All @@ -235,17 +234,17 @@ module.exports = (Commands, Cypress, cy, state, config) ->
subjectNullOrUndefinedErr = (prop, value) ->
errMessagePath = if isCmdIts then "its" else "invoke"

$errUtils.cypressErr(
$errUtils.errMsgByPath("#{errMessagePath}.subject_null_or_undefined", {
$errUtils.cypressErrObj(
$errUtils.errObjByPath($errMessages, "#{errMessagePath}.subject_null_or_undefined", {
prop,
value,
cmd: name
})
)

propertyNotOnPreviousNullOrUndefinedValueErr = (prop, value, previousProp) ->
$errUtils.cypressErr(
$errUtils.errMsgByPath("invoke_its.previous_prop_null_or_undefined", {
$errUtils.cypressErrObj(
$errUtils.errObjByPath($errMessages, "invoke_its.previous_prop_null_or_undefined", {
prop,
value,
previousProp,
Expand Down Expand Up @@ -359,6 +358,9 @@ module.exports = (Commands, Cypress, cy, state, config) ->
Yielded: getFormattedElement(value)
})

if traversalErr
obj.Error = "#{traversalErr.name}: #{traversalErr.message}"
chrisbreiding marked this conversation as resolved.
Show resolved Hide resolved

return obj
})

Expand Down
2 changes: 1 addition & 1 deletion packages/driver/src/cy/commands/cookies.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ module.exports = function (Commands, Cypress, cy, state, config) {
$errUtils.throwErrByPath('cookies.backend_error', {
args: {
action,
command,
cmd: command,
browserDisplayName: Cypress.browser.displayName,
errMessage: err.message,
errStack: err.stack,
Expand Down
11 changes: 8 additions & 3 deletions packages/driver/src/cy/commands/files.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ _ = require("lodash")
Promise = require("bluebird")

$errUtils = require("../../cypress/error_utils")
$errMessages = require("../../cypress/error_messages")

module.exports = (Commands, Cypress, cy, state, config) ->
Commands.addAll({
Expand Down Expand Up @@ -50,16 +51,20 @@ module.exports = (Commands, Cypress, cy, state, config) ->
onFail: (err) ->
return unless err.type is "existence"

if contents?
{ message, docsUrl } = if contents?
## file exists but it shouldn't
err.displayMessage = $errUtils.errMsgByPath("files.existent", {
$errUtils.errObjByPath($errMessages, "files.existent", {
file, filePath
})
else
## file doesn't exist but it should
err.displayMessage = $errUtils.errMsgByPath("files.nonexistent", {
$errUtils.errObjByPath($errMessages, "files.nonexistent", {
file, filePath
})

err.message = message
err.docsUrl = docsUrl

onRetry: verifyAssertions
})

Expand Down
4 changes: 2 additions & 2 deletions packages/driver/src/cy/commands/querying.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ module.exports = (Commands, Cypress, cy) => {
get (selector, options = {}) {
const ctx = this

if ((options === null) || Array.isArray(options) || (typeof options !== 'object')) {
if ((options === null) || _.isArray(options) || !_.isPlainObject(options)) {
return $errUtils.throwErrByPath('get.invalid_options', {
args: { options },
})
Expand Down Expand Up @@ -497,7 +497,7 @@ module.exports = (Commands, Cypress, cy) => {

break
case 'existence':
return err.displayMessage = getErr(err)
return err.message = getErr(err)
default:
break
}
Expand Down
2 changes: 1 addition & 1 deletion packages/driver/src/cy/commands/screenshot.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->
isWin = $dom.isWindow(subject)

screenshotConfig = _.pick(options, "capture", "scale", "disableTimersAndAnimations", "blackout", "waitForCommandSynchronization", "padding", "clip", "onBeforeScreenshot", "onAfterScreenshot")
screenshotConfig = $Screenshot.validate(screenshotConfig, "cy.screenshot", options._log)
screenshotConfig = $Screenshot.validate(screenshotConfig, "screenshot", options._log)
screenshotConfig = _.extend($Screenshot.getConfig(), screenshotConfig)

## set this regardless of options.log b/c its used by the
Expand Down
4 changes: 2 additions & 2 deletions packages/driver/src/cy/commands/traversals.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,5 @@ module.exports = (Commands, Cypress, cy, state, config) ->
onFail: (err) ->
if err.type is "existence"
node = $dom.stringify(subject, "short")
err.displayMessage += " Queried from element: #{node}"
})
err.message += " Queried from element: #{node}"
})
2 changes: 1 addition & 1 deletion packages/driver/src/cy/commands/window.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->

orientationIsValidAndLandscape = (orientation) =>
if orientation not in validOrientations
all = validOrientations.join("' or '")
all = validOrientations.join("` or `")
$errUtils.throwErrByPath "viewport.invalid_orientation", {
onFail: options._log
args: { all, orientation }
Expand Down
6 changes: 4 additions & 2 deletions packages/driver/src/cy/ensures.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ create = (state, expect) ->

if types.length > 1
## append a nice error message telling the user this
err = $errUtils.appendErrMsg(err, "All #{types.length} subject validations failed on this subject.")
errProps = $errUtils.appendErrMsg(err, "All #{types.length} subject validations failed on this subject.")

$errUtils.mergeErrProps(err, errProps)

throw err

Expand Down Expand Up @@ -223,7 +225,7 @@ create = (state, expect) ->
## TODO: REFACTOR THIS TO CALL THE CHAI-OVERRIDES DIRECTLY
## OR GO THROUGH I18N

cy.ensureExistence($el)
ensureExistence($el)

ensureElDoesNotHaveCSS = ($el, cssProperty, cssValue, onFail) ->
cmd = state("current").get("name")
Expand Down
29 changes: 20 additions & 9 deletions packages/driver/src/cy/errors.coffee
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
_ = require("lodash")
$dom = require("../dom")
$errUtils = require("../cypress/error_utils")
$errorMessages = require('../cypress/error_messages')

crossOriginScriptRe = /^script error/i

Expand Down Expand Up @@ -37,28 +39,37 @@ create = (state, config, log) ->
msg = $errUtils.errMsgByPath("uncaught.cross_origin_script")

createErrFromMsg = ->
new Error $errUtils.errMsgByPath("uncaught.error", { msg, source, lineno })
new Error($errUtils.errMsgByPath("uncaught.error", {
msg, source, lineno
}))

## if we have the 5th argument it means we're in a super
## modern browser making this super simple to work with.
err ?= createErrFromMsg()

err.name = "Uncaught " + err.name

suffixMsg = switch type
uncaughtErrLookup = switch type
when "app" then "uncaught.fromApp"
when "spec" then "uncaught.fromSpec"

err = $errUtils.appendErrMsg(err, $errUtils.errMsgByPath(suffixMsg))
uncaughtErrObj = $errUtils.errObjByPath($errorMessages, uncaughtErrLookup)

err.name = "Uncaught " + err.name

uncaughtErrProps = $errUtils.modifyErrMsg(err, uncaughtErrObj.message, (msg1, msg2) ->
return "#{msg1}\n\n#{msg2}"
)
_.defaults(uncaughtErrProps, uncaughtErrObj)

uncaughtErr = $errUtils.mergeErrProps(err, uncaughtErrProps)

err.onFail = ->
uncaughtErr.onFail = ->
if l = current and current.getLastLog()
l.error(err)
l.error(uncaughtErr)

## normalize error message for firefox
$errUtils.normalizeErrorStack(err)
$errUtils.normalizeErrorStack(uncaughtErr)

return err
return uncaughtErr

commandRunningFailed = (err) ->
## allow for our own custom onFail function
Expand Down
24 changes: 16 additions & 8 deletions packages/driver/src/cy/retries.coffee
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
_ = require("lodash")
Promise = require("bluebird")
debug = require('debug')('cypress:driver:retries')

$utils = require("../cypress/utils")
$errUtils = require("../cypress/error_utils")

Expand Down Expand Up @@ -58,15 +59,22 @@ create = (Cypress, state, timeout, clearTimeout, whenStable, finishAssertions) -
if assertions = options.assertions
finishAssertions(assertions)

getErrMessage = (err) ->
_.get(err, 'displayMessage') or
_.get(err, 'message') or
err
{ error, onFail } = options

prependMsg = $errUtils.errMsgByPath("miscellaneous.retry_timed_out")

retryErrProps = $errUtils.modifyErrMsg(error, prependMsg, (msg1, msg2) ->
return "#{msg2}#{msg1}"
)

retryErr = $errUtils.mergeErrProps(error, retryErrProps)

if retryErr.type is "existence"
retryErr.showDiff = false
chrisbreiding marked this conversation as resolved.
Show resolved Hide resolved

$errUtils.throwErrByPath "miscellaneous.retry_timed_out", {
onFail: (options.onFail or log)
args: { error: getErrMessage(options.error) }
}
$errUtils.throwErr(retryErr, {
onFail: onFail or log
})

runnableHasChanged = ->
## if we've changed runnables don't retry!
Expand Down
6 changes: 3 additions & 3 deletions packages/driver/src/cypress.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ $Log.command = () => {
return $errUtils.throwErrByPath('miscellaneous.command_log_renamed')
}

const throwDeprecatedCommandInterface = (key, method) => {
const throwDeprecatedCommandInterface = (key = 'commandName', method) => {
let signature = ''

switch (method) {
Expand Down Expand Up @@ -504,7 +504,7 @@ class $Cypress {
// attaching long stace traces
// which otherwise make this err
// unusably long
const err = $errUtils.cloneErr(e)
const err = $errUtils.makeErrFromObj(e)

err.__stackCleaned__ = true
err.backend = true
Expand All @@ -526,7 +526,7 @@ class $Cypress {
const e = reply.error

if (e) {
const err = $errUtils.cloneErr(e)
const err = $errUtils.makeErrFromObj(e)

err.automation = true

Expand Down
14 changes: 8 additions & 6 deletions packages/driver/src/cypress/cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ const create = function (specWindow, Cypress, Cookies, state, config, log) {
const warnMixingPromisesAndCommands = function () {
const title = state('runnable').fullTitle()

const msg = $errUtils.errMsgByPath('miscellaneous.mixing_promises_and_commands', title)

return $utils.warning(msg)
$errUtils.warnByPath('miscellaneous.mixing_promises_and_commands', {
args: { title },
})
}

const $$ = function (selector, context) {
Expand Down Expand Up @@ -678,7 +678,9 @@ const create = function (specWindow, Cypress, Cookies, state, config, log) {

stopped = true

$errUtils.normalizeErrorStack(err)
err = $errUtils.normalizeErrorStack(err)

err = $errUtils.processErr(err, config)

// store the error on state now
state('error', err)
Expand Down Expand Up @@ -723,7 +725,7 @@ const create = function (specWindow, Cypress, Cookies, state, config, log) {
}

// bail if we had callbacks attached
if (rets.length) {
if (rets && rets.length) {
return
}

Expand Down Expand Up @@ -1269,7 +1271,7 @@ const create = function (specWindow, Cypress, Cookies, state, config, log) {
$utils.stringify(ret)

$errUtils.throwErrByPath('miscellaneous.returned_value_and_commands', {
args: ret,
args: { returned: ret },
})
}

Expand Down
Loading