Skip to content

Commit

Permalink
server: fixes #1192 pass reporter errors along and provide stack if m…
Browse files Browse the repository at this point in the history
…odule doesn't exist (#1411)
  • Loading branch information
brian-mann authored Mar 4, 2018
1 parent 934c35b commit 70e5916
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 25 deletions.
50 changes: 50 additions & 0 deletions packages/server/__snapshots__/reporters_spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ We searched for the reporter in these paths:
- /foo/bar/.projects/e2e/module-does-not-exist
- /foo/bar/.projects/e2e/node_modules/module-does-not-exist

The error we received was:

Cannot find module '/foo/bar/.projects/e2e/node_modules/module-does-not-exist'

Learn more at stack trace line
`

Expand Down Expand Up @@ -347,3 +351,49 @@ Because this error occurred during a 'before each' hook we are skipping the rema

`

exports['e2e reporters reports error when thrown from reporter 1'] = `Could not load reporter by name: reporters/throws.js

We searched for the reporter in these paths:

- /foo/bar/.projects/e2e/reporters/throws.js
- /foo/bar/.projects/e2e/node_modules/reporters/throws.js

The error we received was:

Error: this reporter threw an error
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line
at stack trace line


Learn more at stack trace line
`

10 changes: 7 additions & 3 deletions packages/server/lib/errors.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ listPaths = (paths) ->
API = {
# forms well-formatted user-friendly error for most common
# errors Cypress can encounter
getMsgByType: (type, arg1, arg2) ->
getMsgByType: (type, arg1 = {}, arg2 = {}) ->
switch type
when "CANNOT_TRASH_ASSETS"
"""
Expand Down Expand Up @@ -326,11 +326,15 @@ API = {
"""
when "INVALID_REPORTER_NAME"
"""
Could not load reporter by name: #{chalk.yellow(arg1)}
Could not load reporter by name: #{chalk.yellow(arg1.name)}
We searched for the reporter in these paths:
#{listPaths(arg2).join("\n")}
#{listPaths(arg1.paths).join("\n")}
The error we received was:
#{chalk.yellow(arg1.error)}
Learn more at https://on.cypress.io/reporters
"""
Expand Down
23 changes: 18 additions & 5 deletions packages/server/lib/project.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -221,14 +221,27 @@ class Project extends EE
watchSettingsAndStartWebsockets: (options = {}, cfg = {}) ->
@watchSettings(options.onSettingsChanged)

{ reporter, projectRoot } = cfg

## if we've passed down reporter
## then record these via mocha reporter
if cfg.report
if not Reporter.isValidReporterName(cfg.reporter, cfg.projectRoot)
paths = Reporter.getSearchPathsForReporter(cfg.reporter, cfg.projectRoot)
errors.throw("INVALID_REPORTER_NAME", cfg.reporter, paths)

reporter = Reporter.create(cfg.reporter, cfg.reporterOptions, cfg.projectRoot)
try
Reporter.loadReporter(reporter, projectRoot)
catch err
paths = Reporter.getSearchPathsForReporter(reporter, projectRoot)

## only include the message if this is the standard MODULE_NOT_FOUND
## else include the whole stack
errorMsg = if err.code is "MODULE_NOT_FOUND" then err.message else err.stack

errors.throw("INVALID_REPORTER_NAME", {
paths
error: errorMsg
name: reporter
})

reporter = Reporter.create(reporter, cfg.reporterOptions, projectRoot)

@automation = Automation.create(cfg.namespace, cfg.socketIoCookie, cfg.screenshotsFolder)

Expand Down
33 changes: 16 additions & 17 deletions packages/server/lib/reporter.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,8 @@ class Reporter
new Reporter(reporterName, reporterOptions, projectRoot)

@loadReporter = (reporterName, projectRoot) ->
debug("loading reporter #{reporterName}")
debug("trying to load reporter:", reporterName)

if r = reporters[reporterName]
debug("#{reporterName} is built-in reporter")
return require(r)
Expand All @@ -219,35 +220,33 @@ class Reporter
## that is local (./custom-reporter.js)
## or one installed by the user through npm
try
p = path.resolve(projectRoot, reporterName)

## try local
debug("loading local reporter by name #{reporterName}")
debug("trying to require local reporter with path:", p)

## using path.resolve() here so we can just pass an
## absolute path as the reporterName which avoids
## joining projectRoot unnecessarily
return require(path.resolve(projectRoot, reporterName))
return require(p)
catch err
if err.code isnt "MODULE_NOT_FOUND"
## bail early if the error wasn't MODULE_NOT_FOUND
## because that means theres something actually wrong
## with the found reporter
throw err

p = path.resolve(projectRoot, "node_modules", reporterName)

## try npm. if this fails, we're out of options, so let it throw
debug("loading NPM reporter module #{reporterName} from #{projectRoot}")
debug("trying to require local reporter with path:", p)

try
return require(path.resolve(projectRoot, "node_modules", reporterName))
catch err
msg = "Could not find reporter module #{reporterName} relative to #{projectRoot}"
throw new Error(msg)
return require(p)

@getSearchPathsForReporter = (reporterName, projectRoot) ->
_.uniq([
path.resolve(projectRoot, reporterName),
path.resolve(projectRoot, "node_modules", reporterName)
])

@isValidReporterName = (reporterName, projectRoot) ->
try
Reporter.loadReporter(reporterName, projectRoot)
debug("reporter #{reporterName} is valid name")
true
catch
false

module.exports = Reporter
9 changes: 9 additions & 0 deletions packages/server/test/e2e/reporters_spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ describe "e2e reporters", ->
reporter: "module-does-not-exist"
})

## https://github.com/cypress-io/cypress/issues/1192
it "reports error when thrown from reporter", ->
e2e.exec(@, {
spec: "simple_passing_spec.coffee"
snapshot: true
expectedExitCode: 1
reporter: "reporters/throws.js"
})

it "supports junit reporter and reporter options", ->
e2e.exec(@, {
spec: "simple_passing_spec.coffee"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
throw new Error('this reporter threw an error')

0 comments on commit 70e5916

Please sign in to comment.