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

Change the default behaviour of useNewWDA capability #456

Merged
merged 6 commits into from
Jun 24, 2017
Merged

Change the default behaviour of useNewWDA capability #456

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
062036b
Try to reuse running WDA instance and tune WDA startup logic
Jun 17, 2017
c254f43
Add more detailed issue description
Jun 19, 2017
698a79d
Tune retries count
Jun 20, 2017
9cfcc16
Avoid error message duplication
Jun 20, 2017
b60f57f
Tune max line lengths
Jun 21, 2017
0c9a16c
Keep real device tutorial URL in a separate constant
Jun 21, 2017
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
4 changes: 2 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ See [real device configuration documentation](docs/real-device-config.md).

### Known problems

After many failures on real devices, there can be a state where the device will no longer accept connections. To possibly remedy this, set the `useNewWDA` capability to `true`.
After many failures on real devices, there can be a state where the device will no longer accept connections. To possibly remedy this reboot the device. Read https://github.com/facebook/WebDriverAgent/issues/507 for more details. [Getting full control](https://github.com/appium/appium/blob/master/docs/en/advanced-concepts/wda-custom-server.md) over WDA server might help to workaround the problem.

#### Weird state

Expand Down Expand Up @@ -134,7 +134,7 @@ Differences noted here
|`usePrebuiltWDA`|Skips the build phase of running the WDA app. Building is then the responsibility of the user. Only works for Xcode 8+. Defaults to `false`.|e.g., `true`|
|`preventWDAAttachments`|Sets read only permissions to Attachments subfolder of WebDriverAgent root inside Xcode's DerivedData. This is necessary to prevent XCTest framework from creating tons of unnecessary screenshots and logs, which are impossible to turn off using programming interfaces provided by Apple.|Setting the capability to `true` will set Posix permissions of the folder to `555` and `false` will reset them back to `755`. `true` by default|
|`webDriverAgentUrl`|If provided, Appium will connect to an existing WebDriverAgent instance at this URL instead of starting a new one.|e.g., `http://localhost:8100`|
|`useNewWDA`|If `true`, forces uninstall of any existing WebDriverAgent app on device. This can provide stability in some situations. Defaults to `false`.|e.g., `true`|
|`useNewWDA`|If `true`, forces uninstall of any existing WebDriverAgent app on device. Set it to `true` if you want to apply different startup options for WebDriverAgent for each session. Although, it is only guaranteed to work stable on Simulator. Real devices require WebDriverAgent client to run for as long as possible without reinstall/restart to avoid issues like https://github.com/facebook/WebDriverAgent/issues/507. The `false` value (the default behaviour since Appium 1.6.6) will try to detect currently running WDA listener executed by previous testing session(s) and reuse it if possible, which is highly recommended for real device testing and to speed up suites of multiple tests in general. A new WDA session will be triggered at the default URL (http://localhost:8100) if WDA is not listening and `webDriverAgentUrl` capability is not set. The negative/unset value of `useNewWDA` capability has no effect prior to Appium 1.6.6.|e.g., `true`|
Copy link
Member

Choose a reason for hiding this comment

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

probably don't want to refer to appium versions in the xcuitest repo, maybe refer to the xcuitest version? unless the default is set in appium and not here. which might be the case, hmm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to driver version

Copy link
Member

Choose a reason for hiding this comment

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

i don't see a change

|`wdaLaunchTimeout`|Time, in ms, to wait for WebDriverAgewnt to be pingable. Defaults to 60000ms.|e.g., `30000`|
|`wdaConnectionTimeout`|Timeout, in ms, for waiting for a response from WebDriverAgent. Defaults to 240000ms.|e.g., `1000`|
|`resetOnSessionStartOnly`|Whether to perform reset on test session finish (`false`) or not (`true`). Keeping this variable set to `true` and Simulator running (the default behaviour since version 1.6.4) may significantly shorten the duratiuon of test session initialization.|Either `true` or `false`. Defaults to `true`|
Expand Down
75 changes: 56 additions & 19 deletions lib/driver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { detectUdid, getAndCheckXcodeVersion, getAndCheckIosSdkVersion,
clearSystemFiles, translateDeviceName } from './utils';
import { getConnectedDevices, runRealDeviceReset, installToRealDevice,
getRealDeviceObj } from './real-device-management';
import { NoSessionProxy } from "./wda/no-session-proxy";
import B from 'bluebird';
import { version } from '../../package.json'; // eslint-disable-line import/no-unresolved

Expand Down Expand Up @@ -383,32 +384,59 @@ class XCUITestDriver extends BaseDriver {
}

