diff --git a/docs/element-selection.md b/docs/element-selection.md index f8441b17c..556962140 100644 --- a/docs/element-selection.md +++ b/docs/element-selection.md @@ -115,11 +115,33 @@ And when you know there are multiple matches and want a specific one, `elementIn I.click('a', step.opts({ elementIndex: 2 })) ``` +## Text Passed Instead of a Selector + +`waitForElement`, `seeElement`, `waitForVisible` and the rest of the wait/assert family expect a CSS or XPath locator. Unlike `click` or `fillField`, they don't fall back to searching by text. A sentence passed to them is treated as CSS, matches nothing, and the step fails with a timeout that says nothing about the real cause: + +```js +I.waitForElement('Description Persistence Suite') // waits 10s, then "still not present on page" +``` + +CodeceptJS detects this and warns when the run is in debug mode: + +``` +I wait for element "Description Persistence Suite" + › [Warning] "Description Persistence Suite" doesn't look like a CSS or XPath selector. + I.waitForElement() expects an element locator, so this text is matched as CSS + and finds nothing. Use I.waitForText() to wait for a text on page. +``` + +With `strict: true` the same check throws `InvalidSelector` instead of warning, so the test fails immediately with a readable message rather than after the full timeout. + +The check only fires on strings that can't be a selector: they contain a space, carry no CSS or XPath punctuation, and aren't a chain of tag names. `div span`, `my-app my-button`, `text=Save Changes` and `~accessibility id` are all left alone. + ## Summary | Situation | Approach | |-----------|----------| | You want to catch ambiguous locators early | Enable `strict: true` in helper config | +| You passed a text where a selector is expected | Run with `--debug` for the warning, or `strict: true` to fail fast | | You need a specific element from a known list | Use `step.opts({ elementIndex: N })` | | You want to iterate over all matching elements | Use [`eachElement`](/els) from the `els` module | | You need full control over element inspection | Use [`grabWebElements`](/WebElement) to get all matches | diff --git a/lib/helper/Playwright.js b/lib/helper/Playwright.js index 939988c9a..db09f9af2 100644 --- a/lib/helper/Playwright.js +++ b/lib/helper/Playwright.js @@ -55,6 +55,7 @@ import { setRestartStrategy, restartsSession, restartsContext, restartsBrowser } import { createValueEngine, createDisabledEngine } from './extras/PlaywrightPropEngine.js' import { seeElementError, dontSeeElementError, dontSeeElementInDOMError, seeElementInDOMError } from './errors/ElementAssertion.js' import { dontSeeTraffic, seeTraffic, grabRecordedNetworkTraffics, stopRecordingTraffic, flushNetworkTraffics } from './network/actions.js' +import { checkSelectorIsNotText } from './extras/selectorCheck.js' const pathSeparator = path.sep @@ -1499,6 +1500,7 @@ class Playwright extends Helper { * */ async moveCursorTo(locator, offsetX = 0, offsetY = 0) { + checkSelectorIsNotText(this, locator) let context = null if (typeof offsetX !== 'number') { context = offsetX @@ -1672,6 +1674,7 @@ class Playwright extends Helper { * {{> scrollTo }} */ async scrollTo(locator, offsetX = 0, offsetY = 0) { + checkSelectorIsNotText(this, locator) if (typeof locator === 'number' && typeof offsetX === 'number') { offsetY = offsetX offsetX = locator @@ -1829,6 +1832,7 @@ class Playwright extends Helper { * */ async grabWebElements(locator) { + checkSelectorIsNotText(this, locator) const elements = await this._locate(locator) return elements.map(element => new WebElement(element, this)) } @@ -1838,6 +1842,7 @@ class Playwright extends Helper { * */ async grabWebElement(locator) { + checkSelectorIsNotText(this, locator) const element = await this._locateElement(locator) return new WebElement(element, this) } @@ -1967,6 +1972,7 @@ class Playwright extends Helper { * */ async seeElement(locator, context = null) { + checkSelectorIsNotText(this, locator) let els if (context) { const contextEls = await this._locate(context) @@ -1988,6 +1994,7 @@ class Playwright extends Helper { * */ async dontSeeElement(locator, context = null) { + checkSelectorIsNotText(this, locator) let els if (context) { const contextEls = await this._locate(context) @@ -2008,6 +2015,7 @@ class Playwright extends Helper { * {{> seeElementInDOM }} */ async seeElementInDOM(locator) { + checkSelectorIsNotText(this, locator) const els = await this._locate(locator) try { return empty('elements on page').negate(els.filter(v => v).fill('ELEMENT')) @@ -2020,6 +2028,7 @@ class Playwright extends Helper { * {{> dontSeeElementInDOM }} */ async dontSeeElementInDOM(locator) { + checkSelectorIsNotText(this, locator) const els = await this._locate(locator) try { return empty('elements on a page').assert(els.filter(v => v).fill('ELEMENT')) @@ -2415,6 +2424,7 @@ class Playwright extends Helper { * */ async grabNumberOfVisibleElements(locator) { + checkSelectorIsNotText(this, locator) let els = await this._locate(locator) els = await Promise.all(els.map(el => el.isVisible())) return els.filter(v => v).length @@ -2546,6 +2556,7 @@ class Playwright extends Helper { * */ async seeNumberOfElements(locator, num) { + checkSelectorIsNotText(this, locator) const elements = await this._locate(locator) return equals(`expected number of elements (${new Locator(locator)}) is ${num}, but found ${elements.length}`).assert(elements.length, num) } @@ -2556,6 +2567,7 @@ class Playwright extends Helper { * */ async seeNumberOfVisibleElements(locator, num) { + checkSelectorIsNotText(this, locator) const res = await this.grabNumberOfVisibleElements(locator) return equals(`expected number of visible elements (${new Locator(locator)}) is ${num}, but found ${res}`).assert(res, num) } @@ -2704,6 +2716,7 @@ class Playwright extends Helper { * */ async grabTextFrom(locator) { + checkSelectorIsNotText(this, locator) const roleElements = await handleRoleLocator(this.page, locator) if (roleElements && roleElements.length > 0) { const text = await roleElements[0].textContent() @@ -2734,6 +2747,7 @@ class Playwright extends Helper { * */ async grabTextFromAll(locator) { + checkSelectorIsNotText(this, locator) const els = await this._locate(locator) const texts = [] for (const el of els) { @@ -2764,6 +2778,7 @@ class Playwright extends Helper { * {{> grabHTMLFrom }} */ async grabHTMLFrom(locator) { + checkSelectorIsNotText(this, locator) const html = await this.grabHTMLFromAll(locator) assertElementExists(html, locator) this.debugSection('HTML', html[0]) @@ -2774,6 +2789,7 @@ class Playwright extends Helper { * {{> grabHTMLFromAll }} */ async grabHTMLFromAll(locator) { + checkSelectorIsNotText(this, locator) const els = await this._locate(locator) return Promise.all(els.map(el => el.innerHTML())) } @@ -2783,6 +2799,7 @@ class Playwright extends Helper { * */ async grabCssPropertyFrom(locator, cssProperty) { + checkSelectorIsNotText(this, locator) const cssValues = await this.grabCssPropertyFromAll(locator, cssProperty) assertElementExists(cssValues, locator) this.debugSection('CSS', cssValues[0]) @@ -2794,6 +2811,7 @@ class Playwright extends Helper { * */ async grabCssPropertyFromAll(locator, cssProperty) { + checkSelectorIsNotText(this, locator) const els = await this._locate(locator) const cssValues = await Promise.all(els.map(el => el.evaluate((el, cssProperty) => getComputedStyle(el).getPropertyValue(cssProperty), cssProperty))) @@ -2805,6 +2823,7 @@ class Playwright extends Helper { * */ async seeCssPropertiesOnElements(locator, cssProperties) { + checkSelectorIsNotText(this, locator) const res = await this._locate(locator) assertElementExists(res, locator) @@ -2840,6 +2859,7 @@ class Playwright extends Helper { * */ async seeAttributesOnElements(locator, attributes) { + checkSelectorIsNotText(this, locator) const res = await this._locate(locator) assertElementExists(res, locator) @@ -2893,6 +2913,7 @@ class Playwright extends Helper { * */ async grabAttributeFrom(locator, attr) { + checkSelectorIsNotText(this, locator) const attrs = await this.grabAttributeFromAll(locator, attr) assertElementExists(attrs, locator) this.debugSection('Attribute', attrs[0]) @@ -2904,6 +2925,7 @@ class Playwright extends Helper { * */ async grabAttributeFromAll(locator, attr) { + checkSelectorIsNotText(this, locator) const els = await this._locate(locator) const array = [] @@ -2946,6 +2968,7 @@ class Playwright extends Helper { * */ async saveElementScreenshot(locator, fileName) { + checkSelectorIsNotText(this, locator) const outputFile = screenshotOutputFolder(fileName) const res = await this._locateElement(locator) @@ -3162,6 +3185,7 @@ class Playwright extends Helper { * {{> waitForEnabled }} */ async waitForEnabled(locator, sec) { + checkSelectorIsNotText(this, locator) const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout locator = new Locator(locator, 'css') @@ -3188,6 +3212,7 @@ class Playwright extends Helper { * {{> waitForDisabled }} */ async waitForDisabled(locator, sec) { + checkSelectorIsNotText(this, locator) const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout locator = new Locator(locator, 'css') @@ -3244,6 +3269,7 @@ class Playwright extends Helper { * */ async waitNumberOfVisibleElements(locator, num, sec) { + checkSelectorIsNotText(this, locator) const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout locator = new Locator(locator, 'css') @@ -3285,6 +3311,7 @@ class Playwright extends Helper { * */ async waitForElement(locator, sec) { + checkSelectorIsNotText(this, locator) const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout locator = new Locator(locator, 'css') @@ -3300,6 +3327,7 @@ class Playwright extends Helper { * {{> waitForVisible }} */ async waitForVisible(locator, sec) { + checkSelectorIsNotText(this, locator) const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout locator = new Locator(locator, 'css') @@ -3330,6 +3358,7 @@ class Playwright extends Helper { * {{> waitForInvisible }} */ async waitForInvisible(locator, sec) { + checkSelectorIsNotText(this, locator) const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout locator = new Locator(locator, 'css') @@ -3361,6 +3390,7 @@ class Playwright extends Helper { * {{> waitToHide }} */ async waitToHide(locator, sec) { + checkSelectorIsNotText(this, locator) const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout locator = new Locator(locator, 'css') @@ -3741,6 +3771,7 @@ class Playwright extends Helper { * {{> waitForDetached }} */ async waitForDetached(locator, sec) { + checkSelectorIsNotText(this, locator) const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout locator = new Locator(locator, 'css') @@ -3817,6 +3848,7 @@ class Playwright extends Helper { * {{> grabElementBoundingRect }} */ async grabElementBoundingRect(locator, prop) { + checkSelectorIsNotText(this, locator) const el = await this._locateElement(locator) assertElementExists(el, locator) const rect = await el.boundingBox() diff --git a/lib/helper/Puppeteer.js b/lib/helper/Puppeteer.js index ff00f6dd8..f89ec8795 100644 --- a/lib/helper/Puppeteer.js +++ b/lib/helper/Puppeteer.js @@ -46,6 +46,7 @@ import { dontSeeTraffic, seeTraffic, grabRecordedNetworkTraffics, stopRecordingT import WebElement from '../element/WebElement.js' import { selectElement } from './extras/elementSelection.js' import { fillRichEditor } from './extras/richTextEditor.js' +import { checkSelectorIsNotText } from './extras/selectorCheck.js' let puppeteer @@ -823,6 +824,7 @@ class Puppeteer extends Helper { * {{> moveCursorTo }} */ async moveCursorTo(locator, offsetX = 0, offsetY = 0) { + checkSelectorIsNotText(this, locator) let context = null if (typeof offsetX !== 'number') { context = offsetX @@ -918,6 +920,7 @@ class Puppeteer extends Helper { * {{> scrollTo }} */ async scrollTo(locator, offsetX = 0, offsetY = 0) { + checkSelectorIsNotText(this, locator) if (typeof locator === 'number' && typeof offsetX === 'number') { offsetY = offsetX offsetX = locator @@ -1064,6 +1067,7 @@ class Puppeteer extends Helper { * */ async grabWebElements(locator) { + checkSelectorIsNotText(this, locator) const elements = await this._locate(locator) return elements.map(element => new WebElement(element, this)) } @@ -1073,6 +1077,7 @@ class Puppeteer extends Helper { * */ async grabWebElement(locator) { + checkSelectorIsNotText(this, locator) const elements = await this._locate(locator) if (elements.length === 0) { throw new ElementNotFound(locator, 'Element') @@ -1081,6 +1086,7 @@ class Puppeteer extends Helper { } async grabWebElement(locator) { + checkSelectorIsNotText(this, locator) const els = await this._locate(locator) assertElementExists(els, locator) return els[0] @@ -1190,6 +1196,7 @@ class Puppeteer extends Helper { * {{> seeElement }} */ async seeElement(locator, context = null) { + checkSelectorIsNotText(this, locator) let els if (context) { const contextPage = await this.context @@ -1213,6 +1220,7 @@ class Puppeteer extends Helper { * {{> dontSeeElement }} */ async dontSeeElement(locator, context = null) { + checkSelectorIsNotText(this, locator) let els if (context) { const contextPage = await this.context @@ -1236,6 +1244,7 @@ class Puppeteer extends Helper { * {{> seeElementInDOM }} */ async seeElementInDOM(locator) { + checkSelectorIsNotText(this, locator) const els = await this._locate(locator) try { return empty('elements on page').negate(els.filter(v => v).fill('ELEMENT')) @@ -1248,6 +1257,7 @@ class Puppeteer extends Helper { * {{> dontSeeElementInDOM }} */ async dontSeeElementInDOM(locator) { + checkSelectorIsNotText(this, locator) const els = await this._locate(locator) try { return empty('elements on a page').assert(els.filter(v => v).fill('ELEMENT')) @@ -1724,6 +1734,7 @@ class Puppeteer extends Helper { * {{> grabNumberOfVisibleElements }} */ async grabNumberOfVisibleElements(locator) { + checkSelectorIsNotText(this, locator) let els = await this._locate(locator) els = (await Promise.all(els.map(el => el.boundingBox() && el))).filter(v => v) // Puppeteer visibility was ignored? | Remove when Puppeteer is fixed @@ -1853,6 +1864,7 @@ class Puppeteer extends Helper { * */ async seeNumberOfElements(locator, num) { + checkSelectorIsNotText(this, locator) const elements = await this._locate(locator) return equals(`expected number of elements (${new Locator(locator)}) is ${num}, but found ${elements.length}`).assert(elements.length, num) } @@ -1862,6 +1874,7 @@ class Puppeteer extends Helper { * */ async seeNumberOfVisibleElements(locator, num) { + checkSelectorIsNotText(this, locator) const res = await this.grabNumberOfVisibleElements(locator) return equals(`expected number of visible elements (${new Locator(locator)}) is ${num}, but found ${res}`).assert(res, num) } @@ -1988,6 +2001,7 @@ class Puppeteer extends Helper { * {{> grabTextFromAll }} */ async grabTextFromAll(locator) { + checkSelectorIsNotText(this, locator) const els = await this._locate(locator) const texts = [] for (const el of els) { @@ -2000,6 +2014,7 @@ class Puppeteer extends Helper { * {{> grabTextFrom }} */ async grabTextFrom(locator) { + checkSelectorIsNotText(this, locator) const texts = await this.grabTextFromAll(locator) assertElementExists(texts, locator) if (texts.length > 1) { @@ -2038,6 +2053,7 @@ class Puppeteer extends Helper { * {{> grabHTMLFromAll }} */ async grabHTMLFromAll(locator) { + checkSelectorIsNotText(this, locator) const els = await this._locate(locator) const values = await Promise.all(els.map(el => el.evaluate(element => element.innerHTML))) return values @@ -2047,6 +2063,7 @@ class Puppeteer extends Helper { * {{> grabHTMLFrom }} */ async grabHTMLFrom(locator) { + checkSelectorIsNotText(this, locator) const html = await this.grabHTMLFromAll(locator) assertElementExists(html, locator) if (html.length > 1) { @@ -2060,6 +2077,7 @@ class Puppeteer extends Helper { * {{> grabCssPropertyFromAll }} */ async grabCssPropertyFromAll(locator, cssProperty) { + checkSelectorIsNotText(this, locator) const els = await this._locate(locator) const res = await Promise.all(els.map(el => el.evaluate(el => JSON.parse(JSON.stringify(getComputedStyle(el)))))) const cssValues = res.map(props => props[toCamelCase(cssProperty)]) @@ -2071,6 +2089,7 @@ class Puppeteer extends Helper { * {{> grabCssPropertyFrom }} */ async grabCssPropertyFrom(locator, cssProperty) { + checkSelectorIsNotText(this, locator) const cssValues = await this.grabCssPropertyFromAll(locator, cssProperty) assertElementExists(cssValues, locator) @@ -2085,6 +2104,7 @@ class Puppeteer extends Helper { * {{> seeCssPropertiesOnElements }} */ async seeCssPropertiesOnElements(locator, cssProperties) { + checkSelectorIsNotText(this, locator) const res = await this._locate(locator) assertElementExists(res, locator) @@ -2119,6 +2139,7 @@ class Puppeteer extends Helper { * {{> seeAttributesOnElements }} */ async seeAttributesOnElements(locator, attributes) { + checkSelectorIsNotText(this, locator) const elements = await this._locate(locator) assertElementExists(elements, locator) @@ -2177,6 +2198,7 @@ class Puppeteer extends Helper { * {{> grabAttributeFromAll }} */ async grabAttributeFromAll(locator, attr) { + checkSelectorIsNotText(this, locator) const els = await this._locate(locator) const array = [] for (let index = 0; index < els.length; index++) { @@ -2190,6 +2212,7 @@ class Puppeteer extends Helper { * {{> grabAttributeFrom }} */ async grabAttributeFrom(locator, attr) { + checkSelectorIsNotText(this, locator) const attrs = await this.grabAttributeFromAll(locator, attr) assertElementExists(attrs, locator) if (attrs.length > 1) { @@ -2203,6 +2226,7 @@ class Puppeteer extends Helper { * {{> saveElementScreenshot }} */ async saveElementScreenshot(locator, fileName) { + checkSelectorIsNotText(this, locator) const outputFile = screenshotOutputFolder(fileName) const res = await this._locate(locator) @@ -2291,6 +2315,7 @@ class Puppeteer extends Helper { * {{> waitForEnabled }} */ async waitForEnabled(locator, sec) { + checkSelectorIsNotText(this, locator) const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout locator = new Locator(locator, 'css') await this.context @@ -2352,6 +2377,7 @@ class Puppeteer extends Helper { * {{> waitNumberOfVisibleElements }} */ async waitNumberOfVisibleElements(locator, num, sec) { + checkSelectorIsNotText(this, locator) const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout locator = new Locator(locator, 'css') let waiter @@ -2381,6 +2407,7 @@ class Puppeteer extends Helper { * {{> waitForClickable }} */ async waitForClickable(locator, waitTimeout) { + checkSelectorIsNotText(this, locator) const el = await this._locateElement(locator) if (!el) { throw new ElementNotFound(locator, 'Element to wait for clickable') @@ -2399,6 +2426,7 @@ class Puppeteer extends Helper { * {{> waitForElement }} */ async waitForElement(locator, sec) { + checkSelectorIsNotText(this, locator) const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout locator = new Locator(locator, 'css') @@ -2419,6 +2447,7 @@ class Puppeteer extends Helper { * */ async waitForVisible(locator, sec) { + checkSelectorIsNotText(this, locator) const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout locator = new Locator(locator, 'css') await this.context @@ -2438,6 +2467,7 @@ class Puppeteer extends Helper { * {{> waitForInvisible }} */ async waitForInvisible(locator, sec) { + checkSelectorIsNotText(this, locator) const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout locator = new Locator(locator, 'css') await this.context @@ -2457,6 +2487,7 @@ class Puppeteer extends Helper { * {{> waitToHide }} */ async waitToHide(locator, sec) { + checkSelectorIsNotText(this, locator) const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout locator = new Locator(locator, 'css') let waiter @@ -2740,6 +2771,7 @@ class Puppeteer extends Helper { * {{> waitForDetached }} */ async waitForDetached(locator, sec) { + checkSelectorIsNotText(this, locator) const waitTimeout = sec ? sec * 1000 : this.options.waitForTimeout locator = new Locator(locator, 'css') @@ -2777,6 +2809,7 @@ class Puppeteer extends Helper { * {{> grabElementBoundingRect }} */ async grabElementBoundingRect(locator, prop) { + checkSelectorIsNotText(this, locator) const els = await this._locate(locator) assertElementExists(els, locator) const rect = await els[0].boundingBox() diff --git a/lib/helper/WebDriver.js b/lib/helper/WebDriver.js index 62c0b4dc2..7a689ab2c 100644 --- a/lib/helper/WebDriver.js +++ b/lib/helper/WebDriver.js @@ -43,6 +43,7 @@ import { dontSeeTraffic, seeTraffic, grabRecordedNetworkTraffics, stopRecordingT import WebElement from '../element/WebElement.js' import { selectElement } from './extras/elementSelection.js' import { fillRichEditor } from './extras/richTextEditor.js' +import { checkSelectorIsNotText } from './extras/selectorCheck.js' const SHADOW = 'shadow' const webRoot = 'body' @@ -1018,6 +1019,7 @@ class WebDriver extends Helper { * */ async grabWebElements(locator) { + checkSelectorIsNotText(this, locator) const elements = await this._locate(locator) return elements.map(element => new WebElement(element, this)) } @@ -1027,6 +1029,7 @@ class WebDriver extends Helper { * */ async grabWebElement(locator) { + checkSelectorIsNotText(this, locator) const elements = await this._locate(locator) if (elements.length === 0) { throw new ElementNotFound(locator, 'Element') @@ -1424,6 +1427,7 @@ class WebDriver extends Helper { * */ async grabTextFromAll(locator) { + checkSelectorIsNotText(this, locator) const res = await this._locate(locator, true) let val = [] await forEachAsync(res, async el => { @@ -1439,6 +1443,7 @@ class WebDriver extends Helper { * */ async grabTextFrom(locator) { + checkSelectorIsNotText(this, locator) const texts = await this.grabTextFromAll(locator) assertElementExists(texts, locator) if (texts.length > 1) { @@ -1453,6 +1458,7 @@ class WebDriver extends Helper { * */ async grabHTMLFromAll(locator) { + checkSelectorIsNotText(this, locator) const elems = await this._locate(locator, true) const html = await forEachAsync(elems, elem => elem.getHTML(false)) this.debugSection('GrabHTML', String(html)) @@ -1464,6 +1470,7 @@ class WebDriver extends Helper { * */ async grabHTMLFrom(locator) { + checkSelectorIsNotText(this, locator) const html = await this.grabHTMLFromAll(locator) assertElementExists(html, locator) if (html.length > 1) { @@ -1503,6 +1510,7 @@ class WebDriver extends Helper { * {{> grabCssPropertyFromAll }} */ async grabCssPropertyFromAll(locator, cssProperty) { + checkSelectorIsNotText(this, locator) const res = await this._locate(locator, true) const val = await forEachAsync(res, async el => this.browser.getElementCSSValue(getElementId(el), cssProperty)) this.debugSection('Grab', String(val)) @@ -1513,6 +1521,7 @@ class WebDriver extends Helper { * {{> grabCssPropertyFrom }} */ async grabCssPropertyFrom(locator, cssProperty) { + checkSelectorIsNotText(this, locator) const cssValues = await this.grabCssPropertyFromAll(locator, cssProperty) assertElementExists(cssValues, locator) @@ -1527,6 +1536,7 @@ class WebDriver extends Helper { * {{> grabAttributeFromAll }} */ async grabAttributeFromAll(locator, attr) { + checkSelectorIsNotText(this, locator) const res = await this._locate(locator, true) const val = await forEachAsync(res, async el => el.getAttribute(attr)) this.debugSection('GrabAttribute', String(val)) @@ -1537,6 +1547,7 @@ class WebDriver extends Helper { * {{> grabAttributeFrom }} */ async grabAttributeFrom(locator, attr) { + checkSelectorIsNotText(this, locator) const attrs = await this.grabAttributeFromAll(locator, attr) assertElementExists(attrs, locator) if (attrs.length > 1) { @@ -1640,6 +1651,7 @@ class WebDriver extends Helper { * */ async seeElement(locator, context = null) { + checkSelectorIsNotText(this, locator) const locateFn = prepareLocateFn.call(this, context) const res = context ? await locateFn(locator) : await this._locate(locator, true) assertElementExists(res, locator) @@ -1655,6 +1667,7 @@ class WebDriver extends Helper { * {{> dontSeeElement }} */ async dontSeeElement(locator, context = null) { + checkSelectorIsNotText(this, locator) const locateFn = prepareLocateFn.call(this, context) const res = context ? await locateFn(locator) : await this._locate(locator, false) if (!res || res.length === 0) { @@ -1673,6 +1686,7 @@ class WebDriver extends Helper { * */ async seeElementInDOM(locator) { + checkSelectorIsNotText(this, locator) const res = await this._res(locator) try { return empty('elements').negate(res) @@ -1686,6 +1700,7 @@ class WebDriver extends Helper { * */ async dontSeeElementInDOM(locator) { + checkSelectorIsNotText(this, locator) const res = await this._res(locator) try { return empty('elements').assert(res) @@ -1739,6 +1754,7 @@ class WebDriver extends Helper { * {{> seeNumberOfElements }} */ async seeNumberOfElements(locator, num) { + checkSelectorIsNotText(this, locator) const res = await this._locate(locator) return assert.equal(res.length, num, `expected number of elements (${new Locator(locator)}) is ${num}, but found ${res.length}`) } @@ -1747,6 +1763,7 @@ class WebDriver extends Helper { * {{> seeNumberOfVisibleElements }} */ async seeNumberOfVisibleElements(locator, num) { + checkSelectorIsNotText(this, locator) const res = await this.grabNumberOfVisibleElements(locator) return assert.equal(res, num, `expected number of visible elements (${new Locator(locator)}) is ${num}, but found ${res}`) } @@ -1755,6 +1772,7 @@ class WebDriver extends Helper { * {{> seeCssPropertiesOnElements }} */ async seeCssPropertiesOnElements(locator, cssProperties) { + checkSelectorIsNotText(this, locator) const res = await this._locate(locator) assertElementExists(res, locator) @@ -1789,6 +1807,7 @@ class WebDriver extends Helper { * {{> seeAttributesOnElements }} */ async seeAttributesOnElements(locator, attributes) { + checkSelectorIsNotText(this, locator) const res = await this._locate(locator) assertElementExists(res, locator) const elemAmount = res.length @@ -1817,6 +1836,7 @@ class WebDriver extends Helper { * {{> grabNumberOfVisibleElements }} */ async grabNumberOfVisibleElements(locator) { + checkSelectorIsNotText(this, locator) const res = await this._locate(locator) let selected = await forEachAsync(res, async el => el.isDisplayed()) @@ -1903,6 +1923,7 @@ class WebDriver extends Helper { * */ async scrollIntoView(locator, scrollIntoViewOptions) { + checkSelectorIsNotText(this, locator) const res = await this._locate(withStrictLocator(locator), true) assertElementExists(res, locator) const elem = usingFirstElement(res) @@ -1914,6 +1935,7 @@ class WebDriver extends Helper { * */ async scrollTo(locator, offsetX = 0, offsetY = 0) { + checkSelectorIsNotText(this, locator) if (typeof locator === 'number' && typeof offsetX === 'number') { offsetY = offsetX offsetX = locator @@ -1953,6 +1975,7 @@ class WebDriver extends Helper { * {{> moveCursorTo }} */ async moveCursorTo(locator, xOffset, yOffset) { + checkSelectorIsNotText(this, locator) let context = null if (typeof xOffset !== 'number' && xOffset !== undefined) { context = xOffset @@ -1982,6 +2005,7 @@ class WebDriver extends Helper { * */ async saveElementScreenshot(locator, fileName) { + checkSelectorIsNotText(this, locator) const outputFile = screenshotOutputFolder(fileName) const res = await this._locate(withStrictLocator(locator), true) @@ -2457,6 +2481,7 @@ class WebDriver extends Helper { * {{> waitForEnabled }} */ async waitForEnabled(locator, sec = null) { + checkSelectorIsNotText(this, locator) const aSec = sec || this.options.waitForTimeoutInSeconds return this.browser.waitUntil( @@ -2482,6 +2507,7 @@ class WebDriver extends Helper { * {{> waitForElement }} */ async waitForElement(locator, sec = null) { + checkSelectorIsNotText(this, locator) const aSec = sec || this.options.waitForTimeoutInSeconds return this.browser.waitUntil( @@ -2500,6 +2526,7 @@ class WebDriver extends Helper { * {{> waitForClickable }} */ async waitForClickable(locator, waitTimeout) { + checkSelectorIsNotText(this, locator) waitTimeout = waitTimeout || this.options.waitForTimeoutInSeconds let res = await this._locate(locator) res = usingFirstElement(res) @@ -2648,6 +2675,7 @@ class WebDriver extends Helper { * */ async waitForVisible(locator, sec = null) { + checkSelectorIsNotText(this, locator) const aSec = sec || this.options.waitForTimeoutInSeconds return this.browser.waitUntil( @@ -2671,6 +2699,7 @@ class WebDriver extends Helper { * {{> waitNumberOfVisibleElements }} */ async waitNumberOfVisibleElements(locator, num, sec = null) { + checkSelectorIsNotText(this, locator) const aSec = sec || this.options.waitForTimeoutInSeconds return this.browser @@ -2698,6 +2727,7 @@ class WebDriver extends Helper { * {{> waitForInvisible }} */ async waitForInvisible(locator, sec = null) { + checkSelectorIsNotText(this, locator) const aSec = sec || this.options.waitForTimeoutInSeconds return this.browser.waitUntil( @@ -2715,6 +2745,7 @@ class WebDriver extends Helper { * {{> waitToHide }} */ async waitToHide(locator, sec = null) { + checkSelectorIsNotText(this, locator) return this.waitForInvisible(locator, sec) } @@ -2722,6 +2753,7 @@ class WebDriver extends Helper { * {{> waitForDetached }} */ async waitForDetached(locator, sec = null) { + checkSelectorIsNotText(this, locator) const aSec = sec || this.options.waitForTimeoutInSeconds return this.browser.waitUntil( @@ -2913,6 +2945,7 @@ class WebDriver extends Helper { * {{> grabElementBoundingRect }} */ async grabElementBoundingRect(locator, prop) { + checkSelectorIsNotText(this, locator) const res = await this._locate(locator, true) assertElementExists(res, locator) const el = usingFirstElement(res) diff --git a/lib/helper/errors/InvalidSelector.js b/lib/helper/errors/InvalidSelector.js new file mode 100644 index 000000000..e900f0c46 --- /dev/null +++ b/lib/helper/errors/InvalidSelector.js @@ -0,0 +1,8 @@ +class InvalidSelector extends Error { + constructor(message) { + super(message) + this.name = 'InvalidSelector' + } +} + +export default InvalidSelector diff --git a/lib/helper/extras/selectorCheck.js b/lib/helper/extras/selectorCheck.js new file mode 100644 index 000000000..41db25846 --- /dev/null +++ b/lib/helper/extras/selectorCheck.js @@ -0,0 +1,46 @@ +import store from '../../store.js' +import InvalidSelector from '../errors/InvalidSelector.js' + +const CSS_CHARS = ['.', '#', '[', ']', '=', ':', '>', '+', '~', '*', ',', '|', '^', '$'] +const XPATH_CHARS = ['/', '(', ')', '@', '"', "'"] +const TAG_NAME = /^[a-z][a-z0-9-]*$/ + +const SUGGESTIONS = [ + [/^waitFor|^waitTo|^waitNumber/, 'Use I.waitForText() to wait for a text on page.'], + [/^see|^dontSee/, 'Use I.see() or I.dontSee() to assert a text on page.'], + [/^grab/, 'Locate the element by CSS or XPath, or use I.grabTextFrom() on its container.'], +] + +export function looksLikeSelector(value) { + if (CSS_CHARS.some(char => value.includes(char))) return true + if (XPATH_CHARS.some(char => value.includes(char))) return true + + const words = value.trim().split(/\s+/) + if (words.length === 1) return true + if (words.every(word => TAG_NAME.test(word))) return true + + return false +} + +export function checkSelectorIsNotText(helper, locator) { + if (!helper.options.strict && !store.debugMode) return + if (typeof locator !== 'string' || !locator.trim()) return + if (looksLikeSelector(locator)) return + + const step = store.currentStep + const method = step?.title || '' + const suggestion = SUGGESTIONS.find(([pattern]) => pattern.test(method))?.[1] || '' + const action = method ? `I.${method}()` : 'This step' + + const message = `"${locator}" doesn't look like a CSS or XPath selector. ${action} expects an element locator, so this text is matched as CSS and finds nothing. ${suggestion}`.trim() + + if (helper.options.strict) throw new InvalidSelector(message) + + if (step) { + const reported = step.__selectorChecks || (step.__selectorChecks = new Set()) + if (reported.has(locator)) return + reported.add(locator) + } + + helper.debugSection('Warning', message) +} diff --git a/test/unit/selectorCheck_test.js b/test/unit/selectorCheck_test.js new file mode 100644 index 000000000..44cdbc8e3 --- /dev/null +++ b/test/unit/selectorCheck_test.js @@ -0,0 +1,120 @@ +import { expect } from 'chai' +import { looksLikeSelector, checkSelectorIsNotText } from '../../lib/helper/extras/selectorCheck.js' +import store from '../../lib/store.js' + +describe('selectorCheck', () => { + describe('looksLikeSelector', () => { + it('rejects a text passed instead of a selector', () => { + expect(looksLikeSelector('Description Persistence Suite OtherYappiestIndigo973')).to.be.false + expect(looksLikeSelector('Log in to your account')).to.be.false + }) + + it('accepts CSS locators', () => { + expect(looksLikeSelector('.monaco-editor')).to.be.true + expect(looksLikeSelector('#save')).to.be.true + expect(looksLikeSelector('[data-test] a')).to.be.true + expect(looksLikeSelector('ul > li')).to.be.true + expect(looksLikeSelector('li:first-child')).to.be.true + }) + + it('accepts XPath locators', () => { + expect(looksLikeSelector('//*[contains(@class,"monaco-editor")][1]')).to.be.true + expect(looksLikeSelector('.//div')).to.be.true + expect(looksLikeSelector('(//a)[1]')).to.be.true + }) + + it('accepts descendant selectors built from tag names', () => { + expect(looksLikeSelector('div span')).to.be.true + expect(looksLikeSelector('my-app my-button')).to.be.true + }) + + it('accepts a single word', () => { + expect(looksLikeSelector('button')).to.be.true + expect(looksLikeSelector('Save')).to.be.true + }) + + it('accepts mobile and engine locators', () => { + expect(looksLikeSelector('~my Button')).to.be.true + expect(looksLikeSelector('android=new UiSelector().text("Save now")')).to.be.true + expect(looksLikeSelector('text=Save Changes')).to.be.true + }) + }) + + describe('checkSelectorIsNotText', () => { + const warnings = [] + const helper = options => ({ + options, + debugSection: (section, msg) => warnings.push(`[${section}] ${msg}`), + }) + + beforeEach(() => { + warnings.length = 0 + store.debugMode = false + store.currentStep = { title: 'waitForElement' } + }) + + afterEach(() => { + store.debugMode = false + store.currentStep = null + }) + + it('stays silent outside of debug and strict mode', () => { + checkSelectorIsNotText(helper({}), 'Description Persistence Suite') + expect(warnings).to.be.empty + }) + + it('stays silent on non-string and empty locators', () => { + store.debugMode = true + checkSelectorIsNotText(helper({ strict: true }), { css: 'a b' }) + checkSelectorIsNotText(helper({ strict: true }), undefined) + checkSelectorIsNotText(helper({ strict: true }), ' ') + expect(warnings).to.be.empty + }) + + it('warns in debug mode', () => { + store.debugMode = true + checkSelectorIsNotText(helper({}), 'Description Persistence Suite') + expect(warnings).to.have.lengthOf(1) + expect(warnings[0]).to.include('[Warning]') + expect(warnings[0]).to.include("doesn't look like a CSS or XPath selector") + expect(warnings[0]).to.include('I.waitForElement()') + expect(warnings[0]).to.include('I.waitForText()') + }) + + it('does not warn on a valid selector in debug mode', () => { + store.debugMode = true + checkSelectorIsNotText(helper({}), '.monaco-editor') + expect(warnings).to.be.empty + }) + + it('throws in strict mode', () => { + const check = () => checkSelectorIsNotText(helper({ strict: true }), 'Description Persistence Suite') + expect(check).to.throw(/doesn't look like a CSS or XPath selector/) + try { + check() + } catch (err) { + expect(err.name).to.equal('InvalidSelector') + } + }) + + it('does not throw on a valid selector in strict mode', () => { + checkSelectorIsNotText(helper({ strict: true }), '//div[@id="save"]') + expect(warnings).to.be.empty + }) + + it('warns once per locator within a step', () => { + store.debugMode = true + const h = helper({}) + checkSelectorIsNotText(h, 'Description Persistence Suite') + checkSelectorIsNotText(h, 'Description Persistence Suite') + expect(warnings).to.have.lengthOf(1) + }) + + it('suggests I.see() for assertion steps', () => { + store.debugMode = true + store.currentStep = { title: 'seeElement' } + checkSelectorIsNotText(helper({}), 'Description Persistence Suite') + expect(warnings[0]).to.include('I.see()') + }) + }) +})