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.

Sign up for GitHub

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

Jump to bottom

Fix visible commands for Appium. #3566

Merged
merged 10 commits into from Feb 9, 2023
+274 −15
Merged

Fix visible commands for Appium. #3566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8a6df55
Fix visible commands for Appium.
garg3133 Jan 11, 2023
fe571b7
Move appium visible logic to method-mappings.
garg3133 Jan 19, 2023
64132ee
Add assertions for isAppiumClient.
garg3133 Jan 19, 2023
43e4cb8
Fix visible commands for BrowserStack.
garg3133 Jan 19, 2023
6782f1b
Fix locate strategy for some visible command.
garg3133 Jan 19, 2023
35bfe4f
Create Locator sub-class to work with Appium client.
garg3133 Feb 3, 2023
199fd3a
Create separate AppiumLocator class.
garg3133 Feb 9, 2023
e16fbcb
Create common locateInstanceOfBy method.
garg3133 Feb 9, 2023
96af4dc
Rename appiumLocator.js to appium-locator.js
beatfactor Feb 9, 2023
46228d6
Update method-mappings.js
beatfactor Feb 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 17 additions & 1 deletion lib/core/client.js
View file
Open in desktop
Expand Up @@ -10,6 +10,7 @@ const Transport = require('../transport');
const Element = require('../element');
const ApiLoader = require('../api');
const ElementGlobal = require('../api/_loaders/element-global.js');
const Factory = require('../transport/factory.js');
const {isAndroid, isIos} = require('../utils/mobile');

const {LocateStrategy, Locator} = Element;
Expand Down Expand Up @@ -76,7 +77,7 @@ class NightwatchAPI {
return false;
}

return this.platformName.toLowerCase() === platform.toLowerCase();
return this.platformName.toLowerCase() === platform.toLowerCase();
}

