From 05f05f0f3165289df064fd67849fbcd6993af07b Mon Sep 17 00:00:00 2001 From: Simon Coen <32054457+Siolto@users.noreply.github.com> Date: Thu, 8 Sep 2022 16:34:02 +0200 Subject: [PATCH] refactor: better error message output and smaller bugfixes (#338) closes #318, paves the way toward completing #331 * fix: catch getWebElement error * fix: improve fluent async api error messages and behaviour * feat: throw an error if the aggregation could not be found * fix: resolve browserInstance before receiving the aggregation in the browser scope * fix: rephrase error messages * feat: add error logging tests * refactor: remove duplicated comment from error-logging test * fix: make the fluentAsync error consistent over the different node versions Co-authored-by: Volker Buzek --- client-side-js/_getAggregation.js | 1 + client-side-js/injectUI5.js | 4 + .../e2e-test-config/wdio.base.conf.js | 7 +- .../webapp/test/e2e/error-logging.test.js | 194 ++++++++++++++++++ package-lock.json | 3 + src/lib/wdi5-bridge.ts | 39 +++- src/lib/wdi5-control.ts | 145 +++++++------ 7 files changed, 319 insertions(+), 74 deletions(-) create mode 100644 examples/ui5-js-app/webapp/test/e2e/error-logging.test.js diff --git a/client-side-js/_getAggregation.js b/client-side-js/_getAggregation.js index 8c5a3084..751f6edc 100644 --- a/client-side-js/_getAggregation.js +++ b/client-side-js/_getAggregation.js @@ -1,5 +1,6 @@ async function clientSide_getAggregation(webElement, aggregationName, browserInstance) { webElement = await Promise.resolve(webElement) // to plug into fluent async api + browserInstance = await Promise.resolve(browserInstance) return await browserInstance.executeAsync( (webElement, aggregationName, done) => { window.bridge.waitForUI5(window.wdi5.waitForUI5Options).then(() => { diff --git a/client-side-js/injectUI5.js b/client-side-js/injectUI5.js index 25711578..67fe988b 100644 --- a/client-side-js/injectUI5.js +++ b/client-side-js/injectUI5.js @@ -284,10 +284,14 @@ async function clientSide_injectUI5(config, waitForUI5Timeout, browserInstance) /** * creates a array of objects containing their id as a property * @param {[sap.ui.core.Control]} aControls + * @throws {Error} error if the aggregation was not found that has to be catched * @return {Array} Object */ window.wdi5.createControlIdMap = (aControls, controlType = "") => { // the array of UI5 controls need to be mapped (remove circular reference) + if (!aControls) { + throw new Error("Aggregation was not found!") + } return aControls.map((element) => { // just use the absolute ID of the control if (controlType === "sap.m.ComboBox" && element.data("InputWithSuggestionsListItem")) { diff --git a/examples/ui5-js-app/e2e-test-config/wdio.base.conf.js b/examples/ui5-js-app/e2e-test-config/wdio.base.conf.js index 0c064c60..727deeea 100644 --- a/examples/ui5-js-app/e2e-test-config/wdio.base.conf.js +++ b/examples/ui5-js-app/e2e-test-config/wdio.base.conf.js @@ -3,7 +3,8 @@ const { join } = require("path") exports.baseConfig = { wdi5: { screenshotPath: join("webapp", "test", "__screenshots__"), - logLevel: "error" + logLevel: "error", + waitForUI5Timeout: 29000 }, maxInstances: 10, capabilities: [ @@ -25,7 +26,7 @@ exports.baseConfig = { bail: 0, baseUrl: "http://localhost:8888", - waitforTimeout: 10000, + waitforTimeout: 20000, connectionRetryTimeout: process.argv.indexOf("--debug") > -1 ? 1200000 : 120000, connectionRetryCount: 3, @@ -34,7 +35,7 @@ exports.baseConfig = { framework: "mocha", mochaOpts: { ui: "bdd", - timeout: process.argv.indexOf("--debug") > -1 ? 600000 : 60000 + timeout: process.argv.indexOf("--debug") > -1 ? 600000 : 90000 }, reporters: ["spec"] } diff --git a/examples/ui5-js-app/webapp/test/e2e/error-logging.test.js b/examples/ui5-js-app/webapp/test/e2e/error-logging.test.js new file mode 100644 index 00000000..7b85c585 --- /dev/null +++ b/examples/ui5-js-app/webapp/test/e2e/error-logging.test.js @@ -0,0 +1,194 @@ +const { afterEach } = require("mocha") +const sinon = require("sinon") + +const expectDefaultErrorMessages = (wdi5Control) => { + expect(console.red.called).toBeTruthy() + expect( + console.red + .getCall(0) + .calledWith( + "[wdi5]", + `call of _getControl() failed because of: Error: No DOM element found using the control selector ${JSON.stringify( + wdi5Control._controlSelector.selector + )}` + ) + ).toBeTruthy() // dirty but otherwise we have no access to the selector + expect( + console.red.getCall(1).calledWith("[wdi5]", `error retrieving control: ${wdi5Control._wdio_ui5_key}`) + ).toBeTruthy() // dirty but otherwise we have no acces to the key +} + +/** + * test the error logs of the wdi5 logger when controls cannot be found + * for every test we are using a slightly different selector so we don't have + * to use "foceSelect: true" all the time + */ +describe("Error logging", () => { + const sandbox = sinon.createSandbox() + + beforeEach(() => { + sandbox.spy(console, "red") + }) + + afterEach(() => { + sandbox.restore() + }) + + it("should log the correct error messages when control was not found", async () => { + const selectorWithWrongId = { + selector: { + id: "wrongId" + } + } + const wdi5ControlWithWrongId = await browser.asControl(selectorWithWrongId) + + expectDefaultErrorMessages(wdi5ControlWithWrongId) + expect(console.red.callCount).toEqual(2) + }) + + it("should log the correct error messages when 'press' is executed on an not found control; WITHOUT fluent async api", async () => { + const selectorWithWrongId = { + selector: { + id: "wrongIdWithPress" + } + } + const wdi5ControlWithWrongId = await browser.asControl(selectorWithWrongId) + await wdi5ControlWithWrongId.press() + + expectDefaultErrorMessages(wdi5ControlWithWrongId) + expect(console.red.callCount).toEqual(3) + expect( + console.red.getCall(2).calledWith("[wdi5]", `cannot call press(), because control could not be found`) + ).toBeTruthy() + }) + + it("should log the correct error messages when 'press' is executed on an not found control; WITH fluent async api", async () => { + const selectorWithWrongId = { + selector: { + id: "wrongIdWithPressFluentAsync" + } + } + + const wdi5ControlWithWrongId = await browser.asControl(selectorWithWrongId).press() + + expectDefaultErrorMessages(wdi5ControlWithWrongId) + expect(console.red.callCount).toEqual(3) + expect( + console.red.getCall(2).calledWith("[wdi5]", `cannot call press(), because control could not be found`) + ).toBeTruthy() + }) + + it("should log the correct error messages when 'getAggregation' is executed on an not found control; WITHOUT fluent async api", async () => { + const selectorWithWrongId = { + selector: { + id: "wrongIdWithGetAggregation" + } + } + + const wdi5ControlWithWrongId = await browser.asControl(selectorWithWrongId) + await wdi5ControlWithWrongId.getAggregation("items") + + expectDefaultErrorMessages(wdi5ControlWithWrongId) + + expect(console.red.callCount).toEqual(3) + expect( + console.red + .getCall(2) + .calledWith("[wdi5]", `cannot get aggregation "items", because control could not be found`) + ).toBeTruthy() + }) + it("should log the correct error messages when 'getAggregation' is executed on an not found control; WITH fluent async api", async () => { + const selectorWithWrongId = { + selector: { + id: "wrongIdWithGetAggregationFluentAsync" + } + } + await browser.asControl(selectorWithWrongId).getAggregation("items") + // we need the wdi5 control for the assertions. As we are not using forceSelect + // there should be no additional error messages + const wdi5ControlWithWrongId = await browser.asControl(selectorWithWrongId) + + expectDefaultErrorMessages(wdi5ControlWithWrongId) + expect(console.red.callCount).toEqual(3) + expect( + console.red + .getCall(2) + .calledWith("[wdi5]", `cannot get aggregation "items", because control could not be found`) + ).toBeTruthy() + }) + + it("should log the correct error messages when 'getAggregation' is executed on an control with a wrong aggregation; WITH fluent async api", async () => { + const selectorWithWrongId = { + selector: { + id: "container-Sample---Main--NavFwdButton" + } + } + + await browser.asControl(selectorWithWrongId).getAggregation("tooltip") + + expect(console.red.callCount).toEqual(1) + expect( + console.red + .getCall(0) + .calledWith("[wdi5]", `call of _getAggregation() failed because of: Error: Aggregation was not found!`) + ).toBeTruthy() + }) + + it("should log the correct error messages when 'enterText' is executed on an not found control; WITHOUT fluent async api", async () => { + const selectorWithWrongId = { + selector: { + id: "wrongIdWithEnterText" + } + } + + const wdi5ControlWithWrongId = await browser.asControl(selectorWithWrongId) + await wdi5ControlWithWrongId.enterText("test") + + expectDefaultErrorMessages(wdi5ControlWithWrongId) + + expect(console.red.callCount).toEqual(3) + expect( + console.red.getCall(2).calledWith("[wdi5]", `cannot call enterText(), because control could not be found`) + ).toBeTruthy() + }) + // TODO: introudce fluent async api for enterText + // it("should log the correct error messages when 'enterText' is executed on an not found control WITH fluent async api", async () => { + // const selectorWithWrongId = { + // selector: { + // id: "wrongIdWithEnterTextFluentAsync" + // } + // } + + // const wdi5ControlWithWrongId = await (await browser.asControl(selectorWithWrongId)).enterText() + + // expectDefaultErrorMessages(wdi5ControlWithWrongId) + + // expect(console.red.callCount).toEqual(3) + // expect(console.red.getCall(2).calledWith("[wdi5]", `cannot call enterText(), because control could not be found`)).toBeTruthy() + // }); + + it("should log the correct error messages when multiple functions are executed on an control where an error in the queue occurrs; WITH fluent async api", async () => { + const selectorWithWrongId = { + selector: { + id: "container-Sample---Main--NavFwdButton" + } + } + + await browser.asControl(selectorWithWrongId).getWrongFunction().getSecondWrongFunction() + + expect(console.red.callCount).toEqual(2) + expect( + console.red + .getCall(0) + .calledWith( + "[wdi5]", + `One of the calls in the queue "getWrongFunction().getSecondWrongFunction()" previously failed!` + ) + ).toBeTruthy() + expect( + console.red + .getCall(1) + .calledWith("[wdi5]", `Cannot read property 'getSecondWrongFunction' in the execution queue!`) + ).toBeTruthy() + }) +}) diff --git a/package-lock.json b/package-lock.json index a8e2eb11..38dc8d56 100644 --- a/package-lock.json +++ b/package-lock.json @@ -58,6 +58,7 @@ } }, "examples/fe-app": { + "name": "fiori_elements_tutorial", "version": "1.0.0", "hasInstallScript": true, "license": "UNLICENSED", @@ -74,6 +75,7 @@ } }, "examples/ui5-js-app": { + "name": "ui5-app", "version": "0.8.15-notimportant", "license": "UNLICENSED", "devDependencies": { @@ -90,6 +92,7 @@ } }, "examples/ui5-ts-app": { + "name": "ui5-t-sapp", "version": "0.8.15-notimportant", "license": "Apache-2.0", "devDependencies": { diff --git a/src/lib/wdi5-bridge.ts b/src/lib/wdi5-bridge.ts index ff153571..3e0fc6db 100644 --- a/src/lib/wdi5-bridge.ts +++ b/src/lib/wdi5-bridge.ts @@ -355,20 +355,37 @@ export async function _addWdi5Commands(browserInstance: WebdriverIO.Browser) { functionQueue.push(prop) return asyncMethods.includes(prop) ? (...boundArgs) => makeFluent(promise[prop](...boundArgs)) - : makeFluent(promise.then((object) => object[prop])) + : makeFluent( + promise.then((object) => { + // when object is undefined the previous function call failed + try { + return object[prop] + } catch (error) { + // different node versions return a different `error.message` so we use our own message + Logger.error(`Cannot read property '${prop}' in the execution queue!`) + } + }) + ) }, apply(_, thisArg, boundArgs) { return makeFluent( - // When "targetFunction" is empty we can assume that the ui5 control was not found - promise.then((targetFunction) => - targetFunction - ? Reflect.apply(targetFunction, thisArg, boundArgs) - : Logger.error( - `Can not call "${functionQueue.filter( - (name) => !asyncMethods.includes(name) - )}", because control could not be found` - ) - ) + // When "targetFunction" is empty we can assume that there are errors in the execution queue + promise.then((targetFunction) => { + if (targetFunction) { + return Reflect.apply(targetFunction, thisArg, boundArgs) + } else { + // a functionQueue without a 'then' can be ignored + // as the original error was already logged + if (functionQueue.includes("then")) { + functionQueue.splice(functionQueue.indexOf("then")) + Logger.error( + `One of the calls in the queue "${functionQueue.join( + "()." + )}()" previously failed!` + ) + } + } + }) ) } } diff --git a/src/lib/wdi5-control.ts b/src/lib/wdi5-control.ts index dcc3018f..872dda16 100644 --- a/src/lib/wdi5-control.ts +++ b/src/lib/wdi5-control.ts @@ -121,23 +121,15 @@ export class WDI5Control { } /** + * tries to retrieve the webdriver representation of the current wdi5 control * @return {WebdriverIO.Element} the webdriver Element */ async getWebElement() { - if (util.types.isProxy(this._domId)) { - const id = await Promise.resolve(this._domId) - if (id) { - const webElement = await $(`//*[@id="${id}"]`) - return webElement - } else { - throw Error("control could not be found") - } - } - if (!this._webdriverRepresentation) { - // to enable transition from wdi5 to wdio api in allControls - await this.renewWebElement() + try { + return await this._getWebElement() + } catch (error) { + Logger.error(`cannot call "getWebElement()", because ${error.message}`) } - return this._webdriverRepresentation } /** @@ -148,37 +140,17 @@ export class WDI5Control { return this._wdioBridge // this.getWebElement() } - /** - * @param id - * @returns - */ - async renewWebElement(id: string = this._domId) { - if (this._domId) { - this._webdriverRepresentation = await this._browserInstance.$(`//*[@id="${id}"]`) - return this._webdriverRepresentation - } else { - throw Error("control could not be found") - } - } - /** * bridge to UI5 control api "getAggregation" * @param name name of the aggregation * @return array of UI5 controls representing the aggregation */ async getAggregation(name: string) { - const _forceSelect: boolean = util.types.isProxy(this._forceSelect) - ? await Promise.resolve(this._forceSelect) - : this._forceSelect - - if (_forceSelect) { - try { - await this._renewWebElementReference() - } catch (error) { - Logger.error(`Can not get aggregation "${name}", because ${error.message}`) - } + try { + return await this._getAggregation(name) + } catch (error) { + Logger.error(`cannot get aggregation "${name}", because ${error.message}`) } - return await this._getAggregation(name) } /** @@ -186,17 +158,17 @@ export class WDI5Control { * @param text */ async enterText(text: string) { - // if (this._forceSelect) { - // this._renewWebElementReference(); - // } - const oOptions = { enterText: text, selector: this._controlSelector.selector, clearTextFirst: true, interactionType: "ENTER_TEXT" } - await this.interactWithControl(oOptions) + try { + await this._interactWithControl(oOptions) + } catch (error) { + Logger.error(`cannot call enterText(), because ${error.message}`) + } return this } @@ -206,28 +178,13 @@ export class WDI5Control { */ async press() { try { - await ((await this.getWebElement()) as unknown as WebdriverIO.Element).click() + await ((await this._getWebElement()) as unknown as WebdriverIO.Element).click() } catch (error) { - Logger.error(`Can not call press(), because ${error.message}`) + Logger.error(`cannot call press(), because ${error.message}`) } return this } - /** - * Interact with specific control. - * @param {object} oOptions - * @param {sap.ui.test.RecordReplay.ControlSelector} oOptions.selector - UI5 type - * @param {sap.ui.test.RecordReplay.InteractionType} oOptions.interactionType - UI5 type - * @param {string} oOptions.enterText - * @param {boolean} oOptions.clearTextFirst - */ - async interactWithControl(oOptions) { - const result = (await clientSide_interactWithControl(oOptions, this._browserInstance)) as clientSide_ui5Response - - this._writeObjectResultLog(result, "interactWithControl()") - return result.result - } - /** * fire a named event on a UI5 control * @param {String} eventName @@ -261,6 +218,63 @@ export class WDI5Control { // --- private methods --- + /** + * Interact with specific control. + * @param {object} oOptions + * @param {sap.ui.test.RecordReplay.ControlSelector} oOptions.selector - UI5 type + * @param {sap.ui.test.RecordReplay.InteractionType} oOptions.interactionType - UI5 type + * @param {string} oOptions.enterText + * @param {boolean} oOptions.clearTextFirst + */ + private async _interactWithControl(oOptions) { + if (this._domId) { + const result = (await clientSide_interactWithControl( + oOptions, + this._browserInstance + )) as clientSide_ui5Response + + this._writeObjectResultLog(result, "interactWithControl()") + return result.result + } else { + throw Error("control could not be found") + } + } + + /** + * returns the wdio web element. + * @throws will throw an error when no DOM Element was found + * @return {WebdriverIO.Element} the webdriver Element + */ + private async _getWebElement() { + if (util.types.isProxy(this._domId)) { + const id = await Promise.resolve(this._domId) + if (id) { + const webElement = await $(`//*[@id="${id}"]`) + return webElement + } else { + throw Error("control could not be found") + } + } + if (!this._webdriverRepresentation) { + // to enable transition from wdi5 to wdio api in allControls + await this._renewWebElement() + } + return this._webdriverRepresentation + } + + /** + * @param id + * @returns + */ + private async _renewWebElement(id: string = this._domId) { + if (this._domId) { + this._webdriverRepresentation = await this._browserInstance.$(`//*[@id="${id}"]`) + return this._webdriverRepresentation + } else { + throw Error("control could not be found") + } + } + /** * retrieve UI5 control represenation of a UI5 control's aggregation * @@ -367,7 +381,7 @@ export class WDI5Control { try { this._webElement = await this._renewWebElementReference() } catch (error) { - Logger.error(`Can not execute ${methodName}(), because ${error.message}`) + Logger.error(`cannot execute ${methodName}(), because ${error.message}`) } } // special case for custom data attached to a UI5 control: @@ -436,15 +450,26 @@ export class WDI5Control { * * @param aggregationName * @param webElement + * @throws will throw an error when no webElement was found * @return {any} */ private async _getAggregation( aggregationName: string, webElement: WebdriverIO.Element | string = this._webElement ) { + const _forceSelect: boolean = util.types.isProxy(this._forceSelect) + ? await Promise.resolve(this._forceSelect) + : this._forceSelect + + if (_forceSelect) { + await this._renewWebElementReference() + } if (util.types.isProxy(webElement)) { webElement = await Promise.resolve(webElement) } + if (!webElement) { + throw Error("control could not be found") + } const result = (await clientSide_getAggregation( webElement, aggregationName,