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

fix: terminateApp with devicectl for iOS 17 #1997

Merged
merged 11 commits into from
Sep 19, 2023
2 changes: 1 addition & 1 deletion lib/commands/app-management.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export default {
}

// @ts-expect-error - do not assign arbitrary properties to `this.opts`
return await this.opts.device.terminateApp(bundleId);
return await this.opts.device.terminateApp(bundleId, this.opts.platformVersion);
},

/**
Expand Down
12 changes: 2 additions & 10 deletions lib/commands/performance.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import path from 'path';
import {fs, zip, logger, util, tempDir} from 'appium/support';
import {SubProcess, exec} from 'teen_process';
import {encodeBase64OrUpload} from '../utils';
import { XCRUN, requireXcrun } from '../xcrun';
import {waitForCondition} from 'asyncbox';
import B from 'bluebird';

Expand All @@ -18,19 +19,10 @@ const DEFAULT_PROFILE_NAME = 'Activity Monitor';
const DEFAULT_EXT = '.trace';
const DEFAULT_PID = 'current';
const INSTRUMENTS = 'instruments';
const XCRUN = 'xcrun';
const XCTRACE = 'xctrace';

async function requireXctrace() {
let xcrunPath;
try {
xcrunPath = await fs.which(XCRUN);
} catch (e) {
throw new Error(
`${XCRUN} has not been found in PATH. ` +
`Please make sure XCode development tools are installed`,
);
}
const xcrunPath = await requireXcrun();
try {
await exec(xcrunPath, [XCTRACE, 'help']);
} catch (e) {
Expand Down
77 changes: 60 additions & 17 deletions lib/ios-deploy.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {fs, timing} from 'appium/support';
import {fs, timing, util} from 'appium/support';
import path from 'path';
import {services, utilities, INSTRUMENT_CHANNEL} from 'appium-ios-device';
import B from 'bluebird';
Expand All @@ -7,6 +7,7 @@ import _ from 'lodash';
import {exec} from 'teen_process';
import {extractBundleId} from './app-utils';
import {pushFolder} from './ios-fs-helpers';
import { requireXcrun } from './xcrun';

const APPLICATION_INSTALLED_NOTIFICATION = 'com.apple.mobile.application_installed';
const INSTALLATION_STAGING_DIR = 'PublicStaging';
Expand Down Expand Up @@ -192,7 +193,7 @@ class IOSDeploy {
}
}

async terminateApp(bundleId) {
async terminateApp(bundleId, platformVersion) {
let instrumentService;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say it makes sense to keep platformVersion as IOSDeploy property as it might be useful in other methods as well

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, that sounds good. let me do it as a follow-up pr (to pr split)

let installProxyService;
try {
Expand All @@ -204,22 +205,63 @@ class IOSDeploy {
}
const executableName = apps[bundleId].CFBundleExecutable;
log.debug(`The executable name for the bundle id '${bundleId}' was '${executableName}'`);
instrumentService = await services.startInstrumentService(this.udid);
const processes = await instrumentService.callChannel(
INSTRUMENT_CHANNEL.DEVICE_INFO,
'runningProcesses',
);
const process = processes.selector.find((process) => process.name === executableName);
if (!process) {
log.info(`The process of the bundle id '${bundleId}' was not running`);
return false;

// 'devicectl' has overhead (generally?) than the instrument service via appium-ios-device,
// so hre uses the 'devicectl' only for iOS 17+.
if (util.compareVersions(platformVersion, '>=', '17.0')) {
log.debug(`Calling devicectl to kill the process`);

// FIXME: replace `devicectl` command with a wrapper later
// or remove after implementing appium-ios-device for iOS 17+.
KazuCocoa marked this conversation as resolved.
Show resolved Hide resolved

const xcrunBinnaryPath = await requireXcrun();
const {stdout} = await exec(xcrunBinnaryPath, [
'devicectl',
'device',
'info',
'processes',
`--device`, this.udid
]);
// Each line has spaces. devicectl has JSON output option, but it writes it to a file only.
// Here parse the standard output directly.
// e.g.:
// 823 /private/var/containers/Bundle/Application/8E748312-8CBE-4C13-8295-C2EF3ED2C0C1/WebDriverAgentRunner-Runner.app/WebDriverAgentRunner-Runner
// 824 /Applications/MobileSafari.app/MobileSafari
// 825 /usr/libexec/debugserver
const executableLine = stdout.split('\n').find((line) => line.trim().endsWith(executableName));
if (!executableLine) {
log.info(`The process of the bundle id '${bundleId}' was not running`);
return false;
}
await exec(xcrunBinnaryPath, [
'devicectl',
'device',
'process',
'signal',
'-s', `2`,
// e.g.
// '824 /Applications/MobileSafari.app/MobileSafari '
// ' 999 /Applications/MobileSafari.app/MobileSafari ' (can include spaces in the top)
'-p', `${executableLine.trim().split(/\s+/)[0]}`,
`--device`, this.udid
]);
} else {
instrumentService = await services.startInstrumentService(this.udid);
const processes = await instrumentService.callChannel(
INSTRUMENT_CHANNEL.DEVICE_INFO,
'runningProcesses',
);
const process = processes.selector.find((process) => process.name === executableName);
if (!process) {
log.info(`The process of the bundle id '${bundleId}' was not running`);
return false;
}
await instrumentService.callChannel(
INSTRUMENT_CHANNEL.PROCESS_CONTROL,
'killPid:',
`${process.pid}`,
);
}
await instrumentService.callChannel(
INSTRUMENT_CHANNEL.PROCESS_CONTROL,
'killPid:',
`${process.pid}`,
);
return true;
} catch (err) {
log.warn(`Failed to kill '${bundleId}'. Original error: ${err.stderr || err.message}`);
return false;
Expand All @@ -231,6 +273,7 @@ class IOSDeploy {
instrumentService.close();
}
}
return true;
}

/**
Expand Down
16 changes: 16 additions & 0 deletions lib/xcrun.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import {fs} from 'appium/support';

const XCRUN = 'xcrun';

async function requireXcrun() {
try {
return await fs.which(XCRUN);
} catch (e) {
throw new Error(
`${XCRUN} has not been found in PATH. ` +
`Please make sure XCode development tools are installed`,
);
}
}

export {requireXcrun, XCRUN};
Loading