From ed4c9cfa55c988b69140c08dcec01edde7b6b371 Mon Sep 17 00:00:00 2001 From: Sai Krishna Date: Tue, 21 Jan 2025 14:28:59 +0530 Subject: [PATCH] feat: create port forward when mjpegServerPort is provided in caps (#2517) --- lib/commands/recordscreen.js | 22 ++------------ lib/driver.js | 58 +++++++++++++++++++++++++++++++----- test/unit/driver-specs.js | 30 ++++++++++++++++++- 3 files changed, 82 insertions(+), 28 deletions(-) diff --git a/lib/commands/recordscreen.js b/lib/commands/recordscreen.js index b26371515..4b0e15b73 100644 --- a/lib/commands/recordscreen.js +++ b/lib/commands/recordscreen.js @@ -1,8 +1,7 @@ import _ from 'lodash'; import {fs, tempDir, logger, util} from 'appium/support'; import {SubProcess} from 'teen_process'; -import {encodeBase64OrUpload, isLocalHost} from '../utils'; -import {DEVICE_CONNECTIONS_FACTORY} from '../device-connections-factory'; +import {encodeBase64OrUpload} from '../utils'; import {WDA_BASE_URL} from 'appium-webdriveragent'; import {waitForCondition} from 'asyncbox'; import url from 'url'; @@ -14,9 +13,9 @@ import url from 'url'; */ const MAX_RECORDING_TIME_SEC = 4200; const DEFAULT_RECORDING_TIME_SEC = 60 * 3; -const DEFAULT_MJPEG_SERVER_PORT = 9100; const DEFAULT_FPS = 10; const DEFAULT_QUALITY = 'medium'; +const DEFAULT_MJPEG_SERVER_PORT = 9100; const DEFAULT_VCODEC = 'mjpeg'; const MP4_EXT = '.mp4'; const FFMPEG_BINARY = 'ffmpeg'; @@ -52,7 +51,6 @@ export class ScreenRecorder { const { remotePort, remoteUrl, - usePortForwarding, videoFps, videoType, videoScale, @@ -60,19 +58,6 @@ export class ScreenRecorder { pixelFormat, } = this.opts; - try { - await DEVICE_CONNECTIONS_FACTORY.requestConnection(this.udid, remotePort, { - devicePort: remotePort, - usePortForwarding, - }); - } catch { - this.log.warn( - `Cannot forward the local port ${remotePort} to ${remotePort} ` + - `on the device ${this.udid}. Set the custom value to 'mjpegServerPort' ` + - `capability if this is an undesired behavior.`, - ); - } - const args = [ '-f', 'mjpeg', @@ -167,8 +152,6 @@ export class ScreenRecorder { } } - DEVICE_CONNECTIONS_FACTORY.releaseConnection(this.udid, this.opts.remotePort); - return result; } @@ -232,7 +215,6 @@ export default { const screenRecorder = new ScreenRecorder(this.device.udid, this.log, videoPath, { remotePort: this.opts.mjpegServerPort || DEFAULT_MJPEG_SERVER_PORT, remoteUrl: wdaBaseUrl, - usePortForwarding: this.isRealDevice() && isLocalHost(wdaBaseUrl), videoType, videoFilters, videoScale, diff --git a/lib/driver.js b/lib/driver.js index 49a602b83..67d68d72c 100644 --- a/lib/driver.js +++ b/lib/driver.js @@ -103,6 +103,8 @@ const DEFAULT_SETTINGS = { const SHARED_RESOURCES_GUARD = new AsyncLock(); const WEB_ELEMENTS_CACHE_SIZE = 500; const SUPPORTED_ORIENATIONS = ['LANDSCAPE', 'PORTRAIT']; +const DEFAULT_MJPEG_SERVER_PORT = 9100; + /* eslint-disable no-useless-escape */ /** @type {import('@appium/types').RouteMatcher[]} */ const NO_PROXY_NATIVE_LIST = [ @@ -467,12 +469,8 @@ export class XCUITestDriver extends BaseDriver { // ensure WDA gets our defaults instead of whatever its own might be await this.updateSettings(wdaSettings); - // turn on mjpeg stream reading if requested - if (this.opts.mjpegScreenshotUrl) { - this.log.info(`Starting MJPEG stream reading URL: '${this.opts.mjpegScreenshotUrl}'`); - this.mjpegStream = new mjpeg.MJpegStream(this.opts.mjpegScreenshotUrl); - await this.mjpegStream.start(); - } + await this.handleMjpegOptions(); + return /** @type {[string, import('@appium/types').DriverCaps]} */ ([ sessionId, caps, @@ -484,6 +482,53 @@ export class XCUITestDriver extends BaseDriver { } } + /** + * Handles MJPEG server-related capabilities + * @returns {Promise} + */ + async handleMjpegOptions() { + await this.allocateMjpegServerPort(); + // turn on mjpeg stream reading if requested + if (this.opts.mjpegScreenshotUrl) { + this.log.info(`Starting MJPEG stream reading URL: '${this.opts.mjpegScreenshotUrl}'`); + this.mjpegStream = new mjpeg.MJpegStream(this.opts.mjpegScreenshotUrl); + await this.mjpegStream.start(); + } + } + + /** + * Allocates and configures port forwarding for the MJPEG server + * @returns {Promise} + * @throws {Error} If port forwarding fails and mjpegServerPort capability value is provided explicitly + */ + async allocateMjpegServerPort() { + const mjpegServerPort = this.opts.mjpegServerPort || DEFAULT_MJPEG_SERVER_PORT; + this.log.debug( + `Forwarding MJPEG server port ${mjpegServerPort} to local port ${mjpegServerPort}`, + ); + try { + await DEVICE_CONNECTIONS_FACTORY.requestConnection(this.opts.udid, mjpegServerPort, { + devicePort: mjpegServerPort, + usePortForwarding: this.isRealDevice(), + }); + } catch (error) { + if (_.isUndefined(this.opts.mjpegServerPort)) { + this.log.warn( + `Cannot forward the device port ${DEFAULT_MJPEG_SERVER_PORT} to the local port ${DEFAULT_MJPEG_SERVER_PORT}. ` + + `Certain features, like MJPEG-based screen recording, will be unavailable during this session. ` + + `Try to customize the value of 'mjpegServerPort' capability as a possible solution`, + ); + } else { + this.log.debug(error.stack); + throw new Error( + `Cannot ensure MJPEG broadcast functionality by forwarding the local port ${mjpegServerPort} ` + + `requested by the 'mjpegServerPort' capability to the device port ${mjpegServerPort}. ` + + `Original error: ${error}`, + ); + } + } + } + /** * Returns the default URL for Safari browser * @returns {string} The default URL @@ -1025,7 +1070,6 @@ export class XCUITestDriver extends BaseDriver { await this.wda.quit(); } } - DEVICE_CONNECTIONS_FACTORY.releaseConnection(this.opts.udid); } diff --git a/test/unit/driver-specs.js b/test/unit/driver-specs.js index c8bb249a3..f13b17a22 100644 --- a/test/unit/driver-specs.js +++ b/test/unit/driver-specs.js @@ -7,7 +7,7 @@ import {XCUITestDriver} from '../../lib/driver'; import * as utils from '../../lib/utils'; import {MOCHA_LONG_TIMEOUT} from './helpers'; import {RealDevice} from '../../lib/real-device'; - +import net from 'node:net'; const caps = { fistMatch: [{}], @@ -104,6 +104,9 @@ describe('XCUITestDriver', function () { let device; let realDevice; + afterEach(async function () { + await driver.deleteSession(); + }); beforeEach(function () { driver = new XCUITestDriver(); device = { @@ -241,6 +244,31 @@ describe('XCUITestDriver', function () { ); spy.notCalled.should.be.true; }); + + it('should throw an error if mjpegServerPort is occupied', async function () { + this.timeout(MOCHA_LONG_TIMEOUT); + delete device.simctl; + device.devicectl = true; + const server = net.createServer(); + await new Promise((resolve, reject) => { + server.listen(9100, resolve); + server.on('error', reject); + }); + try { + await driver.createSession( + null, + null, + _.merge({}, caps, { + alwaysMatch: {'appium:mjpegServerPort': 9100}, + }), + ).should.be.rejectedWith(/mjpegServerPort.*port #9100 is occupied/); + } finally { + await new Promise((resolve, reject) => { + server.close(resolve); + server.on('error', reject); + }); + } + }); }); describe('execute', function () {