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
+60 −23
Merged

Change the default behaviour of useNewWDA capability #456

Show file tree
Hide file tree
Changes from 3 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
78 changes: 57 additions & 21 deletions lib/driver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ 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


const SAFARI_BUNDLE_ID = 'com.apple.mobilesafari';
const WDA_STARTUP_RETRIES = 2;
const WDA_SIM_STARTUP_RETRIES = 2;
const WDA_REAL_DEV_STARTUP_RETRIES = 1;
const DEFAULT_TIMEOUT_KEY = 'default';
const WDA_STARTUP_RETRY_INTERVAL = 10000;
const DEFAULT_SETTINGS = {
Expand Down Expand Up @@ -383,33 +385,58 @@ 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 || (this.isRealDevice() ? WDA_REAL_DEV_STARTUP_RETRIES : WDA_SIM_STARTUP_RETRIES);
const startupRetryInterval = this.opts.wdaStartupRetryInterval || WDA_STARTUP_RETRY_INTERVAL;
await retryInterval(startupRetries, startupRetryInterval, async () => {
this.logEvent('wdaStartAttempted');
let wdaStatus = null;
try {
wdaStatus = await this.wda.launch(sessionId, realDevice);
} catch (err) {
this.logEvent('wdaStartFailed');
await quitAndUninstall(`Unable to launch WebDriverAgent because of xcodebuild failure: ${err.message}`);
let errorMsg = `Unable to launch WebDriverAgent because of xcodebuild failure: ${err.message}`;
if (this.isRealDevice()) {
errorMsg = `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(errorMsg);
}

this.proxyReqRes = this.wda.proxyReqRes.bind(this.wda);
Expand All @@ -434,7 +461,13 @@ class XCUITestDriver extends BaseDriver {
});
this.logEvent('wdaSessionStarted');
} catch (err) {
return await quitAndUninstall(`Unable to start WebDriverAgent session: ${err.message}`);
let errorMsg = `Unable to start WebDriverAgent sesssion because of xcodebuild failure: ${err.message}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor into a function call to handle the error, so it's not repeated here?

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.

error messages are different for both cases. I separated them intentionally for a case when it is necessary to add any other helpful message details to help user to fix his problem for the particular case.

if (this.isRealDevice()) {
errorMsg = `Unable to start WebDriverAgent session 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(errorMsg);
}

this.opts.preventWDAAttachments = !util.hasValue(this.opts.preventWDAAttachments) || this.opts.preventWDAAttachments;
Expand Down Expand Up @@ -530,7 +563,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 +842,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