Skip to content

Commit

Permalink
Invalidate stalled elements in ElementStore (webdriverio#5768)
Browse files Browse the repository at this point in the history
  • Loading branch information
mgrybyk authored Aug 28, 2020
1 parent b0a19d5 commit a676b4c
Show file tree
Hide file tree
Showing 19 changed files with 60 additions and 33 deletions.
2 changes: 1 addition & 1 deletion packages/devtools/src/commands/elementClear.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import command from '../scripts/elementClear'
import { getStaleElementError } from '../utils'

export default async function elementClear ({ elementId }) {
const elementHandle = this.elementStore.get(elementId)
const elementHandle = await this.elementStore.get(elementId)

if (!elementHandle) {
throw getStaleElementError(elementId)
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools/src/commands/elementClick.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const SELECT_SCRIPT = 'return (function select (elem) { elem.selected = true }).

export default async function elementClick ({ elementId }) {
const page = this.getPageHandle()
const elementHandle = this.elementStore.get(elementId)
const elementHandle = await this.elementStore.get(elementId)

if (!elementHandle) {
throw getStaleElementError(elementId)
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools/src/commands/elementSendKeys.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { getStaleElementError } from '../utils'
import path from 'path'

export default async function elementSendKeys ({ elementId, text }) {
const elementHandle = this.elementStore.get(elementId)
const elementHandle = await this.elementStore.get(elementId)

if (!elementHandle) {
throw getStaleElementError(elementId)
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools/src/commands/executeAsyncScript.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export default async function executeAsyncScript ({ script, args }) {
scriptTimeout,
SERIALIZE_PROPERTY,
SERIALIZE_FLAG,
...transformExecuteArgs.call(this, args)
...(await transformExecuteArgs.call(this, args))
)

return transformExecuteResult.call(this, page, result)
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools/src/commands/executeScript.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default async function executeScript ({ script, args }) {
script,
SERIALIZE_PROPERTY,
SERIALIZE_FLAG,
...transformExecuteArgs.call(this, args)
...(await transformExecuteArgs.call(this, args))
)

let executeTimeout
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools/src/commands/findElementFromElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default async function findElementFromElement ({ elementId, using, value
throw new Error(`selector strategy "${using}" is not yet supported`)
}

const elementHandle = this.elementStore.get(elementId)
const elementHandle = await this.elementStore.get(elementId)

if (!elementHandle) {
throw getStaleElementError(elementId)
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools/src/commands/findElementsFromElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default async function findElementFromElements ({ elementId, using, value
throw new Error(`selector strategy "${using}" is not yet supported`)
}

const elementHandle = this.elementStore.get(elementId)
const elementHandle = await this.elementStore.get(elementId)

if (!elementHandle) {
throw getStaleElementError(elementId)
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools/src/commands/getElementAttribute.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import command from '../scripts/getElementAttribute'
import { getStaleElementError } from '../utils'

export default async function getElementAttribute ({ elementId, name }) {
const elementHandle = this.elementStore.get(elementId)
const elementHandle = await this.elementStore.get(elementId)

if (!elementHandle) {
throw getStaleElementError(elementId)
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools/src/commands/getElementCSSValue.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import command from '../scripts/getElementCSSValue'
import { getStaleElementError } from '../utils'

export default async function getElementCSSValue ({ elementId, propertyName }) {
const elementHandle = this.elementStore.get(elementId)
const elementHandle = await this.elementStore.get(elementId)

if (!elementHandle) {
throw getStaleElementError(elementId)
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools/src/commands/getElementProperty.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import { getStaleElementError } from '../utils'

export default async function getElementProperty ({ elementId, name }) {
const elementHandle = this.elementStore.get(elementId)
const elementHandle = await this.elementStore.get(elementId)

if (!elementHandle) {
throw getStaleElementError(elementId)
Expand Down
4 changes: 2 additions & 2 deletions packages/devtools/src/commands/getElementRect.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
import command from '../scripts/getElementRect'
import { getStaleElementError } from '../utils'

export default function getElementRect ({ elementId }) {
const elementHandle = this.elementStore.get(elementId)
export default async function getElementRect ({ elementId }) {
const elementHandle = await this.elementStore.get(elementId)

if (!elementHandle) {
throw getStaleElementError(elementId)
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools/src/commands/getElementTagName.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import command from '../scripts/getElementTagName'
import { getStaleElementError } from '../utils'

export default async function getElementTagName ({ elementId }) {
const elementHandle = this.elementStore.get(elementId)
const elementHandle = await this.elementStore.get(elementId)

if (!elementHandle) {
throw getStaleElementError(elementId)
Expand Down
4 changes: 2 additions & 2 deletions packages/devtools/src/commands/getElementText.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
import command from '../scripts/getElementText'
import { getStaleElementError } from '../utils'

export default function getElementText ({ elementId }) {
const elementHandle = this.elementStore.get(elementId)
export default async function getElementText ({ elementId }) {
const elementHandle = await this.elementStore.get(elementId)

if (!elementHandle) {
throw getStaleElementError(elementId)
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools/src/commands/switchToFrame.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export default async function switchToFrame ({ id }) {
* switch frame by element ID
*/
if (typeof id[ELEMENT_KEY] === 'string') {
const elementHandle = this.elementStore.get(id[ELEMENT_KEY])
const elementHandle = await this.elementStore.get(id[ELEMENT_KEY])

if (!elementHandle) {
throw getStaleElementError(elementHandle)
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools/src/commands/takeElementScreenshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import { getStaleElementError } from '../utils'

export default async function takeElementScreenshot ({ elementId }) {
const elementHandle = this.elementStore.get(elementId)
const elementHandle = await this.elementStore.get(elementId)

if (!elementHandle) {
throw getStaleElementError(elementHandle)
Expand Down
15 changes: 13 additions & 2 deletions packages/devtools/src/elementstore.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,19 @@ export default class ElementStore {
return index
}

get (index) {
return this.elementMap.get(index)
async get (index) {
const elementHandle = this.elementMap.get(index)

if (!elementHandle) {
return elementHandle
}

const isElementAttachedToDOM = await elementHandle.evaluate(el => {
// https://developer.mozilla.org/en-US/docs/Web/API/Node/isConnected
return el.isConnected
})

return isElementAttachedToDOM ? elementHandle : undefined
}

clear () {
Expand Down
8 changes: 4 additions & 4 deletions packages/devtools/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,10 @@ export function sanitizeError (err) {
/**
* transform elements in argument list to Puppeteer element handles
*/
export function transformExecuteArgs (args = []) {
return args.map((arg) => {
export async function transformExecuteArgs (args = []) {
return Promise.all(args.map(async (arg) => {
if (arg[ELEMENT_KEY]) {
const elementHandle = this.elementStore.get(arg[ELEMENT_KEY])
const elementHandle = await this.elementStore.get(arg[ELEMENT_KEY])

if (!elementHandle) {
throw getStaleElementError(arg[ELEMENT_KEY])
Expand All @@ -175,7 +175,7 @@ export function transformExecuteArgs (args = []) {
}

return arg
})
}))
}

/**
Expand Down
26 changes: 21 additions & 5 deletions packages/devtools/tests/elementStore.test.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,29 @@
import ElementStore from '../src/elementstore'

test('should keep a map of elements', () => {
const elementHandleFactory = ({ isConnected = true } = {}) => ({
id: Math.random(),
async evaluate(cb) {
return cb({ isConnected })
}
})

test('should keep a map of elements', async () => {
const store = new ElementStore()
store.set('foobar')
expect(store.get('ELEMENT-1')).toBe('foobar')
const elementHandle1 = elementHandleFactory()
store.set(elementHandle1)
expect(await store.get('ELEMENT-1')).toBe(elementHandle1)

store.clear()
expect(store.elementMap.size).toBe(0)

store.set('barfoo')
expect(store.get('ELEMENT-2')).toBe('barfoo')
const elementHandle2 = elementHandleFactory()
store.set(elementHandle2)
expect(await store.get('ELEMENT-2')).toBe(elementHandle2)
})

test('should not return element if it is not attached to the DOM', async () => {
const store = new ElementStore()
const elementHandle = elementHandleFactory({ isConnected: false })
store.set(elementHandle)
expect(await store.get('ELEMENT-3')).toBe(undefined)
})
10 changes: 5 additions & 5 deletions packages/devtools/tests/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,28 +268,28 @@ describe('sanitizeError', () => {
})
})

test('transformExecuteArgs', () => {
test('transformExecuteArgs', async () => {
const scope = { elementStore: new Map() }
scope.elementStore.set('foobar', 'barfoo')

expect(transformExecuteArgs.call(scope, [
expect(await transformExecuteArgs.call(scope, [
'foo',
{ 'element-6066-11e4-a52e-4f735466cecf': 'foobar' },
true,
42
])).toEqual(['foo', 'barfoo', true, 42])
})

test('transformExecuteArgs throws stale element if element is not in store', () => {
test('transformExecuteArgs throws stale element if element is not in store', async () => {
const scope = { elementStore: new Map() }
scope.elementStore.set('foobar', 'barfoo')

expect(() => transformExecuteArgs.call(scope, [
expect(transformExecuteArgs.call(scope, [
'foo',
{ 'element-6066-11e4-a52e-4f735466cecf': 'not-existing' },
true,
42
])).toThrow()
])).rejects.toThrow()
})

describe('transformExecuteResult', () => {
Expand Down

0 comments on commit a676b4c

Please sign in to comment.