From a9459476c0303e5add8b552461da714daab93f29 Mon Sep 17 00:00:00 2001 From: Jonathan Lipps Date: Wed, 26 Sep 2018 22:22:00 -0700 Subject: [PATCH 1/4] add custom locator strategy --- lib/basedriver/commands/find.js | 76 ++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/lib/basedriver/commands/find.js b/lib/basedriver/commands/find.js index 4f509f7cf..56a2f1843 100644 --- a/lib/basedriver/commands/find.js +++ b/lib/basedriver/commands/find.js @@ -1,14 +1,16 @@ import log from '../logger'; +import { logger, imageUtil } from 'appium-support'; +import _ from 'lodash'; import { errors } from '../../..'; import { MATCH_TEMPLATE_MODE } from './images'; import { W3C_ELEMENT_KEY, MJSONWP_ELEMENT_KEY } from '../../protocol/protocol'; import { ImageElement } from '../image-element'; -import { imageUtil } from 'appium-support'; const commands = {}, helpers = {}, extensions = {}; const IMAGE_STRATEGY = "-image"; +const CUSTOM_STRATEGY = "-custom"; // Override the following function for your own driver, and the rest is taken // care of! @@ -41,6 +43,8 @@ helpers.findElOrElsWithProcessing = async function (strategy, selector, mult, co commands.findElement = async function (strategy, selector) { if (strategy === IMAGE_STRATEGY) { return await this.findByImage(selector, {multiple: false}); + } else if (strategy === CUSTOM_STRATEGY) { + return await this.findByCustom(selector, false); } return await this.findElOrElsWithProcessing(strategy, selector, false); @@ -49,6 +53,8 @@ commands.findElement = async function (strategy, selector) { commands.findElements = async function (strategy, selector) { if (strategy === IMAGE_STRATEGY) { return await this.findByImage(selector, {multiple: true}); + } else if (strategy === CUSTOM_STRATEGY) { + return await this.findByCustom(selector, true); } return await this.findElOrElsWithProcessing(strategy, selector, true); @@ -62,6 +68,74 @@ commands.findElementsFromElement = async function (strategy, selector, elementId return await this.findElOrElsWithProcessing(strategy, selector, true, elementId); }; +/** + * Find an element using a custom plugin specified by the customFindModule cap. + * + * @param {string} selector - the selector which the plugin will use to find + * elements + * @param {boolean} multiple - whether we want one element or multiple + * + * @returns {WebElement} - WebDriver element or list of elements + */ +commands.findByCustom = async function (selector, multiple) { + if (!this.opts.customFindModule) { + throw new Error("Finding an element using a pluagin is currently an " + + "incubating feature. To use it you must manually install a " + + "plugin module inside your Appium node_modules tree, " + + "and set the capability 'customFindModule' to the name of " + + "that module so that it can be required."); + } + + let finder; + try { + finder = require(this.opts.customFindModule); + } catch (err) { + throw new Error(`Could not load your custom find module. Original error: ${err}`); + } + + if (!_.isFunction(finder.find)) { + throw new Error("Your custom find module did not appear to be constructed " + + "correctly. It needs to export an object with a `find` method."); + } + + const customFinderLog = logger.getLogger(this.opts.customFindModule); + + let els; + const condition = async () => { + // get a list of matched elements from the custom finder, which can + // potentially use the entire suite of methods the current driver provides. + // the finder should always return a list of elements, but may use the + // knowledge of whether we are looking for one or many to perform internal + // optimizations + els = await finder.find(this, customFinderLog, selector, multiple); + + // if we're looking for multiple elements, or if we're looking for only + // one and found it, we're done + if (els.length || multiple) { + return true; + } + + // otherwise we should retry, so return false to trigger the retry loop + return false; + }; + + try { + // make sure we respect implicit wait + await this.implicitWaitForCondition(condition); + } catch (err) { + if (err.message.match(/Condition unmet/)) { + throw new errors.NoSuchElementError(); + } + throw err; + } + + if (multiple) { + return els; + } + + return els[0]; +}; + /** * @typedef {Object} FindByImageOptions * @property {boolean} [shouldCheckStaleness=false] - whether this call to find an From ef6230876e9af61ddbd66e2be406e18c1189c606 Mon Sep 17 00:00:00 2001 From: Jonathan Lipps Date: Tue, 2 Oct 2018 15:04:33 -0700 Subject: [PATCH 2/4] address review comments --- lib/basedriver/commands/find.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/basedriver/commands/find.js b/lib/basedriver/commands/find.js index 56a2f1843..ff7c59b3f 100644 --- a/lib/basedriver/commands/find.js +++ b/lib/basedriver/commands/find.js @@ -79,7 +79,7 @@ commands.findElementsFromElement = async function (strategy, selector, elementId */ commands.findByCustom = async function (selector, multiple) { if (!this.opts.customFindModule) { - throw new Error("Finding an element using a pluagin is currently an " + + throw new Error("Finding an element using a plugin is currently an " + "incubating feature. To use it you must manually install a " + "plugin module inside your Appium node_modules tree, " + "and set the capability 'customFindModule' to the name of " + @@ -90,28 +90,29 @@ commands.findByCustom = async function (selector, multiple) { try { finder = require(this.opts.customFindModule); } catch (err) { - throw new Error(`Could not load your custom find module. Original error: ${err}`); + throw new Error(`Could not load your custom find module ` + + `'${this.opts.customFindModule}'. Original error: ${err}`); } - if (!_.isFunction(finder.find)) { + if (!finder || !_.isFunction(finder.find)) { throw new Error("Your custom find module did not appear to be constructed " + "correctly. It needs to export an object with a `find` method."); } const customFinderLog = logger.getLogger(this.opts.customFindModule); - let els; + let elements; const condition = async () => { // get a list of matched elements from the custom finder, which can // potentially use the entire suite of methods the current driver provides. // the finder should always return a list of elements, but may use the // knowledge of whether we are looking for one or many to perform internal // optimizations - els = await finder.find(this, customFinderLog, selector, multiple); + elements = await finder.find(this, customFinderLog, selector, multiple); // if we're looking for multiple elements, or if we're looking for only // one and found it, we're done - if (els.length || multiple) { + if (!_.isEmpty(elements) || multiple) { return true; } @@ -129,11 +130,7 @@ commands.findByCustom = async function (selector, multiple) { throw err; } - if (multiple) { - return els; - } - - return els[0]; + return multiple ? elements : elements[0]; }; /** From aa67a28f6a731930783c62a14a5620c8a2e0a5b3 Mon Sep 17 00:00:00 2001 From: Jonathan Lipps Date: Thu, 11 Oct 2018 12:42:23 -0700 Subject: [PATCH 3/4] allow customFindModules cap to register multiple modules as an object --- lib/basedriver/commands/find.js | 68 +++++++++++++++++++++++++++------ 1 file changed, 56 insertions(+), 12 deletions(-) diff --git a/lib/basedriver/commands/find.js b/lib/basedriver/commands/find.js index ff7c59b3f..cc83d812d 100644 --- a/lib/basedriver/commands/find.js +++ b/lib/basedriver/commands/find.js @@ -69,7 +69,7 @@ commands.findElementsFromElement = async function (strategy, selector, elementId }; /** - * Find an element using a custom plugin specified by the customFindModule cap. + * Find an element using a custom plugin specified by the customFindModules cap. * * @param {string} selector - the selector which the plugin will use to find * elements @@ -78,28 +78,72 @@ commands.findElementsFromElement = async function (strategy, selector, elementId * @returns {WebElement} - WebDriver element or list of elements */ commands.findByCustom = async function (selector, multiple) { - if (!this.opts.customFindModule) { + const plugins = this.opts.customFindModules; + + // first ensure the user has registered one or more find plugins + if (!plugins) { + // TODO this info should go in docs instead; update when docs for this + // feature exist throw new Error("Finding an element using a plugin is currently an " + - "incubating feature. To use it you must manually install a " + - "plugin module inside your Appium node_modules tree, " + - "and set the capability 'customFindModule' to the name of " + - "that module so that it can be required."); + "incubating feature. To use it you must manually install one or more " + + "plugin modules in a way that they can be required by Appium, for " + + "example installing them from the Appium directory, installing them " + + "globally, or installing them elsewhere and passing an absolute path as " + + "the capability. Then construct an object where the key is the shortcut " + + "name for this plugin and the value is the module name or absolute path, " + + "for example: {\"p1\": \"my-find-plugin\"}, and pass this in as the " + + "'customFindModules' capability."); + } + + // then do some basic checking of the type of the capability + if (!_.isPlainObject(plugins)) { + throw new Error("Invalid format for the 'customFindModules' capability. " + + "It should be an object with keys corresponding to the short names and " + + "values corresponding to the full names of the element finding plugins"); + } + + // get the name of the particular plugin used for this invocation of find, + // and separate it from the selector we will pass to the plugin + let [plugin, realSelector] = selector.split(":"); + + // if the user didn't specify a plugin for this find invocation, and we had + // multiple plugins registered, that's a problem + if (_.size(plugins) > 1 && !realSelector) { + throw new Error(`Multiple element finding plugins were registered ` + + `(${_.keys(plugins)}), but your selector did not indicate which plugin ` + + `to use. Ensure you put the short name of the plugin followed by ':' as ` + + `the initial part of the selector string.`); + } + + // but if they did not specify a plugin and we only have one plugin, just use + // that one + if (_.size(plugins) === 1 && !realSelector) { + realSelector = plugin; + plugin = _.keys(plugins)[0]; + } + + if (!plugins[plugin]) { + throw new Error(`Selector specified use of element finding plugin ` + + `'${plugin}' but it was not registered in the 'customFindModules' ` + + `capability.`); } let finder; try { - finder = require(this.opts.customFindModule); + log.debug(`Find plugin '${plugin}' requested; will attempt to use it ` + + `from '${plugins[plugin]}'`); + finder = require(plugins[plugin]); } catch (err) { - throw new Error(`Could not load your custom find module ` + - `'${this.opts.customFindModule}'. Original error: ${err}`); + throw new Error(`Could not load your custom find module '${plugin}'. Did ` + + `you put it somewhere Appium can 'require' it? Original error: ${err}`); } if (!finder || !_.isFunction(finder.find)) { throw new Error("Your custom find module did not appear to be constructed " + - "correctly. It needs to export an object with a `find` method."); + "correctly. It needs to export an object with a `find` method."); } - const customFinderLog = logger.getLogger(this.opts.customFindModule); + const customFinderLog = logger.getLogger(plugin); let elements; const condition = async () => { @@ -108,7 +152,7 @@ commands.findByCustom = async function (selector, multiple) { // the finder should always return a list of elements, but may use the // knowledge of whether we are looking for one or many to perform internal // optimizations - elements = await finder.find(this, customFinderLog, selector, multiple); + elements = await finder.find(this, customFinderLog, realSelector, multiple); // if we're looking for multiple elements, or if we're looking for only // one and found it, we're done From 515894b8ccc3c58ea47573104aeff6b9af11a1b3 Mon Sep 17 00:00:00 2001 From: Jonathan Lipps Date: Thu, 11 Oct 2018 15:05:47 -0700 Subject: [PATCH 4/4] add unit tests for custom element finding plugin --- lib/basedriver/commands/find.js | 2 +- test/basedriver/commands/find-specs.js | 79 ++++++++++++++++++- .../fixtures/custom-element-finder-bad.js | 5 ++ .../fixtures/custom-element-finder.js | 29 +++++++ 4 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 test/basedriver/fixtures/custom-element-finder-bad.js create mode 100644 test/basedriver/fixtures/custom-element-finder.js diff --git a/lib/basedriver/commands/find.js b/lib/basedriver/commands/find.js index cc83d812d..8c9b8d378 100644 --- a/lib/basedriver/commands/find.js +++ b/lib/basedriver/commands/find.js @@ -371,5 +371,5 @@ helpers.getScreenshotForImageFind = async function (screenWidth, screenHeight) { Object.assign(extensions, commands, helpers); -export { commands, helpers, IMAGE_STRATEGY }; +export { commands, helpers, IMAGE_STRATEGY, CUSTOM_STRATEGY }; export default extensions; diff --git a/test/basedriver/commands/find-specs.js b/test/basedriver/commands/find-specs.js index 512dcf587..048c3cb5e 100644 --- a/test/basedriver/commands/find-specs.js +++ b/test/basedriver/commands/find-specs.js @@ -1,8 +1,9 @@ import chai from 'chai'; +import path from 'path'; import chaiAsPromised from 'chai-as-promised'; import sinon from 'sinon'; import { BaseDriver, ImageElement } from '../../..'; -import { IMAGE_STRATEGY } from '../../../lib/basedriver/commands/find'; +import { IMAGE_STRATEGY, CUSTOM_STRATEGY } from '../../../lib/basedriver/commands/find'; import { imageUtil } from 'appium-support'; @@ -14,6 +15,11 @@ class TestDriver extends BaseDriver { async getScreenshot () {} } +const CUSTOM_FIND_MODULE = path.resolve(__dirname, "..", "..", "..", "..", + "test", "basedriver", "fixtures", "custom-element-finder"); +const BAD_CUSTOM_FIND_MODULE = path.resolve(__dirname, "..", "..", "..", "..", + "test", "basedriver", "fixtures", "custom-element-finder-bad"); + const TINY_PNG = "iVBORw0KGgoAAAANSUhEUgAAAAQAAAAECAIAAAAmkwkpAAAAGXRFWHRTb2Z0d2FyZQBBZG9iZSBJbWFnZVJlYWR5ccllPAAAAyhpVFh0WE1MOmNvbS5hZG9iZS54bXAAAAAAADw/eHBhY2tldCBiZWdpbj0i77u/IiBpZD0iVzVNME1wQ2VoaUh6cmVTek5UY3prYzlkIj8+IDx4OnhtcG1ldGEgeG1sbnM6eD0iYWRvYmU6bnM6bWV0YS8iIHg6eG1wdGs9IkFkb2JlIFhNUCBDb3JlIDUuNi1jMTQwIDc5LjE2MDQ1MSwgMjAxNy8wNS8wNi0wMTowODoyMSAgICAgICAgIj4gPHJkZjpSREYgeG1sbnM6cmRmPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5LzAyLzIyLXJkZi1zeW50YXgtbnMjIj4gPHJkZjpEZXNjcmlwdGlvbiByZGY6YWJvdXQ9IiIgeG1sbnM6eG1wPSJodHRwOi8vbnMuYWRvYmUuY29tL3hhcC8xLjAvIiB4bWxuczp4bXBNTT0iaHR0cDovL25zLmFkb2JlLmNvbS94YXAvMS4wL21tLyIgeG1sbnM6c3RSZWY9Imh0dHA6Ly9ucy5hZG9iZS5jb20veGFwLzEuMC9zVHlwZS9SZXNvdXJjZVJlZiMiIHhtcDpDcmVhdG9yVG9vbD0iQWRvYmUgUGhvdG9zaG9wIENDIDIwMTggKE1hY2ludG9zaCkiIHhtcE1NOkluc3RhbmNlSUQ9InhtcC5paWQ6N0NDMDM4MDM4N0U2MTFFOEEzMzhGMTRFNUUwNzIwNUIiIHhtcE1NOkRvY3VtZW50SUQ9InhtcC5kaWQ6N0NDMDM4MDQ4N0U2MTFFOEEzMzhGMTRFNUUwNzIwNUIiPiA8eG1wTU06RGVyaXZlZEZyb20gc3RSZWY6aW5zdGFuY2VJRD0ieG1wLmlpZDo3Q0MwMzgwMTg3RTYxMUU4QTMzOEYxNEU1RTA3MjA1QiIgc3RSZWY6ZG9jdW1lbnRJRD0ieG1wLmRpZDo3Q0MwMzgwMjg3RTYxMUU4QTMzOEYxNEU1RTA3MjA1QiIvPiA8L3JkZjpEZXNjcmlwdGlvbj4gPC9yZGY6UkRGPiA8L3g6eG1wbWV0YT4gPD94cGFja2V0IGVuZD0iciI/PpdvJjQAAAAlSURBVHjaJInBEQAACIKw/Xe2Ul5wYBtwmJqkk4+zfvUQVoABAEg0EfrZwc0hAAAAAElFTkSuQmCC"; const TINY_PNG_DIMS = [4, 4]; @@ -191,3 +197,74 @@ describe('finding elements by image', function () { }); }); }); + +describe('custom element finding plugins', function () { + // happys + it('should find a single element using a custom finder', async function () { + const d = new BaseDriver(); + d.opts.customFindModules = {f: CUSTOM_FIND_MODULE}; + await d.findElement(CUSTOM_STRATEGY, "f:foo").should.eventually.eql("bar"); + }); + it('should not require selector prefix if only one find plugin is registered', async function () { + const d = new BaseDriver(); + d.opts.customFindModules = {f: CUSTOM_FIND_MODULE}; + await d.findElement(CUSTOM_STRATEGY, "foo").should.eventually.eql("bar"); + }); + it('should find multiple elements using a custom finder', async function () { + const d = new BaseDriver(); + d.opts.customFindModules = {f: CUSTOM_FIND_MODULE}; + await d.findElements(CUSTOM_STRATEGY, "f:foos").should.eventually.eql(["baz1", "baz2"]); + }); + it('should give a hint to the plugin about whether multiple are requested', async function () { + const d = new BaseDriver(); + d.opts.customFindModules = {f: CUSTOM_FIND_MODULE}; + await d.findElement(CUSTOM_STRATEGY, "f:foos").should.eventually.eql("bar1"); + }); + it('should be able to use multiple find modules', async function () { + const d = new BaseDriver(); + d.opts.customFindModules = {f: CUSTOM_FIND_MODULE, g: CUSTOM_FIND_MODULE}; + await d.findElement(CUSTOM_STRATEGY, "f:foo").should.eventually.eql("bar"); + await d.findElement(CUSTOM_STRATEGY, "g:foo").should.eventually.eql("bar"); + }); + + // errors + it('should throw an error if customFindModules is not set', async function () { + const d = new BaseDriver(); + await d.findElement(CUSTOM_STRATEGY, "f:foo").should.eventually.be.rejectedWith(/customFindModules/); + }); + it('should throw an error if customFindModules is the wrong shape', async function () { + const d = new BaseDriver(); + d.opts.customFindModules = CUSTOM_FIND_MODULE; + await d.findElement(CUSTOM_STRATEGY, "f:foo").should.eventually.be.rejectedWith(/customFindModules/); + }); + it('should throw an error if customFindModules is size > 1 and no selector prefix is used', async function () { + const d = new BaseDriver(); + d.opts.customFindModules = {f: CUSTOM_FIND_MODULE, g: CUSTOM_FIND_MODULE}; + await d.findElement(CUSTOM_STRATEGY, "foo").should.eventually.be.rejectedWith(/multiple element finding/i); + }); + it('should throw an error in attempt to use unregistered plugin', async function () { + const d = new BaseDriver(); + d.opts.customFindModules = {f: CUSTOM_FIND_MODULE, g: CUSTOM_FIND_MODULE}; + await d.findElement(CUSTOM_STRATEGY, "z:foo").should.eventually.be.rejectedWith(/was not registered/); + }); + it('should throw an error if plugin cannot be loaded', async function () { + const d = new BaseDriver(); + d.opts.customFindModules = {f: "./foo.js"}; + await d.findElement(CUSTOM_STRATEGY, "f:foo").should.eventually.be.rejectedWith(/could not load/i); + }); + it('should throw an error if plugin is not the right shape', async function () { + const d = new BaseDriver(); + d.opts.customFindModules = {f: BAD_CUSTOM_FIND_MODULE}; + await d.findElement(CUSTOM_STRATEGY, "f:foo").should.eventually.be.rejectedWith(/constructed correctly/i); + }); + it('should pass on an error thrown by the finder itself', async function () { + const d = new BaseDriver(); + d.opts.customFindModules = {f: CUSTOM_FIND_MODULE}; + await d.findElement(CUSTOM_STRATEGY, "f:error").should.eventually.be.rejectedWith(/plugin error/i); + }); + it('should throw no such element error if element not found', async function () { + const d = new BaseDriver(); + d.opts.customFindModules = {f: CUSTOM_FIND_MODULE}; + await d.findElement(CUSTOM_STRATEGY, "f:nope").should.eventually.be.rejectedWith(/could not be located/); + }); +}); diff --git a/test/basedriver/fixtures/custom-element-finder-bad.js b/test/basedriver/fixtures/custom-element-finder-bad.js new file mode 100644 index 000000000..2d69ff12e --- /dev/null +++ b/test/basedriver/fixtures/custom-element-finder-bad.js @@ -0,0 +1,5 @@ +module.exports = { + notFind: function () { // eslint-disable-line object-shorthand + return []; + } +}; diff --git a/test/basedriver/fixtures/custom-element-finder.js b/test/basedriver/fixtures/custom-element-finder.js new file mode 100644 index 000000000..0e86b7a61 --- /dev/null +++ b/test/basedriver/fixtures/custom-element-finder.js @@ -0,0 +1,29 @@ +module.exports = { + find: function (driver, logger, selector, multiple) { // eslint-disable-line object-shorthand + if (!driver || !driver.opts) { + throw new Error("Expected driver object"); + } + + if (!logger || !logger.info) { + throw new Error("Expected logger object"); + } + + if (selector === "foo") { + return ["bar"]; + } + + if (selector === "foos") { + if (multiple) { + return ["baz1", "baz2"]; + } + + return ["bar1", "bar2"]; + } + + if (selector === "error") { + throw new Error("This is a plugin error"); + } + + return []; + } +};