Skip to content

Commit

Permalink
chore: Only use portscanner for port-related operations (#865)
Browse files Browse the repository at this point in the history
  • Loading branch information
mykola-mokhnach authored Oct 16, 2023
1 parent d1f6c97 commit 73eb574
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 17 deletions.
34 changes: 25 additions & 9 deletions lib/commands/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
import {util} from '@appium/support';
import Chromedriver from 'appium-chromedriver';
import {errors} from 'appium/driver';
import B from 'bluebird';
import _ from 'lodash';
import PortFinder from 'portfinder';
import {
APP_STATE,
CHROMIUM_WIN,
Expand All @@ -16,9 +14,31 @@ import {
WebviewHelpers,
} from '../helpers';
import {mixin} from './mixins';
import net from 'node:net';
import {findAPortNotInUse} from 'portscanner';

const CHROMEDRIVER_AUTODOWNLOAD_FEATURE = 'chromedriver_autodownload';

/**
* @returns {Promise<number>}
*/
async function getFreePort() {
return await new Promise((resolve, reject) => {
const srv = net.createServer();
srv.listen(0, () => {
const address = srv.address();
let port;
if (_.has(address, 'port')) {
// @ts-ignore The above condition covers possible errors
port = address.port;
} else {
reject(new Error('Cannot determine any free port number'));
}
srv.close(() => resolve(port));
});
});
}

/**
* @type {import('./mixins').ContextMixin & ThisType<import('../driver').AndroidDriver>}
* @satisfies {import('@appium/types').ExternalDriver}
Expand Down Expand Up @@ -362,14 +382,10 @@ const ContextMixin = {
},

async getChromedriverPort(portSpec, log) {
const getPort = /** @type {(opts?: import('portfinder').PortFinderOptions) => B<number>} */ (
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();
const port = await getFreePort();
log?.debug(`A port was not given, using random free port: ${port}`);
return port;
}
Expand All @@ -388,9 +404,9 @@ const ContextMixin = {
port = parseInt(String(potentialPort), 10); // ensure we have a number and not a string
stopPort = port;
}
log?.debug(`Checking port range ${port}:${stopPort}`);
try {
log?.debug(`Checking port range ${port}:${stopPort}`);
foundPort = await getPort({port, stopPort});
foundPort = await findAPortNotInUse(port, stopPort);
break;
} catch (e) {
log?.debug(`Nothing in port range ${port}:${stopPort} was available`);
Expand Down
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@
"lru-cache": "^10.0.1",
"moment": "^2.24.0",
"moment-timezone": "^0.5.26",
"portfinder": "^1.0.6",
"portscanner": "2.2.0",
"portscanner": "^2.2.0",
"semver": "^7.0.0",
"source-map-support": "^0.x",
"teen_process": "^2.0.0",
Expand Down
6 changes: 0 additions & 6 deletions test/unit/commands/context-specs.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
} from '../../../lib/helpers/webview';
import {AndroidDriver} from '../../../lib/driver';
import Chromedriver from 'appium-chromedriver';
import PortFinder from 'portfinder';
import {errors} from 'appium/driver';

let driver;
Expand All @@ -22,10 +21,6 @@ chai.use(chaiAsPromised);

describe('Context', function () {
beforeEach(function () {
sandbox.stub(PortFinder, 'getPort').callsFake(function (cb) {
// eslint-disable-line promise/prefer-await-to-callbacks
return cb(null, 4444); // eslint-disable-line promise/prefer-await-to-callbacks
});
driver = new AndroidDriver();
driver.adb = sandbox.stub();
driver.adb.curDeviceId = 'device_id';
Expand Down Expand Up @@ -173,7 +168,6 @@ describe('Context', function () {
driver.chromedriver.start
.getCall(0)
.args[0].chromeOptions.androidDeviceSerial.should.be.equal('device_id');
driver.chromedriver.proxyPort.should.be.equal('4444');
driver.chromedriver.proxyReq.bind.calledWithExactly(driver.chromedriver);
driver.proxyReqRes.should.be.equal('proxy');
driver.jwpProxyActive.should.be.true;
Expand Down

0 comments on commit 73eb574

Please sign in to comment.