Skip to content
This repository has been archived by the owner on Oct 25, 2023. It is now read-only.

feat: Allow to configure host/port/app path for Appium for mac app in appium-mac-driver #42

Merged
merged 7 commits into from
Jan 19, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Allow to specify the a4mac app path
  • Loading branch information
KazuCocoa committed Jan 7, 2020
commit 2ed278e107d629d325c044dd8237cba5f5199524
9 changes: 5 additions & 4 deletions lib/appium-for-mac.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@ class AppiumForMac {
constructor (opts = {}) {
this.proxyHost = opts.a4mHost || DEFAULT_A4M_HOST;
this.proxyPort = opts.a4mPort || DEFAULT_A4M_PORT;
this.a4mAppPath = opts.a4mAppPath || REQ_A4M_APP_PATH;
this.proc = null;
this.jwproxy = new JWProxy({server: this.proxyHost, port: this.proxyPort});
}

async start () {
if (!await fs.exists(REQ_A4M_APP_PATH)) {
throw new Error('Could not verify AppiumForMacDriver install; please install to your /Applications folder');
if (!await fs.exists(this.a4mAppPath)) {
throw new Error(`Could not verify AppiumForMacDriver install in ${this.a4mAppPath}; please install it to ${this.a4mAppPath}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

it is a good practice to put variable values in quotes, so people could easily extract them if they contain spaces

Copy link
Contributor

Choose a reason for hiding this comment

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

same below

}

const startDetector = (stdout, stderr) => {
Expand All @@ -33,7 +34,7 @@ class AppiumForMac {
await this.killAll();

// set up our subprocess object
const a4mBinary = path.resolve(REQ_A4M_APP_PATH, 'Contents', 'MacOS', 'AppiumForMac');
const a4mBinary = path.resolve(this.a4mAppPath, 'Contents', 'MacOS', 'AppiumForMac');
this.proc = new SubProcess(a4mBinary, []);
processIsAlive = true;

Expand All @@ -55,7 +56,7 @@ class AppiumForMac {
`signal ${signal}`;
log.error(msg);
});
log.info(`Spawning AppiumForMac with: ${this.appium4macdriver}`);
Copy link
Member Author

Choose a reason for hiding this comment

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

Anyone did not use this. appium4macdriver. So, the value was always undefined.

Copy link

@sankalphirke sankalphirke Jan 13, 2020

Choose a reason for hiding this comment

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

Why where we spawning? And is there a way we can close the app after test, i mean another capability to kill /close the app on OS/machine?

Copy link
Member Author

Choose a reason for hiding this comment

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

Handled the process by https://github.com/appium/node-teen_process/blob/56a139cbf86b819398d7736a442b6c4a985bc716/lib/subprocess.js#L23

The process will stop in delete the session in general.
https://github.com/appium/appium-mac-driver/blob/master/lib/driver.js#L58
So, the kill all process aims to make a new process run safer.

log.info(`Spawning AppiumForMac with: ${this.a4mAppPath}`);

// start subproc and wait for startDetector
await this.proc.start(startDetector);
Expand Down
5 changes: 4 additions & 1 deletion lib/desired-caps.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const desiredCapConstraints = {
platformName: {
platformName: { // override
presence: true,
isString: true,
inclusionCaseInsensitive: ['Mac']
Expand Down Expand Up @@ -38,6 +38,9 @@ const desiredCapConstraints = {
a4mPort: {
isNumber: true
},
a4mAppPath: {
isString: true
}
};

export default desiredCapConstraints;