Let auditors command fail if tracing failed (webdriverio#4416)

* let auditors command fail if tracing failed closes webdriverio#4085 * revert changes in examples

Author: christian-bromann

packages/wdio-devtools-service/src/auditor.js

@@ -67,10 +67,10 @@ export default class Auditor {
  * an Auditor instance is created for every trace so provide an updateCommands
  * function to receive the latest performance metrics with the browser instance
  */
- updateCommands (browser) {
+ updateCommands (browser, customFn) {
  const commands = Object.getOwnPropertyNames(Object.getPrototypeOf(this)).filter(
  fnName => fnName !== 'constructor' && fnName !== 'updateCommands' && !fnName.startsWith('_'))
- commands.forEach(fnName => browser.addCommand(fnName, ::this[fnName]))
+ commands.forEach(fnName => browser.addCommand(fnName, customFn || ::this[fnName]))
  }
 
  async getMainThreadWorkBreakdown () {

packages/wdio-devtools-service/src/gatherer/trace.js

@@ -99,6 +99,7 @@ export default class TraceGatherer extends EventEmitter {
  this.waitForCPUIdleEvent.cancel()
  this.frameId = '"unsuccessful loaded frame"'
  this.finishTracing()
+ this.emit('tracingError', new Error(`Page with url "${msgObj.frame.url}" failed to load`))
  return clearTimeout(this.clickTraceTimeout)
  }
 
@@ -249,6 +250,7 @@ export default class TraceGatherer extends EventEmitter {
  this.finishTracing()
  } catch (err) {
  log.error(`Error capturing tracing logs: ${err.stack}`)
+ this.emit('tracingError', err)
  return this.finishTracing()
  }
  }
@@ -273,6 +275,7 @@ export default class TraceGatherer extends EventEmitter {
  delete this.frameId
  delete this.loaderId
  delete this.pageUrl
+ this.failingFrameLoadIds = []
  this.waitForNetworkIdleEvent.cancel()
  this.waitForCPUIdleEvent.cancel()
  this.emit('tracingFinished')

packages/wdio-devtools-service/src/index.js

@@ -83,7 +83,7 @@ export default class DevToolsService {
  * allow user to work with Puppeteer directly
  */
  global.browser.addCommand('getPuppeteer',
- () => this.devtoolsDriver)
+ /* istanbul ignore next */ () => this.devtoolsDriver)
  }
 
  async beforeCommand (commandName, params) {
@@ -113,6 +113,13 @@ export default class DevToolsService {
  auditor.updateCommands(global.browser)
  })
 
+ this.traceGatherer.once('tracingError', (err) => {
+ const auditor = new Auditor()
+ auditor.updateCommands(global.browser, /* istanbul ignore next */ () => {
+ throw new Error(`Couldn't capture performance due to: ${err.message}`)
+ })
+ })
+
  return new Promise((resolve) => {
  log.info(`Wait until tracing for command ${commandName} finishes`)
 

packages/wdio-devtools-service/tests/gatherer/trace.test.js

@@ -139,7 +139,7 @@ test('onFrameNavigated: should cancel trace if page load failed', () => {
  traceGatherer.waitForNetworkIdleEvent = { cancel: jest.fn() }
  traceGatherer.waitForCPUIdleEvent = { cancel: jest.fn() }
  traceGatherer.onFrameNavigated({ frame })
- expect(traceGatherer.emit).toHaveBeenCalledTimes(0)
+ expect(traceGatherer.emit).toHaveBeenCalledWith('tracingError', expect.any(Error))
  expect(traceGatherer.finishTracing).toHaveBeenCalledTimes(1)
  expect(traceGatherer.waitForNetworkIdleEvent.cancel).toHaveBeenCalledTimes(1)
  expect(traceGatherer.waitForCPUIdleEvent.cancel).toHaveBeenCalledTimes(1)
@@ -189,7 +189,7 @@ test('completeTracing: in failure case', async () => {
  pageMock.tracing.stop.mockReturnValue(Promise.reject(new Error('boom')))
  await traceGatherer.completeTracing()
  expect(traceGatherer.finishTracing).toHaveBeenCalledTimes(1)
- expect(traceGatherer.emit).toHaveBeenCalledTimes(0)
+ expect(traceGatherer.emit).toBeCalledWith('tracingError', expect.any(Error))
 })
 
 test('onLoadEventFired', async () => {

packages/wdio-devtools-service/tests/service.test.js

@@ -175,13 +175,13 @@ test('afterCommand', () => {
  expect(service.traceGatherer.once).toBeCalledTimes(0)
 
  service.afterCommand('navigateTo')
- expect(service.traceGatherer.once).toBeCalledTimes(2)
+ expect(service.traceGatherer.once).toBeCalledTimes(3)
 
  service.afterCommand('url')
- expect(service.traceGatherer.once).toBeCalledTimes(4)
+ expect(service.traceGatherer.once).toBeCalledTimes(6)
 
  service.afterCommand('click')
- expect(service.traceGatherer.once).toBeCalledTimes(6)
+ expect(service.traceGatherer.once).toBeCalledTimes(9)
 })
 
 test('afterCommand: should create a new auditor instance and should update the browser commands', () => {
@@ -198,6 +198,20 @@ test('afterCommand: should create a new auditor instance and should update the b
  delete global.browser
 })
 
+test('afterCommand: should update browser commands even if failed', () => {
+ const service = new DevToolsService()
+ service.traceGatherer = new EventEmitter()
+ service.traceGatherer.isTracing = true
+ service.devtoolsGatherer = { getLogs: jest.fn() }
+ global.browser = 'some browser'
+ service.afterCommand('url')
+ service.traceGatherer.emit('tracingError', new Error('boom'))
+
+ const auditor = new Auditor()
+ expect(auditor.updateCommands).toBeCalledWith('some browser', expect.any(Function))
+ delete global.browser
+})
+
 test('afterCommand: should continue with command after tracingFinished was emitted', async () => {
  const service = new DevToolsService()
  service.traceGatherer = new EventEmitter()