Skip to content

Commit

Permalink
fix: add defensive checks to safely handle browser disconnects.
Browse files Browse the repository at this point in the history
This is a common occurrence when integrating with SauceLabs or BrowserStack.

* Add safety check to handle the case where a browser disconnects and
specSuccess/specSkipped/specFailure will throw an Error due to
suites[browser.id] returning undefined.
* Add safety check to handle the case where a browser disconnects and
onBrowserComplete receives a result object without total or failed values.
Causes XMLAttribute in xmlbuilder to throw an error.
* Add tests to validate new behavior works correctly and that no errors are thrown.
  • Loading branch information
larrymyers committed Jun 14, 2016
1 parent 908bc9d commit 485d87a
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
12 changes: 9 additions & 3 deletions index.js
Expand Up @@ -123,9 +123,9 @@ var JUnitReporter = function (baseReporterDecorator, config, logger, helper, for
return // don't die if browser didn't start
}

suite.att('tests', result.total)
suite.att('tests', result.total ? result.total : 0)
suite.att('errors', result.disconnected || result.error ? 1 : 0)
suite.att('failures', result.failed)
suite.att('failures', result.failed ? result.failed : 0)
suite.att('time', (result.netTime || 0) / 1000)

suite.ele('system-out').dat(allMessages.join() + '\n')
Expand All @@ -140,7 +140,13 @@ var JUnitReporter = function (baseReporterDecorator, config, logger, helper, for
}

this.specSuccess = this.specSkipped = this.specFailure = function (browser, result) {
var spec = suites[browser.id].ele('testcase', {
var testsuite = suites[browser.id]

if (!testsuite) {
return
}

var spec = testsuite.ele('testcase', {
name: nameFormatter(browser, result),
time: ((result.time || 0) / 1000),
classname: (typeof classNameFormatter === 'function' ? classNameFormatter : getClassName)(browser, result)
Expand Down
24 changes: 24 additions & 0 deletions test/reporter.spec.js
Expand Up @@ -80,4 +80,28 @@ describe('JUnit reporter', function () {
var writtenXml = fakeFs.writeFile.firstCall.args[1]
expect(writtenXml).to.have.string('testcase name="Sender using it get request should not fail"')
})

it('should safely handle missing suite browser entries when specSuccess fires', function () {
reporter.onRunStart([])

// don't try to call null.ele()
expect(reporter.specSuccess.bind(reporter, {id: 1}, {})).to.not.throw(TypeError)
})

it('should safely handle invalid test result objects when onBrowserComplete fires', function () {
var badBrowserResult = {
id: 'Android_4_1_2',
name: 'Android',
fullName: 'Android 4.1.2',
lastResult: {
error: true,
netTime: 0
}
}

reporter.onRunStart([ badBrowserResult ])

// never pass a null value to XMLAttribute via xmlbuilder attr()
expect(reporter.onBrowserComplete.bind(reporter, badBrowserResult)).not.to.throw(Error)
})
})

0 comments on commit 485d87a

Please sign in to comment.