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

add chromedriverPorts cap which allows specifying multiple ports or a range #529

Merged
merged 4 commits into from
Apr 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
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
69 changes: 63 additions & 6 deletions lib/commands/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,16 +271,73 @@ async function setupExistingChromedriver (chromedriver) {
return chromedriver;
}

/**
* Find a free port to have Chromedriver listen on.
*
* @param {array} [portSpec] - Array which is a list of ports. A list item may
* also itself be an array of length 2 specifying a start and end port of
* a range. Some valid port specs:
* - [8000, 8001, 8002]
* - [[8000, 8005]]
* - [8000, [9000, 9100]]
*
* @return {number} A free port
*/
async function getChromedriverPort (portSpec) {
const getPort = B.promisify(PortFinder.getPort, {context: PortFinder});

// if the user didn't give us any specific information about chromedriver
// port ranges, just find any free port
if (!portSpec) {
const port = await getPort();
log.debug(`A port was not given, using random free port: ${port}`);
return port;
}

// otherwise find the free port based on a list or range provided by the user
log.debug(`Finding a free port for chromedriver using spec ${JSON.stringify(portSpec)}`);
let foundPort = null;
for (const potentialPort of portSpec) {
let port, stopPort;
if (_.isArray(potentialPort)) {
([port, stopPort] = potentialPort);
} else {
port = parseInt(potentialPort, 10); // ensure we have a number and not a string
stopPort = port;
}
try {
log.debug(`Checking port range ${port}:${stopPort}`);
foundPort = await getPort({port, stopPort});
break;
} catch (e) {
log.debug(`Nothing in port range ${port}:${stopPort} was available`);
}
}

if (foundPort === null) {
throw new Error(`Could not find a free port for chromedriver using ` +
`chromedriverPorts spec ${JSON.stringify(portSpec)}`);
}

log.debug(`Using free port ${foundPort} for chromedriver`);
return foundPort;
}

helpers.setupNewChromedriver = async function setupNewChromedriver (opts, curDeviceId, adb) {
// if a port wasn't given, pick a random available one
if (!opts.chromeDriverPort) {
const getPort = B.promisify(PortFinder.getPort, {context: PortFinder});
opts.chromeDriverPort = await getPort();
log.debug(`A port was not given, using random port: ${opts.chromeDriverPort}`);
if (opts.chromeDriverPort) {
log.warn(`The 'chromeDriverPort' capability is deprecated. Please use 'chromedriverPort' instead`);
Copy link
Member

Choose a reason for hiding this comment

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

👍

opts.chromedriverPort = opts.chromeDriverPort;
}

if (opts.chromedriverPort) {
log.debug(`Using user-specified port ${opts.chromedriverPort} for chromedriver`);
} else {
// if a single port wasn't given, we'll look for a free one
opts.chromedriverPort = await getChromedriverPort(opts.chromedriverPorts);
}

const chromedriver = new Chromedriver({
port: opts.chromeDriverPort,
port: opts.chromedriverPort,
executable: opts.chromedriverExecutable,
adb,
cmdArgs: opts.chromedriverArgs,
Expand Down
13 changes: 12 additions & 1 deletion lib/desired-caps.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,17 @@ let commonCapConstraints = {
keyPassword: {
isString: true
},
// this one is deprecated
chromeDriverPort: {
isNumber: true
},
// duplicate of above with better spelling
chromedriverPort: {
isNumber: true
},
chromedriverPorts: {
isArray: true
},
chromedriverArgs: {
isObject: true,
},
Expand Down Expand Up @@ -225,7 +233,10 @@ let uiautomatorCapConstraints = {
},
skipLogcatCapture: {
isBoolean: true
}
},
bootstrapPort: {
isNumber: true
},
};

let desiredCapConstraints = {};
Expand Down
4 changes: 2 additions & 2 deletions lib/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ class AndroidDriver extends BaseDriver {
this.onSettingsUpdate.bind(this));
this.chromedriver = null;
this.apkStrings = {};
this.bootstrapPort = opts.bootstrapPort || DEVICE_PORT;
this.unlocker = helpers.unlocker;

for (let [cmd, fn] of _.toPairs(commands)) {
Expand Down Expand Up @@ -89,6 +88,7 @@ class AndroidDriver extends BaseDriver {
fullReset: false,
autoLaunch: true,
adbPort: DEFAULT_ADB_PORT,
bootstrapPort: DEVICE_PORT,
androidInstallTimeout: 90000,
};
_.defaults(this.opts, defaultOpts);
Expand Down Expand Up @@ -264,7 +264,7 @@ class AndroidDriver extends BaseDriver {
}

// start UiAutomator
this.bootstrap = new helpers.bootstrap(this.adb, this.bootstrapPort, this.opts.websocket);
this.bootstrap = new helpers.bootstrap(this.adb, this.opts.bootstrapPort, this.opts.websocket);
await this.bootstrap.start(this.opts.appPackage, this.opts.disableAndroidWatchers, this.opts.acceptSslCerts);
// handling unexpected shutdown
this.bootstrap.onUnexpectedShutdown.catch(async (err) => { // eslint-disable-line promise/prefer-await-to-callbacks
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"dependencies": {
"@babel/runtime": "^7.0.0",
"appium-adb": "^7.6.0",
"appium-base-driver": "^3.0.0",
"appium-base-driver": "^3.15.5",
"appium-chromedriver": "^4.8.0",
"appium-support": "^2.25.0",
"asyncbox": "^2.0.4",
Expand Down