isIOS() {
Expand Down Expand Up @@ -114,6 +115,21 @@ class NightwatchAPI {
isOpera() {
return this.capabilities.browserName === Browser.OPERA;
}

isAppiumClient() {
if (this.options.selenium && this.options.selenium.use_appium) {
return true;
}

// Handle BrowserStack case
// (BrowserStack always returns platformName in capabilities)
garg3133 marked this conversation as resolved.
Show resolved Hide resolved
const isMobile = this.__isPlatformName('android') || this.__isPlatformName('ios');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use isMobile from utils

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot use isMobile from utils here because that method only considers desiredCapabilites whereas over here we need to check is android or ios is present as platformName inside the capabilities returned by BrowserStack.

This is necessary because users might not explicitly specify platformName in their desiredCapabilites (for example, when testing on mobile browser on BrowserStack, platformName in desired capabilites is not necessary) but BrowserStack would always send back the platformName in capabilities.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would also need to tweek isAndroid() and isIOS() methods available in our API accordingly for it to consider capabilities as well, but that is out of scope of this PR and I'll create a separate issue for that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created an issue here: #3603

if (Factory.usingBrowserstack(this.options) && isMobile) {
return true;
}

return false;
}
}

class NightwatchClient extends EventEmitter {
Expand Down
31 changes: 31 additions & 0 deletions lib/element/appium-locator.js
View file
Open in desktop
@@ -0,0 +1,31 @@
const {By, RelativeBy} = require('selenium-webdriver');

const LocateElement = require('./locator.js');
const {AVAILABLE_LOCATORS} = LocateElement;

class AppiumLocator extends LocateElement {
/**
* @param {object|string} element
* @return {By|RelativeBy}
*/
static create(element) {
if (!element) {
throw new Error(`Error while trying to locate element: missing element definition; got: "${element}".`);
}

const byInstance = LocateElement.locateInstanceOfBy(element);
if (byInstance !== null) {
return byInstance;
}

const elementInstance = LocateElement.createElementInstance(element);

if (elementInstance.locateStrategy === 'id') {
return new By(elementInstance.locateStrategy, elementInstance.selector);
}

return By[AVAILABLE_LOCATORS[elementInstance.locateStrategy]](elementInstance.selector);
}
}

module.exports = AppiumLocator;
32 changes: 27 additions & 5 deletions lib/element/locator.js
View file
Open in desktop
Expand Up @@ -3,7 +3,7 @@ const Element = require('./index.js');
const ElementsByRecursion = require('./locate/elements-by-recursion.js');
const SingleElementByRecursion = require('./locate/single-element-by-recursion.js');

const availableLocators = {
const AVAILABLE_LOCATORS = {
'css selector': 'css',
'id': 'id',
'link text': 'linkText',
Expand All @@ -17,13 +17,28 @@ const availableLocators = {
class LocateElement {
/**
* @param {object|string} element
* @return {*}
* @return {By|RelativeBy}
*/
static create(element) {
if (!element) {
throw new Error(`Error while trying to locate element: missing element definition; got: "${element}".`);
}

const byInstance = LocateElement.locateInstanceOfBy(element);
if (byInstance !== null) {
return byInstance;
}

const elementInstance = LocateElement.createElementInstance(element);

return By[AVAILABLE_LOCATORS[elementInstance.locateStrategy]](elementInstance.selector);
}

/**
* @param {object} element
* @return {By|RelativeBy|null}
*/
static locateInstanceOfBy(element) {
if (element instanceof By) {
return element;
}
Expand All @@ -36,6 +51,14 @@ class LocateElement {
return element.value;
}

return null;
}

/**
* @param {object|string} element
* @return {Element}
*/
static createElementInstance(element) {
if (typeof element != 'object' && typeof element != 'string') {
throw new Error(`Invalid element definition type; expected string or object, but got: ${typeof element}.`);
}
Expand All @@ -50,9 +73,7 @@ class LocateElement {
selector = element;
}

const elementInstance = Element.createFromSelector(selector, strategy);

return By[availableLocators[elementInstance.locateStrategy]](elementInstance.selector);
return Element.createFromSelector(selector, strategy);
}

get api() {
Expand Down Expand Up @@ -316,3 +337,4 @@ class NoSuchElementError extends Error {

module.exports = LocateElement;
module.exports.NoSuchElementError = NoSuchElementError;
module.exports.AVAILABLE_LOCATORS = AVAILABLE_LOCATORS;
17 changes: 13 additions & 4 deletions lib/transport/selenium-webdriver/method-mappings.js
View file
Open in desktop
@@ -1,5 +1,6 @@
const {WebElement, WebDriver, Origin, By} = require('selenium-webdriver');
const {Locator} = require('../../element');
const AppiumLocator = require('../../element/appium-locator.js');
const {isString} = require('../../utils');
const fs = require('fs');
const cdp = require('./cdp.js');
Expand Down Expand Up @@ -367,7 +368,12 @@ module.exports = class MethodMappings {
},

async locateMultipleElements(element) {
const locator = Locator.create(element);
let locator;
if (this.transport.api.isAppiumClient()) {
locator = AppiumLocator.create(element);
} else {
locator = Locator.create(element);
}
const resultValue = await this.driver.findElements(locator);

if (Array.isArray(resultValue) && resultValue.length === 0) {
Expand Down Expand Up @@ -531,11 +537,14 @@ module.exports = class MethodMappings {
return `/element/${id}/location_in_view`;
},

async isElementDisplayed(webElementOrId) {
isElementDisplayed(webElementOrId) {
if (this.transport.api.isAppiumClient()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not use api methods here. We can move isAppium client to util and api method can use take use of that util function

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up on the above response, this method cannot be moved to utils in its current form since for handling the BrowserStack case, we'd need the capabilities returned by BrowserStack after a session is established with it, and those capabilities are only made available in client.js as this.capabilities.

return `/element/${webElementOrId}/displayed`;
}

const element = this.getWebElement(webElementOrId);
const result = await element.isDisplayed();

return result;
return element.isDisplayed();
},

async isElementEnabled(webElementOrId) {
Expand Down
25 changes: 25 additions & 0 deletions test/lib/nocks.js
View file
Open in desktop
Expand Up @@ -138,6 +138,18 @@ module.exports = {
return this;
},

appiumElementFound() {
nock('http://localhost:10195')
.post('/wd/hub/session/1352110219202/elements', {'using': 'id', 'value': 'com.app:id/web-login'})
.reply(200, {
status: 0,
state: 'success',
value: [{'element-6066-11e4-a52e-4f735466cecf': '0'}]
});

return this;
},

click() {
nock('http://localhost:10195')
.post('/wd/hub/session/1352110219202/element/0/click')
Expand Down Expand Up @@ -370,6 +382,19 @@ module.exports = {
return this;
},

appiumElementVisible() {
nock('http://localhost:10195')
.get('/wd/hub/session/1352110219202/element/0/displayed')
.reply(200, {
status: 0,
sessionId: '1352110219202',
value: true,
state: 'success'
});

return this;
},

notVisible(times) {
var mock = nock('http://localhost:10195')
.post('/wd/hub/session/1352110219202/execute/sync');
Expand Down
84 changes: 82 additions & 2 deletions test/src/api/commands/element/testIsVisible.js
View file
Open in desktop
Expand Up @@ -12,13 +12,21 @@ describe('isVisible', function () {
url: '/wd/hub/session/1352110219202/execute/sync',
method: 'POST'
});
MockServer.removeMock({
url: '/wd/hub/session/1352110219202/elements',
method: 'POST'
});
MockServer.removeMock({
url: '/wd/hub/session/1352110219202/element/999/displayed',
method: 'GET'
});
});

after(function (done) {
CommandGlobals.afterEach.call(this, done);
});

it('client.isVisible()', function (done) {
it('client.isVisible() [visible]', function (done) {
MockServer.addMock({
url: '/wd/hub/session/1352110219202/execute/sync',
method: 'POST',
Expand All @@ -38,7 +46,7 @@ describe('isVisible', function () {
this.client.start(done);
});

it('client.isVisible()', function (done) {
it('client.isVisible() [not visible]', function (done) {
MockServer.addMock({
url: '/wd/hub/session/1352110219202/execute/sync',
method: 'POST',
Expand All @@ -57,4 +65,76 @@ describe('isVisible', function () {

this.client.start(done);
});

it('client.isVisible() [visible] -- appium', function (done) {
MockServer
.addMock({
url: '/wd/hub/session/1352110219202/elements',
postdata: {
using: 'id',
value: 'com.app:id/weblogin'
},
method: 'POST',
response: JSON.stringify({
status: 0,
state: 'success',
value: [{'element-6066-11e4-a52e-4f735466cecf': '999'}]
})
})
.addMock({
url: '/wd/hub/session/1352110219202/element/999/displayed',
method: 'GET',
response: JSON.stringify({
value: true
})
});

// Make appium client
this.client.api.options.selenium.use_appium = true;

this.client.api.isVisible('id', 'com.app:id/weblogin', function callback(result) {
assert.strictEqual(result.value, true);
}).isVisible({selector: 'com.app:id/weblogin', locateStrategy: 'id'}, function callback(result) {
assert.strictEqual(result.value, true);
});

this.client.start(done);
});

it('client.isVisible() [not visible] -- appium', function (done) {
MockServer
.addMock({
url: '/wd/hub/session/1352110219202/elements',
postdata: {
using: 'id',
value: 'com.app:id/weblogin'
},
method: 'POST',
response: JSON.stringify({
status: 0,
state: 'success',
value: [{'element-6066-11e4-a52e-4f735466cecf': '999'}]
})
})
garg3133 marked this conversation as resolved.
Show resolved Hide resolved
.addMock({
url: '/wd/hub/session/1352110219202/element/999/displayed',
method: 'GET',
response: JSON.stringify({
value: false
})
});

// Make appium client
this.client.api.options.selenium.use_appium = true;

assert.strictEqual(this.client.api.isAppiumClient(), true);

this.client.api.isVisible('id', 'com.app:id/weblogin', function callback(result) {
assert.strictEqual(result.value, false);
}).isVisible({selector: 'com.app:id/weblogin', locateStrategy: 'id'}, function callback(result) {
assert.strictEqual(result.value, false);
});

this.client.start(done);
});
});
42 changes: 42 additions & 0 deletions test/src/api/commands/element/testWaitForElementVisible.js
View file
Open in desktop
Expand Up @@ -336,4 +336,46 @@ describe('waitForElementVisible', function () {
});
});

it('client.waitForElementVisible() success with appium client', function () {
MockServer
.addMock({
url: '/wd/hub/session/1352110219202/elements',
postdata: {
using: 'id',
value: 'com.app:id/web-login'
},
method: 'POST',
response: JSON.stringify({
status: 0,
state: 'success',
value: [{'element-6066-11e4-a52e-4f735466cecf': '99'}]
})
}, undefined, true)
.addMock({
url: '/wd/hub/session/1352110219202/element/99/displayed',
method: 'GET',
response: JSON.stringify({
value: true
})
}, undefined, true);

// Make appium client
this.client.api.options.selenium.use_appium = true;

assert.strictEqual(this.client.api.isAppiumClient(), true);

this.client.api.waitForElementVisible('id', 'com.app:id/web-login', function callback(result, instance) {
assert.strictEqual(instance.elementId, '99');
assert.strictEqual(result.value, true);
}).waitForElementVisible({selector: 'com.app:id/web-login', locateStrategy: 'id'}, function callback(result, instance) {
assert.strictEqual(instance.elementId, '99');
assert.strictEqual(result.value, true);
});

return this.client.start(function (err) {
if (err) {
throw err;
}
});
});
});