From d036635217324af34076061235489fef2f387bbb Mon Sep 17 00:00:00 2001 From: Edward Pfremmer Date: Wed, 17 Jun 2020 13:28:11 -0500 Subject: [PATCH 1/2] fix: premature resolution of wait conditions when using scopedBy When using a function style selector wait conditions fail early if the scoped selector does not yield any results. Instead we should continue to wait/check until the wait condition times out. Additionally in order to retain custom friendly error messages that include the selector that an exception occurred on we fallback to the original wait behavior which includes the custom errors. --- packages/page-objects/src/base-element.js | 20 + packages/page-objects/src/element.js | 4 + .../test/acceptance/api-verification-test.js | 433 +++++++++++++++--- 3 files changed, 390 insertions(+), 67 deletions(-) diff --git a/packages/page-objects/src/base-element.js b/packages/page-objects/src/base-element.js index a661e1fa..c2e07fec 100644 --- a/packages/page-objects/src/base-element.js +++ b/packages/page-objects/src/base-element.js @@ -41,6 +41,16 @@ class BaseElement extends BasePageObject { async waitForInsert() { await handleError.call(this, async () => { + if (typeof this._selector === 'function') { + try { + return await this._browser.waitUntil(async () => await this.isExisting()); + } catch (e) { + // if the waitUntil condition fails fall through to the original non-scoped + // call so we continue to throw custom human readable faltest errors about the + // element selector that is missing + } + } + await this._browser.waitForInsert(this._selector); }, async elementError => { // If it's an expected error, that means a parent doesn't exist, @@ -56,6 +66,16 @@ class BaseElement extends BasePageObject { async waitForDestroy() { await handleError.call(this, async () => { + if (typeof this._selector === 'function') { + try { + return await this._browser.waitUntil(async () => !(await this.isExisting())); + } catch (e) { + // if the waitUntil condition fails fall through to the original non-scoped + // call so we continue to throw custom human readable faltest errors about the + // element selector that was not destroyed + } + } + await this._browser.waitForDestroy(this._selector, ...arguments); }, () => { // If it's an expected error, that means a parent doesn't exist, diff --git a/packages/page-objects/src/element.js b/packages/page-objects/src/element.js index 7a990eeb..8d73cd8d 100644 --- a/packages/page-objects/src/element.js +++ b/packages/page-objects/src/element.js @@ -12,10 +12,12 @@ class Element extends BaseElement { } async waitForEnabled() { + await this.waitForInsert(); await this._browser.waitForEnabled(this._selector, ...arguments); } async waitForDisabled() { + await this.waitForInsert(); await this._browser.waitForDisabled(this._selector, ...arguments); } @@ -24,10 +26,12 @@ class Element extends BaseElement { } async waitForVisible() { + await this.waitForInsert(); await this._browser.waitForVisible(this._selector, ...arguments); } async waitForHidden() { + await this.waitForInsert(); await this._browser.waitForHidden(this._selector, ...arguments); } diff --git a/packages/page-objects/test/acceptance/api-verification-test.js b/packages/page-objects/test/acceptance/api-verification-test.js index 200bcec8..6cb2df10 100644 --- a/packages/page-objects/test/acceptance/api-verification-test.js +++ b/packages/page-objects/test/acceptance/api-verification-test.js @@ -13,6 +13,9 @@ const tmpDir = promisify(require('tmp').dir); const writeFile = promisify(require('fs').writeFile); const path = require('path'); +const attributeEquals = (attr, value) => + async (pageObject) => (await pageObject.getAttribute(attr)) === value; + describe(function() { setUpWebDriver.call(this, { shareWebdriver: true, @@ -50,6 +53,12 @@ describe(function() { if (this.server) { await this.server.stop(); } + + if (this.browser && this.browser.options) { + // some of the tests need to validate wait conditions and modify the wait + // timeout so this needs to be reset to avoid leaking context between tests + this.browser.options.waitforTimeout = 0; + } }); it(Element.prototype.isEnabled, async function() { @@ -69,40 +78,157 @@ describe(function() { .enabled.to.eventually.be.false; }); - it(Element.prototype.waitForEnabled, async function() { - await this.writeFixture('index.html', ` - - `); + describe(Element.prototype.waitForEnabled, function() { + it('waits for element to become enabled', async function() { + await this.writeFixture('index.html', ` + + `); - this.page = this.createPage(class extends BasePageObject { - get foo() { - return this._create('.foo'); - } + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._create('.foo'); + } + }); + + await this.open('index.html'); + + await expect(this.page.foo.waitForEnabled()) + .to.eventually.be.fulfilled; }); - await this.open('index.html'); + it('waits for scoped element to become enabled', async function() { + await this.writeFixture('index.html', ` + + + `); + + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._createMany('.foo'); + } + }); + + await this.open('index.html'); + + this.page._browser._browser.options.waitforTimeout = 10000; + this.page._browser._browser.execute(`(() => { + setTimeout(function() { + const element = document.getElementById('foo'); + element.disabled = false; + }, 100); + })()`); + + await expect(this.page.foo.scopeBy(attributeEquals('name', 'bar')).waitForEnabled()) + .to.eventually.be.fulfilled; + }); - await expect(this.page.foo.waitForEnabled()) - .to.eventually.be.fulfilled; + it('waits for element to exist and become enabled', async function() { + await this.writeFixture('index.html', ` + + `); + + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._createMany('.foo'); + } + }); + + await this.open('index.html'); + + this.page._browser._browser.options.waitforTimeout = 10000; + this.page._browser._browser.execute(`(() => { + setTimeout(function() { + const element = document.createElement('input'); + + element.name = 'bar'; + element.className = 'foo'; + element.disabled = false; + + document.body.appendChild(element); + }, 100); + })()`); + + await expect(this.page.foo.scopeBy(attributeEquals('name', 'bar')).waitForEnabled()) + .to.eventually.be.fulfilled; + }); }); - it(Element.prototype.waitForDisabled, async function() { - await this.writeFixture('index.html', ` - - `); + describe(Element.prototype.waitForDisabled, function() { + it('waits for element to become disabled', async function() { + await this.writeFixture('index.html', ` + + `); - this.page = this.createPage(class extends BasePageObject { - get foo() { - return this._create('.foo'); - } + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._create('.foo'); + } + }); + + await this.open('index.html'); + + await expect(this.page.foo.waitForDisabled()) + .to.eventually.be.fulfilled; }); - await this.open('index.html'); + it('waits for scoped element to become disabled', async function() { + await this.writeFixture('index.html', ` + + + `); + + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._createMany('.foo'); + } + }); + + await this.open('index.html'); + + this.page._browser._browser.options.waitforTimeout = 10000; + this.page._browser._browser.execute(`(() => { + setTimeout(function() { + const element = document.getElementById('foo'); + element.disabled = true; + }, 100); + })()`); + + await expect(this.page.foo.scopeBy(attributeEquals('name', 'bar')).waitForDisabled()) + .to.eventually.be.fulfilled; + }); - await expect(this.page.foo.waitForDisabled()) - .to.eventually.be.fulfilled; + it('waits for element to exist and become disabled', async function() { + await this.writeFixture('index.html', ` + + `); + + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._createMany('.foo'); + } + }); + + await this.open('index.html'); + + this.page._browser._browser.options.waitforTimeout = 10000; + this.page._browser._browser.execute(`(() => { + setTimeout(function() { + const element = document.createElement('input'); + + element.name = 'bar'; + element.className = 'foo'; + element.disabled = true; + + document.body.appendChild(element); + }, 100); + })()`); + + await expect(this.page.foo.scopeBy(attributeEquals('name', 'bar')).waitForDisabled()) + .to.eventually.be.fulfilled; + }); }); + it(Element.prototype.isDisplayed, async function() { await this.writeFixture('index.html', ` @@ -120,38 +246,154 @@ describe(function() { .displayed.to.eventually.be.false; }); - it(Element.prototype.waitForVisible, async function() { - await this.writeFixture('index.html', ` - - `); + describe(Element.prototype.waitForVisible, function() { + it('waits for element to become visible', async function() { + await this.writeFixture('index.html', ` + + `); - this.page = this.createPage(class extends BasePageObject { - get foo() { - return this._create('.foo'); - } + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._create('.foo'); + } + }); + + await this.open('index.html'); + + await expect(this.page.foo.waitForVisible()) + .to.eventually.be.fulfilled; }); - await this.open('index.html'); + it('waits for scoped element to become visible', async function() { + await this.writeFixture('index.html', ` + + + `); + + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._createMany('.foo'); + } + }); + + await this.open('index.html'); + + this.page._browser._browser.options.waitforTimeout = 10000; + this.page._browser._browser.execute(`(() => { + setTimeout(function() { + const element = document.getElementById('foo'); + element.style = 'display:block'; + }, 100); + })()`); + + await expect(this.page.foo.scopeByText('Bar').waitForVisible()) + .to.eventually.be.fulfilled; + }); - await expect(this.page.foo.waitForVisible()) - .to.eventually.be.fulfilled; + it('waits for element to exist and become visible', async function() { + await this.writeFixture('index.html', ` + + `); + + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._createMany('.foo'); + } + }); + + await this.open('index.html'); + + this.page._browser._browser.options.waitforTimeout = 10000; + this.page._browser._browser.execute(`(() => { + setTimeout(function() { + const element = document.createElement('div'); + + element.innerHTML = 'Bar'; + element.className = 'foo'; + element.style = 'display:block'; + + document.body.appendChild(element); + }, 100); + })()`); + + await expect(this.page.foo.scopeByText('Bar').waitForVisible()) + .to.eventually.be.fulfilled; + }); }); - it(Element.prototype.waitForHidden, async function() { - await this.writeFixture('index.html', ` - - `); + describe(Element.prototype.waitForHidden, function() { + it('waits for element to become hidden', async function() { + await this.writeFixture('index.html', ` + + `); - this.page = this.createPage(class extends BasePageObject { - get foo() { - return this._create('.foo'); - } + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._create('.foo'); + } + }); + + await this.open('index.html'); + + await expect(this.page.foo.waitForHidden()) + .to.eventually.be.fulfilled; }); - await this.open('index.html'); + it('waits for scoped element to become hidden', async function() { + await this.writeFixture('index.html', ` +
Foo
+
Bar
+ `); + + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._createMany('.foo'); + } + }); + + await this.open('index.html'); + + this.page._browser._browser.options.waitforTimeout = 10000; + this.page._browser._browser.execute(`(() => { + setTimeout(function() { + const element = document.getElementById('foo'); + element.style = 'display:none'; + }, 100); + })()`); + + await expect(this.page.foo.scopeByText('Bar').waitForHidden()) + .to.eventually.be.fulfilled; + }); - await expect(this.page.foo.waitForHidden()) - .to.eventually.be.fulfilled; + it('waits for element to exist and become hidden', async function() { + await this.writeFixture('index.html', ` +
Foo
+ `); + + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._createMany('.foo'); + } + }); + + await this.open('index.html'); + + this.page._browser._browser.options.waitforTimeout = 10000; + this.page._browser._browser.execute(`(() => { + setTimeout(function() { + const element = document.createElement('div'); + + element.innerHTML = 'Bar'; + element.className = 'foo'; + element.style = 'display:none'; + + document.body.appendChild(element); + }, 100); + })()`); + + await expect(this.page.foo.scopeByText('Bar').waitForHidden()) + .to.eventually.be.fulfilled; + }); }); it(Element.prototype.isExisting, async function() { @@ -170,37 +412,94 @@ describe(function() { .exist.to.eventually.be.false; }); - it(Element.prototype.waitForInsert, async function() { - await this.writeFixture('index.html', ` - - `); + describe(Element.prototype.waitForInsert, function() { + it('waits for element to be inserted', async function() { + await this.writeFixture('index.html', ` + + `); - this.page = this.createPage(class extends BasePageObject { - get foo() { - return this._create('.foo'); - } - }); + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._create('.foo'); + } + }); - await this.open('index.html'); + await this.open('index.html'); - await expect(this.page.foo.waitForInsert()) - .to.eventually.be.fulfilled; + await expect(this.page.foo.waitForInsert()) + .to.eventually.be.fulfilled; + }); + + it('waits for scoped element to be inserted', async function() { + await this.writeFixture('index.html', ` +
Foo
+ `); + + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._createMany('.foo'); + } + }); + + await this.open('index.html'); + + this.page._browser._browser.options.waitforTimeout = 10000; + this.page._browser._browser.execute(`(() => { + setTimeout(function() { + const element = document.createElement('div'); + element.innerHTML = 'Bar'; + element.className = 'foo'; + document.body.appendChild(element); + }, 100); + })()`); + + await expect(this.page.foo.scopeByText('Bar').waitForInsert()) + .to.eventually.be.fulfilled; + }); }); - it(Element.prototype.waitForDestroy, async function() { - await this.writeFixture('index.html', ` - `); + describe(Element.prototype.waitForDestroy, function() { + it('waits for element to be destroyed', async function() { + await this.writeFixture('index.html', ` + `); - this.page = this.createPage(class extends BasePageObject { - get foo() { - return this._create('.foo'); - } - }); + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._create('.foo'); + } + }); - await this.open('index.html'); + await this.open('index.html'); + + await expect(this.page.foo.waitForDestroy()) + .to.eventually.be.fulfilled; + }); - await expect(this.page.foo.waitForDestroy()) - .to.eventually.be.fulfilled; + it('waits for scoped element to be destroyed', async function() { + await this.writeFixture('index.html', ` +
Foo
+
Bar
+ `); + + this.page = this.createPage(class extends BasePageObject { + get foo() { + return this._createMany('.foo'); + } + }); + + await this.open('index.html'); + + this.page._browser._browser.options.waitforTimeout = 10000; + this.page._browser._browser.execute(`(() => { + setTimeout(function() { + const element = document.getElementById('foo'); + element.parentNode.removeChild(element); + }, 100); + })()`); + + await expect(this.page.foo.scopeByText('Bar').waitForDestroy()) + .to.eventually.be.fulfilled; + }); }); it(Element.prototype.getAttribute, async function() { From d68f9aa0f14d53b005fb197aef50701298d94d32 Mon Sep 17 00:00:00 2001 From: Edward Pfremmer Date: Mon, 22 Jun 2020 13:35:06 -0500 Subject: [PATCH 2/2] fix: remove capability to wait for elements to exist first before waiting on other conditions like enabled/visible --- packages/page-objects/src/element.js | 4 - .../test/acceptance/api-verification-test.js | 120 ------------------ 2 files changed, 124 deletions(-) diff --git a/packages/page-objects/src/element.js b/packages/page-objects/src/element.js index 8d73cd8d..7a990eeb 100644 --- a/packages/page-objects/src/element.js +++ b/packages/page-objects/src/element.js @@ -12,12 +12,10 @@ class Element extends BaseElement { } async waitForEnabled() { - await this.waitForInsert(); await this._browser.waitForEnabled(this._selector, ...arguments); } async waitForDisabled() { - await this.waitForInsert(); await this._browser.waitForDisabled(this._selector, ...arguments); } @@ -26,12 +24,10 @@ class Element extends BaseElement { } async waitForVisible() { - await this.waitForInsert(); await this._browser.waitForVisible(this._selector, ...arguments); } async waitForHidden() { - await this.waitForInsert(); await this._browser.waitForHidden(this._selector, ...arguments); } diff --git a/packages/page-objects/test/acceptance/api-verification-test.js b/packages/page-objects/test/acceptance/api-verification-test.js index 6cb2df10..70050905 100644 --- a/packages/page-objects/test/acceptance/api-verification-test.js +++ b/packages/page-objects/test/acceptance/api-verification-test.js @@ -121,36 +121,6 @@ describe(function() { await expect(this.page.foo.scopeBy(attributeEquals('name', 'bar')).waitForEnabled()) .to.eventually.be.fulfilled; }); - - it('waits for element to exist and become enabled', async function() { - await this.writeFixture('index.html', ` - - `); - - this.page = this.createPage(class extends BasePageObject { - get foo() { - return this._createMany('.foo'); - } - }); - - await this.open('index.html'); - - this.page._browser._browser.options.waitforTimeout = 10000; - this.page._browser._browser.execute(`(() => { - setTimeout(function() { - const element = document.createElement('input'); - - element.name = 'bar'; - element.className = 'foo'; - element.disabled = false; - - document.body.appendChild(element); - }, 100); - })()`); - - await expect(this.page.foo.scopeBy(attributeEquals('name', 'bar')).waitForEnabled()) - .to.eventually.be.fulfilled; - }); }); describe(Element.prototype.waitForDisabled, function() { @@ -196,36 +166,6 @@ describe(function() { await expect(this.page.foo.scopeBy(attributeEquals('name', 'bar')).waitForDisabled()) .to.eventually.be.fulfilled; }); - - it('waits for element to exist and become disabled', async function() { - await this.writeFixture('index.html', ` - - `); - - this.page = this.createPage(class extends BasePageObject { - get foo() { - return this._createMany('.foo'); - } - }); - - await this.open('index.html'); - - this.page._browser._browser.options.waitforTimeout = 10000; - this.page._browser._browser.execute(`(() => { - setTimeout(function() { - const element = document.createElement('input'); - - element.name = 'bar'; - element.className = 'foo'; - element.disabled = true; - - document.body.appendChild(element); - }, 100); - })()`); - - await expect(this.page.foo.scopeBy(attributeEquals('name', 'bar')).waitForDisabled()) - .to.eventually.be.fulfilled; - }); }); @@ -289,36 +229,6 @@ describe(function() { await expect(this.page.foo.scopeByText('Bar').waitForVisible()) .to.eventually.be.fulfilled; }); - - it('waits for element to exist and become visible', async function() { - await this.writeFixture('index.html', ` - - `); - - this.page = this.createPage(class extends BasePageObject { - get foo() { - return this._createMany('.foo'); - } - }); - - await this.open('index.html'); - - this.page._browser._browser.options.waitforTimeout = 10000; - this.page._browser._browser.execute(`(() => { - setTimeout(function() { - const element = document.createElement('div'); - - element.innerHTML = 'Bar'; - element.className = 'foo'; - element.style = 'display:block'; - - document.body.appendChild(element); - }, 100); - })()`); - - await expect(this.page.foo.scopeByText('Bar').waitForVisible()) - .to.eventually.be.fulfilled; - }); }); describe(Element.prototype.waitForHidden, function() { @@ -364,36 +274,6 @@ describe(function() { await expect(this.page.foo.scopeByText('Bar').waitForHidden()) .to.eventually.be.fulfilled; }); - - it('waits for element to exist and become hidden', async function() { - await this.writeFixture('index.html', ` -
Foo
- `); - - this.page = this.createPage(class extends BasePageObject { - get foo() { - return this._createMany('.foo'); - } - }); - - await this.open('index.html'); - - this.page._browser._browser.options.waitforTimeout = 10000; - this.page._browser._browser.execute(`(() => { - setTimeout(function() { - const element = document.createElement('div'); - - element.innerHTML = 'Bar'; - element.className = 'foo'; - element.style = 'display:none'; - - document.body.appendChild(element); - }, 100); - })()`); - - await expect(this.page.foo.scopeByText('Bar').waitForHidden()) - .to.eventually.be.fulfilled; - }); }); it(Element.prototype.isExisting, async function() {