async startWda (sessionId, realDevice) {
let startupRetries = this.opts.wdaStartupRetries || WDA_STARTUP_RETRIES;
let startupRetryInterval = this.opts.wdaStartupRetryInterval || WDA_STARTUP_RETRY_INTERVAL;
await retryInterval(startupRetries, startupRetryInterval, async () => {
this.logEvent('wdaStartAttempted');
this.wda = new WebDriverAgent(this.xcodeVersion, this.opts);
this.wda = new WebDriverAgent(this.xcodeVersion, this.opts);

if (this.opts.useNewWDA) {
log.debug(`Capability 'useNewWDA' set, so uninstalling WDA before proceeding`);
await this.wda.uninstall();
this.logEvent('wdaUninstalled');
if (this.opts.useNewWDA) {
log.debug(`Capability 'useNewWDA' set to true, so uninstalling WDA before proceeding`);
await this.wda.quit();
await this.wda.uninstall();
this.logEvent('wdaUninstalled');
} else if (!util.hasValue(this.wda.webDriverAgentUrl)) {
log.debug(`Capability 'useNewWDA' set to false, so trying to reuse currently running WDA instance at ${this.wda.url}...`);
const noSessionProxy = new NoSessionProxy({
server: this.wda.url.hostname,
port: this.wda.url.port,
base: '',
timeout: 3000,
});
try {
const status = await noSessionProxy.command('/status', 'GET');
if (!status) {
throw new Error(`WDA response to /status command should be defined.`);
}
log.debug(`Detected WDA listening at ${this.wda.url}. Setting WDA endpoint to ${this.wda.url}`);
this.wda.webDriverAgentUrl = this.wda.url;
} catch (err) {
log.debug(`WDA is not listening at ${this.wda.url}. Rebuilding...`);
}
}

// local helper for the two places we need to uninstall wda and re-start it
let quitAndUninstall = async (msg) => {
log.debug(msg);
log.debug('Quitting and uninstalling WebDriverAgent, then retrying');
await this.wda.quit();
await this.wda.uninstall();
throw new Error(msg);
};
// local helper for the two places we need to uninstall wda and re-start it
const quitAndUninstall = async (msg) => {
log.debug(msg);
log.debug('Quitting and uninstalling WebDriverAgent, then retrying');
await this.wda.quit();
await this.wda.uninstall();
throw new Error(msg);
};

const startupRetries = this.opts.wdaStartupRetries || WDA_STARTUP_RETRIES;
const startupRetryInterval = this.opts.wdaStartupRetryInterval || WDA_STARTUP_RETRY_INTERVAL;
let launchRetriesCount = 0;
let sessionStartupRetriesCount = 0;
await retryInterval(startupRetries, startupRetryInterval, async () => {
this.logEvent('wdaStartAttempted');
let wdaStatus = null;
try {
wdaStatus = await this.wda.launch(sessionId, realDevice);
} catch (err) {
launchRetriesCount++;
this.logEvent('wdaStartFailed');
if (this.isRealDevice() && launchRetriesCount > 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I presume you want to not retry if there is an xcodebuild failure on a real device?

This will not do that. Any error thrown here will trigger a retry at the retryInterval level (on line 427). It would be better to check for real device before the retryInterval, and adjust the startupRetries accordingly.

const startupRetries = this.isRealDevice() ? 1 : (this.opts.wdaStartupRetries || WDA_STARTUP_RETRIES);

Copy link
Contributor Author

@mykola-mokhnach mykola-mokhnach Jun 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

It might not be a good idea to implicitly override user-supplied option.

How about this one?

const startupRetries = this.opts.wdaStartupRetries || (this.isRealDevice() ? 1 : WDA_STARTUP_RETRIES);

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. Yeah, that would work. Since the number varies, it should probably be logged, as well.

throw new Error(`Unable to launch WebDriverAgent because of xcodebuild failure: ${err.message}. ` +
`Make sure you follow the tutorial at https://github.com/appium/appium-xcuitest-driver/blob/master/docs/real-device-config.md. ` +
`Try to remove the WebDriverAgentRunner application from the device if it is installed and reboot the device.`);
}
await quitAndUninstall(`Unable to launch WebDriverAgent because of xcodebuild failure: ${err.message}`);
}

Expand All @@ -434,7 +462,13 @@ class XCUITestDriver extends BaseDriver {
});
this.logEvent('wdaSessionStarted');
} catch (err) {
return await quitAndUninstall(`Unable to start WebDriverAgent session: ${err.message}`);
sessionStartupRetriesCount++;
if (this.isRealDevice() && sessionStartupRetriesCount > 1) {
throw new Error(`Unable to start WebDriverAgent session: ${err.message}. ` +
`Make sure you follow the tutorial at https://github.com/appium/appium-xcuitest-driver/blob/master/docs/real-device-config.md. ` +
`Try to remove the WebDriverAgentRunner application from the device if it is installed and reboot the device.`);
}
await quitAndUninstall(`Unable to start WebDriverAgent session: ${err.message}`);
}

this.opts.preventWDAAttachments = !util.hasValue(this.opts.preventWDAAttachments) || this.opts.preventWDAAttachments;
Expand Down Expand Up @@ -530,7 +564,9 @@ class XCUITestDriver extends BaseDriver {
log.debug(`Unable to DELETE session on WDA: '${err.message}'. Continuing shutdown.`);
}
}
await this.wda.quit();
if (!this.wda.webDriverAgentUrl && this.opts.useNewWDA) {
await this.wda.quit();
}
}
}

Expand Down Expand Up @@ -807,6 +843,7 @@ class XCUITestDriver extends BaseDriver {

// `resetOnSessionStartOnly` should be set to true by default
this.opts.resetOnSessionStartOnly = !util.hasValue(this.opts.resetOnSessionStartOnly) || this.opts.resetOnSessionStartOnly;
this.opts.useNewWDA = util.hasValue(this.opts.useNewWDA) ? this.opts.useNewWDA : false;

// finally, return true since the superclass check passed, as did this
return true;
Expand Down