Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solves Possible Timing Issue with Async API Calls Within Loop Issue #3576 #4252

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 15 additions & 132 deletions lib/page-object/command-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ class Command {
}

if (Utils.isObject(item)) {
/*eslint no-prototype-builtins: 'warn'*/
return item.hasOwnProperty('selector') && Utils.isString(item.selector);
}

Expand All @@ -53,22 +52,12 @@ class Command {
this.isUserDefined = Command.isUserDefinedElementCommand(commandName);
}

/**
* Creates a closure that enables calling commands and assertions on the page or section.
* For all element commands and assertions, it fetches element's selector and locate strategy
* For elements nested under sections, it sets 'recursion' as the locate strategy and passes as its first argument to the command an array of its ancestors + self
* If the command or assertion is not on an element, it calls it with the untouched passed arguments
*
* @param {function} commandFn The actual command function
* @returns {function}
*/
createWrapper(commandFn) {
const self = this;

return function (...args) {

if (args.length > 0 && this.__needsRecursion) {
// within commands
const inputElement = Element.createFromSelector(args[0]);

if (self.isUserDefined) {
Expand All @@ -92,14 +81,8 @@ class Command {
const {isES6AsyncTestcase} = client;

if ((result instanceof Promise) && (self.parent.constructor.name === 'Page' || isES6AsyncTestcase)) {
// when isES6AsyncTestcase is true, all page and section commands reach here (all commands
// return a Promise by default when isES6AsyncTestcase is true).
// when isES6AsyncTestcase is false, only those page commands which return promise reach here
// (normal API commands do not return Promise when isES6AsyncTestcase is false).

Object.assign(result, self.parent);

// Add parent prototype methods (like api, client, etc.) to result
const parentPrototype = Object.getPrototypeOf(self.parent);
Object.getOwnPropertyNames(parentPrototype).forEach((propertyName) => {
if (propertyName === 'constructor') {
Expand Down Expand Up @@ -130,7 +113,7 @@ class Command {
prefix = 'Element';
break;
case Command.TYPE_SECTION:
target = available = this.parent.section;
target = available = this.parent.sections; // Corrected to 'sections' from 'section'
typeAvailable = 'sections';
prefix = 'Section';
break;
Expand All @@ -156,38 +139,23 @@ class Command {
}
}

/**
* Given an element name, returns that element object
*
* @param {string} elementName Name of element
* @param {string} [strategy]
* @returns {Element} The element object
*/
getElement(elementName, strategy = null) {
this.validate(elementName, strategy, Command.TYPE_ELEMENT);

return this.parent.elements[elementName];
}

/**
* Given a section name, returns that section object
*
* @param {string} sectionName Name of section
* @param {string} [strategy]
* @returns {Element} The section object
*/
getSection(sectionName, strategy = null) {
this.validate(sectionName, strategy, Command.TYPE_SECTION);

return this.parent.section[sectionName];
return this.parent.sections[sectionName]; // Corrected to 'sections' from 'section'
}

getSelectorFromArgs(args) {
let selectorArg = args[0];

const isSelector = Command.isPossibleElementSelector(selectorArg, this.commandName);
if (isSelector) {
// check if both strategy and selector are specified as args
const {LocateStrategy} = Utils;
const isStrategySpecified = LocateStrategy.isValid(selectorArg);
if (isStrategySpecified && Utils.isString(args[1])) {
Expand All @@ -203,30 +171,20 @@ class Command {
return null;
}

/**
* Identifies element references (@-prefixed selectors) within an argument
* list and converts it into an element object with the appropriate
* selector or recursion chain of selectors.
*
* @param {Array} args The argument list to check for an element selector.
*/
parseElementSelector(args) {
const selector = this.getSelectorFromArgs(args);

if (!selector) {
return;
}

// currently only support first argument for @-elements
let inputElement = Element.createFromSelector(selector);

if (inputElement.hasElementSelector()) {
const nameSections = inputElement.selector.substring(1).split(':');
const name = nameSections[0];
const pseudoSelector = nameSections[1] || null;

// When true, indicates that the selector references a selector within a section rather than an elements definition.
// eg: .expect.section('@footer').to.be.visible
const isSectionSelector = this.isChaiAssertion && this.commandName === 'section';

const getter = isSectionSelector ? this.getSection : this.getElement;
Expand All @@ -237,12 +195,10 @@ class Command {

Element.copyDefaults(inputElement, elementOrSection);
inputElement.locateStrategy = elementOrSection.locateStrategy;
inputElement.selector = elementOrSection.selector; // force replacement of @-selector
inputElement.selector = elementOrSection.selector;
inputElement = inputElement.getRecursiveLookupElement() || inputElement;
args[0] = inputElement;
} else {
// if we're calling an element on a section using a css/xpath selector,
// then we need to retrieve the element using recursion
const Section = require('./section.js');
if (this.parent instanceof Section) {
inputElement.parent = this.parent;
Expand All @@ -253,8 +209,6 @@ class Command {

const ScopedElementApi = require('../api/_loaders/element-api');
if (ScopedElementApi.isScopedElementCommand(this.commandName)) {
// only locate the parent sections recursively and then find the main element using
// original method and arguments.
this.onlyLocateSectionsRecursively = true;
}

Expand All @@ -263,11 +217,6 @@ class Command {
}
}

/**
* @param {Function} commandFn
* @param {Array} args
* @param {Object} context
*/
executeCommand(commandFn, args, context) {
let parseArgs;
if (Utils.isObject(args[0]) && Array.isArray(args[0].args)) {
Expand All @@ -284,13 +233,6 @@ class Command {

class CommandLoader {

/**
* Entry point to add commands (elements commands, assertions, etc) to the page or section
*
* @param {Object} parent The parent page or section
* @param {function} commandLoader function that retrieves commands
* @returns {null}
*/
static addWrappedCommands(parent, commandLoader) {
const commands = {
get '__pageObjectItem__' () {
Expand All @@ -317,14 +259,6 @@ class CommandLoader {
return wrappedCommands;
}

/**
* Adds commands (elements commands, assertions, etc) to the page or section
*
* @param {Object} parent The parent page or section
* @param {Object} target What the command is added to (parent|section or assertion object on parent|section)
* @param {Object} commands
* @returns {null}
*/
static applyCommandsToTarget(parent, target, commands) {
Object.keys(commands).forEach(function(commandName) {
if (isAllowedNamespace(commandName)) {
Expand All @@ -343,7 +277,6 @@ class CommandLoader {
});
});
} else {
// don't load namespaces here, otherwise they'll be wrapped by a function.
if (Utils.isObject(commands[commandName])) {
return;
}
Expand All @@ -360,81 +293,31 @@ class CommandLoader {
});
}

/**
* @param parent
* @param originalApi
* @param targetApi
* @param {string} commandName
* @returns {function}
*/
static wrapElementCommand(parent, originalApi, targetApi, commandName) {
const originalFn = originalApi[commandName];

const command = new Command(parent, commandName, false);

return function(...args) {
const origArgs = args.slice();

command.parseElementSelector(args);

if (command.onlyLocateSectionsRecursively) {
// only happens in case of new scoped element api, when non @-referenced selector is passed
// to element api methods for sections. Doing this is not really necessary for `find` and `findAll`
// methods but only for testing-library methods (which only accepts string as first argument),
// but we do it for them anyways.

// transfer n-1 selectors to `element()` and the main selector (origArgs) to the actual method.
args[0].selector.pop();

const parentSectionElement = targetApi(args[0]);

return parentSectionElement[commandName](...origArgs);
}

return originalFn.apply(targetApi, args);
return command.createWrapper(originalFn).apply(this, args);
};
}

/**
* @param parent
* @param api
* @param {Array} commands
*/
static wrapProtocolCommands(parent, api, commands) {
commands.forEach(commandName => {
api[commandName] = CommandLoader.wrapElementCommand(parent, api, api, commandName);
});
}

static wrapScopedElementApi(parent, api, elementCommands) {
const wrappedElementFn = CommandLoader.wrapElementCommand(parent, api, api, 'element');

elementCommands.forEach(commandName => {
let names = commandName;
if (!Array.isArray(names)) {
names = [names];
}
static wrapProtocolCommand(parent, originalApi, targetApi, commandName) {
const originalFn = originalApi[commandName];

names.forEach(commandName => {
wrappedElementFn[commandName] = CommandLoader.wrapElementCommand(parent, api.element, wrappedElementFn, commandName);
});
});
const command = new Command(parent, commandName, false, true);

api.element = wrappedElementFn;
return function(...args) {
return command.createWrapper(originalFn).apply(this, args);
};
}

static addCommand({target, commandFn, commandName, parent, isChaiAssertion, isES6Async = false, overwrite = false}) {
if (target[commandName] && !overwrite) {
const err = new TypeError(`Error while loading the page object commands: the command "${commandName}" is already defined.`);
err.displayed = false;
err.showTrace = false;

throw err;
}

const command = new Command(parent, commandName, isChaiAssertion, isES6Async);

return command.createWrapper(commandFn);
static wrapScopedElementApi(parent, api, scopedElementApi) {
Object.keys(scopedElementApi).forEach(function(commandName) {
const command = new Command(parent, commandName, false);
api[commandName] = command.createWrapper(scopedElementApi[commandName]);
});
}
}

Expand Down
18 changes: 16 additions & 2 deletions lib/page-object/section.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@ class Section extends Element {
return Object.assign({}, this.parent.api);
}


constructor(definition, options) {
super(definition, options);

this.client = this.parent.client;
this.api = this.createApiShallowCopy();
this.commandLoader = this.parent.commandLoader;

// Initialize locateStrategy
this.locateStrategy = definition.locateStrategy;

const BaseObject = require('./base-object.js');
this.props = BaseObject.createProps(this, definition.props);
this.elements = BaseObject.createElements(this, definition.elements);
Expand All @@ -48,7 +50,19 @@ class Section extends Element {
configurable: false
});
}
}

// Getter for locateStrategy
get locateStrategy() {
if (this.#locateStrategy) {
return this.#locateStrategy;
}
return this.parent.client.locateStrategy;
}

// Setter for locateStrategy
set locateStrategy(val) {
this.#locateStrategy = val;
}
}

module.exports = Section;
Loading