From 2d78337a35ee5fbdb9ba8a1277c9c5ccc92ea076 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Mon, 28 Mar 2022 06:18:20 +0200 Subject: [PATCH] refactor: Use built-in driver logger (#762) --- lib/commands/general.js | 15 +++--- lib/driver.js | 63 ++++++++++++------------ lib/espresso-runner.js | 79 +++++++++++++++--------------- lib/server-builder.js | 20 ++++---- package.json | 2 +- test/unit/espresso-runner-specs.js | 11 +++-- test/unit/server-builder-specs.js | 19 +++---- 7 files changed, 103 insertions(+), 106 deletions(-) diff --git a/lib/commands/general.js b/lib/commands/general.js index f924e7c02..f6951dec5 100644 --- a/lib/commands/general.js +++ b/lib/commands/general.js @@ -1,11 +1,10 @@ import _ from 'lodash'; import { util } from '@appium/support'; -import logger from '../logger'; import validate from 'validate.js'; import { errors } from '@appium/base-driver'; import { qualifyActivityName } from '../utils'; -let commands = {}, helpers = {}, extensions = {}; +const commands = {}, helpers = {}, extensions = {}; function assertRequiredOptions (options, requiredOptionNames) { if (!_.isArray(requiredOptionNames)) { @@ -62,7 +61,7 @@ commands.mobileGetDeviceInfo = async function mobileGetDeviceInfo () { commands.mobileIsToastVisible = async function mobileIsToastVisible (opts = {}) { const {text, isRegexp} = opts; if (!util.hasValue(text)) { - logger.errorAndThrow(`'text' argument is mandatory`); + throw new errors.InvalidArgumentError(`'text' argument is mandatory`); } return await this.espresso.jwproxy.command('/appium/execute_mobile/is_toast_displayed', 'POST', { text, @@ -110,8 +109,7 @@ commands.mobileNavigateTo = async function mobileNavigateTo (opts = {}) { let menuItemIdAsNumber = parseInt(menuItemId, 10); if (_.isNaN(menuItemIdAsNumber) || menuItemIdAsNumber < 0) { - logger.errorAndThrow(`'menuItemId' must be a non-negative number. Found ${menuItemId}`); - menuItemId = menuItemIdAsNumber; + throw new errors.InvalidArgumentError(`'menuItemId' must be a non-negative number. Found ${menuItemId}`); } return await this.espresso.jwproxy.command(`/appium/execute_mobile/${util.unwrapElement(element)}/navigate_to`, 'POST', { @@ -163,15 +161,14 @@ commands.mobileScrollToPage = async function mobileScrollToPage (opts = {}) { }, }, }); - if (util.hasValue(res)) { - logger.errorAndThrow(`Invalid scrollTo parameters: ${JSON.stringify(res)}`); + throw new errors.InvalidArgumentError(`Invalid scrollTo parameters: ${JSON.stringify(res)}`); } const {element, scrollTo, scrollToPage, smoothScroll} = opts; if (util.hasValue(scrollTo) && util.hasValue(scrollToPage)) { - logger.warn(`'scrollTo' and 'scrollToPage' where both provided. Defaulting to 'scrollTo'`); + this.log.warn(`'scrollTo' and 'scrollToPage' where both provided. Defaulting to 'scrollTo'`); } return await this.espresso.jwproxy.command(`/appium/execute_mobile/${util.unwrapElement(element)}/scroll_to_page`, 'POST', { @@ -307,7 +304,7 @@ commands.startActivity = async function startActivity (appPackage, appActivity, appWaitPackage = appWaitPackage || appPackage; appActivity = qualifyActivityName(appActivity, appPackage); appWaitActivity = qualifyActivityName(appWaitActivity || appActivity, appWaitPackage); - logger.debug(`Starting activity '${appActivity}' for package '${appPackage}'`); + this.log.debug(`Starting activity '${appActivity}' for package '${appPackage}'`); await this.espresso.jwproxy.command(`/appium/device/start_activity`, 'POST', { appPackage, appActivity, diff --git a/lib/driver.js b/lib/driver.js index 36f3cf5ba..483fdeb94 100644 --- a/lib/driver.js +++ b/lib/driver.js @@ -3,7 +3,6 @@ import path from 'path'; import { BaseDriver, errors, isErrorType, DeviceSettings} from '@appium/base-driver'; import { EspressoRunner, TEST_APK_PKG } from './espresso-runner'; import { fs, tempDir, zip } from '@appium/support'; -import logger from './logger'; import commands from './commands'; import { DEFAULT_ADB_PORT } from 'appium-adb'; import { androidHelpers, androidCommands, SETTINGS_HELPER_PKG_ID } from 'appium-android-driver'; @@ -180,11 +179,11 @@ class EspressoDriver extends BaseDriver { if (this.isChromeSession) { if (this.opts.app) { - logger.warn(`'browserName' capability will be ignored`); - logger.warn(`Chrome browser cannot be run in Espresso sessions because Espresso automation doesn't ` + + this.log.warn(`'browserName' capability will be ignored`); + this.log.warn(`Chrome browser cannot be run in Espresso sessions because Espresso automation doesn't ` + `have permission to access Chrome`); } else { - logger.errorAndThrow(`Chrome browser sessions cannot be run in Espresso because Espresso ` + + this.log.errorAndThrow(`Chrome browser sessions cannot be run in Espresso because Espresso ` + `automation doesn't have permission to access Chrome`); } } @@ -214,10 +213,10 @@ class EspressoDriver extends BaseDriver { } else if (this.appOnDevice) { // the app isn't an actual app file but rather something we want to // assume is on the device and just launch via the appPackage - logger.info(`App file was not listed, instead we're going to run ` + + this.log.info(`App file was not listed, instead we're going to run ` + `${this.opts.appPackage} directly on the device`); if (!await this.adb.isAppInstalled(this.opts.appPackage)) { - logger.errorAndThrow(`Could not find the package '${this.opts.appPackage}' ` + + this.log.errorAndThrow(`Could not find the package '${this.opts.appPackage}' ` + `installed on the device`); } } @@ -260,18 +259,18 @@ class EspressoDriver extends BaseDriver { })).sort((a, b) => a.split(path.sep).length - b.split(path.sep).length); if (sortedBundleItems.length === 0) { // no expected packages in the zip - logger.errorAndThrow(`${this.opts.app} did not have any of '${SUPPORTED_EXTENSIONS.join(', ')}' ` + + this.log.errorAndThrow(`${this.opts.app} did not have any of '${SUPPORTED_EXTENSIONS.join(', ')}' ` + `extension packages. Please make sure the provided .zip archive contains at least one valid application package.`); } const unzippedAppPath = path.join(tmpRoot, _.first(sortedBundleItems)); - logger.debug(`'${unzippedAppPath}' is the unzipped file from '${appPath}'`); + this.log.debug(`'${unzippedAppPath}' is the unzipped file from '${appPath}'`); return unzippedAppPath; } async onPostConfigureApp ({cachedAppInfo, isUrl, appPath}) { const presignApp = async (appLocation) => { if (this.opts.noSign) { - logger.info('Skipping application signing because noSign capability is set to true. ' + + this.log.info('Skipping application signing because noSign capability is set to true. ' + 'Having the application under test with improper signature/non-signed will cause ' + 'Espresso automation startup failure.'); } else if (!await this.adb.checkApkCert(appLocation, this.opts.appPackage)) { @@ -289,7 +288,7 @@ class EspressoDriver extends BaseDriver { if (_.isPlainObject(cachedAppInfo)) { const packageHash = await fs.hash(appPath); if (packageHash === cachedAppInfo.packageHash && await fs.exists(cachedAppInfo.fullPath)) { - logger.info(`Using '${cachedAppInfo.fullPath}' which is cached from '${appPath}'`); + this.log.info(`Using '${cachedAppInfo.fullPath}' which is cached from '${appPath}'`); isResultAppPathAlreadyCached = true; pathInCache = cachedAppInfo.fullPath; } @@ -346,13 +345,13 @@ class EspressoDriver extends BaseDriver { // TODO this method is duplicated from uiautomator2-driver; consolidate setAvdFromCapabilities (caps) { if (this.opts.avd) { - logger.info('avd name defined, ignoring device name and platform version'); + this.log.info('avd name defined, ignoring device name and platform version'); } else { if (!caps.deviceName) { - logger.errorAndThrow('avd or deviceName should be specified when reboot option is enables'); + this.log.errorAndThrow('avd or deviceName should be specified when reboot option is enables'); } if (!caps.platformVersion) { - logger.errorAndThrow('avd or platformVersion should be specified when reboot option is enabled'); + this.log.errorAndThrow('avd or platformVersion should be specified when reboot option is enabled'); } let avdDevice = caps.deviceName.replace(/[^a-zA-Z0-9_.]/g, '-'); this.opts.avd = `${avdDevice}__${caps.platformVersion}`; @@ -370,11 +369,11 @@ class EspressoDriver extends BaseDriver { // TODO much of this logic is duplicated from uiautomator2 async startEspressoSession () { - logger.info(`EspressoDriver version: ${version}`); + this.log.info(`EspressoDriver version: ${version}`); // Read https://github.com/appium/appium-android-driver/pull/461 what happens if ther is no setHiddenApiPolicy for Android P+ if (await this.adb.getApiLevel() >= 28) { // Android P - logger.warn('Relaxing hidden api policy'); + this.log.warn('Relaxing hidden api policy'); await this.adb.setHiddenApiPolicy('1', !!this.opts.ignoreHiddenApiPolicyError); } @@ -396,7 +395,7 @@ class EspressoDriver extends BaseDriver { await this.adb.setAnimationState(false); this.wasAnimationEnabled = true; } catch (err) { - logger.warn(`Unable to turn off animations: ${err.message}`); + this.log.warn(`Unable to turn off animations: ${err.message}`); } } @@ -407,14 +406,14 @@ class EspressoDriver extends BaseDriver { // set up the modified espresso server etc this.initEspressoServer(); // Further prepare the device by forwarding the espresso port - logger.debug(`Forwarding Espresso Server port ${DEVICE_PORT} to ${this.opts.systemPort}`); + this.log.debug(`Forwarding Espresso Server port ${DEVICE_PORT} to ${this.opts.systemPort}`); await this.adb.forwardPort(this.opts.systemPort, DEVICE_PORT); if (!this.opts.skipUnlock) { // unlock the device to prepare it for testing await helpers.unlock(this, this.adb, this.caps); } else { - logger.debug(`'skipUnlock' capability set, so skipping device unlock`); + this.log.debug(`'skipUnlock' capability set, so skipping device unlock`); } // set up app under test @@ -443,7 +442,7 @@ class EspressoDriver extends BaseDriver { // launch espresso and wait till its online and we have a session await this.espresso.startSession(this.caps); if (this.caps.autoLaunch === false) { - logger.info(`Not waiting for the application activity to start because 'autoLaunch' is disabled`); + this.log.info(`Not waiting for the application activity to start because 'autoLaunch' is disabled`); } else { await this.adb.waitForActivity(this.caps.appWaitPackage, this.caps.appWaitActivity, this.opts.appWaitDuration); } @@ -463,7 +462,7 @@ class EspressoDriver extends BaseDriver { async initWebview () { const viewName = androidCommands.defaultWebviewName.call(this); const timeout = this.opts.autoWebviewTimeout || 2000; - logger.info(`Setting webview to context '${viewName}' with timeout ${timeout}ms`); + this.log.info(`Setting webview to context '${viewName}' with timeout ${timeout}ms`); await retryInterval(timeout / 500, 500, this.setContext.bind(this), viewName); } @@ -487,7 +486,7 @@ class EspressoDriver extends BaseDriver { initEspressoServer () { // now that we have package and activity, we can create an instance of // espresso with the appropriate data - this.espresso = new EspressoRunner({ + this.espresso = new EspressoRunner(this.log, { host: this.opts.remoteAdbHost || this.opts.host || '127.0.0.1', systemPort: this.opts.systemPort, devicePort: DEVICE_PORT, @@ -532,9 +531,9 @@ class EspressoDriver extends BaseDriver { if (!this.opts.app) { if (this.opts.fullReset) { - logger.errorAndThrow('Full reset requires an app capability, use fastReset if app is not provided'); + this.log.errorAndThrow('Full reset requires an app capability, use fastReset if app is not provided'); } - logger.debug('No app capability. Assuming it is already on the device'); + this.log.debug('No app capability. Assuming it is already on the device'); if (this.opts.fastReset) { await helpers.resetApp(this.adb, this.opts); } @@ -547,20 +546,20 @@ class EspressoDriver extends BaseDriver { await helpers.installApk(this.adb, this.opts); } if (this.opts.skipServerInstallation) { - logger.debug('skipServerInstallation capability is set. Not installig espresso-server '); + this.log.debug('skipServerInstallation capability is set. Not installig espresso-server '); } else { await this.espresso.installTestApk(); try { await this.adb.addToDeviceIdleWhitelist(SETTINGS_HELPER_PKG_ID, TEST_APK_PKG); } catch (e) { - logger.warn(`Cannot add server packages to the Doze whitelist. Original error: ` + + this.log.warn(`Cannot add server packages to the Doze whitelist. Original error: ` + (e.stderr || e.message)); } } } async deleteSession () { - logger.debug('Deleting espresso session'); + this.log.debug('Deleting espresso session'); try { if (!_.isEmpty(this._screenRecordingProperties)) { @@ -586,29 +585,29 @@ class EspressoDriver extends BaseDriver { try { await this.adb.setAnimationState(true); } catch (err) { - logger.warn(`Unable to reset animation: ${err.message}`); + this.log.warn(`Unable to reset animation: ${err.message}`); } } if (this.opts.unicodeKeyboard && this.opts.resetKeyboard && this.defaultIME) { - logger.debug(`Resetting IME to '${this.defaultIME}'`); + this.log.debug(`Resetting IME to '${this.defaultIME}'`); await this.adb.setIME(this.defaultIME); } if (!this.isChromeSession && this.opts.appPackage && !this.opts.dontStopAppOnReset) { await this.adb.forceStop(this.opts.appPackage); } if (this.opts.fullReset && !this.opts.skipUninstall && !this.appOnDevice) { - logger.debug(`FULL_RESET set to 'true', Uninstalling '${this.opts.appPackage}'`); + this.log.debug(`FULL_RESET set to 'true', Uninstalling '${this.opts.appPackage}'`); await this.adb.uninstallApk(this.opts.appPackage); } await this.adb.stopLogcat(); if (this.opts.reboot) { let avdName = this.opts.avd.replace('@', ''); - logger.debug(`closing emulator '${avdName}'`); + this.log.debug(`closing emulator '${avdName}'`); await this.adb.killEmulator(avdName); } if (await this.adb.getApiLevel() >= 28) { // Android P - logger.info('Restoring hidden api policy to the device default configuration'); + this.log.info('Restoring hidden api policy to the device default configuration'); await this.adb.setDefaultHiddenApiPolicy(!!this.opts.ignoreHiddenApiPolicyError); } } @@ -617,7 +616,7 @@ class EspressoDriver extends BaseDriver { try { await this.adb.removePortForward(this.opts.systemPort); } catch (error) { - logger.warn(`Unable to remove port forward '${error.message}'`); + this.log.warn(`Unable to remove port forward '${error.message}'`); //Ignore, this block will also be called when we fall in catch block // and before even port forward. } diff --git a/lib/espresso-runner.js b/lib/espresso-runner.js index 5fa10a0b2..2bd0ac0d6 100644 --- a/lib/espresso-runner.js +++ b/lib/espresso-runner.js @@ -1,6 +1,5 @@ import { JWProxy, errors } from '@appium/base-driver'; import { waitForCondition } from 'asyncbox'; -import logger from './logger'; import { ServerBuilder, buildServerSigningConfig } from './server-builder'; import path from 'path'; import { fs, util, mkdirp, timing } from '@appium/support'; @@ -38,14 +37,16 @@ class EspressoProxy extends JWProxy { } class EspressoRunner { - constructor (opts = {}) { + constructor (log, opts = {}) { for (let req of REQUIRED_PARAMS) { if (!opts || !util.hasValue(opts[req])) { throw new Error(`Option '${req}' is required!`); } this[req] = opts[req]; } + this.log = log; this.jwproxy = new EspressoProxy({ + log, server: this.host, port: this.systemPort, base: '', @@ -85,11 +86,11 @@ class EspressoRunner { async isAppPackageChanged () { if (!await this.adb.fileExists(TARGET_PACKAGE_CONTAINER)) { - logger.debug('The previous target application package is unknown'); + this.log.debug('The previous target application package is unknown'); return true; } const previousAppPackage = (await this.adb.shell(['cat', TARGET_PACKAGE_CONTAINER])).trim(); - logger.debug(`The previous target application package was '${previousAppPackage}'. ` + + this.log.debug(`The previous target application package was '${previousAppPackage}'. ` + `The current package is '${this.appPackage}'`); return previousAppPackage !== this.appPackage; } @@ -110,21 +111,21 @@ class EspressoRunner { ].includes(appState); if (shouldUninstallApp) { - logger.info(`Uninstalling Espresso Test Server apk from the target device (pkg: '${TEST_APK_PKG}')`); + this.log.info(`Uninstalling Espresso Test Server apk from the target device (pkg: '${TEST_APK_PKG}')`); try { await this.adb.uninstallApk(TEST_APK_PKG); } catch (err) { - logger.warn(`Error uninstalling '${TEST_APK_PKG}': ${err.message}`); + this.log.warn(`Error uninstalling '${TEST_APK_PKG}': ${err.message}`); } } if (shouldInstallApp) { - logger.info(`Installing Espresso Test Server apk from the target device (path: '${this.modServerPath}')`); + this.log.info(`Installing Espresso Test Server apk from the target device (path: '${this.modServerPath}')`); try { await this.adb.install(this.modServerPath, { replace: false, timeout: this.androidInstallTimeout }); - logger.info(`Installed Espresso Test Server apk '${this.modServerPath}' (pkg: '${TEST_APK_PKG}')`); + this.log.info(`Installed Espresso Test Server apk '${this.modServerPath}' (pkg: '${TEST_APK_PKG}')`); } catch (err) { - logger.errorAndThrow(`Cannot install '${this.modServerPath}' because of '${err.message}'`); + this.log.errorAndThrow(`Cannot install '${this.modServerPath}' because of '${err.message}'`); } } } @@ -132,14 +133,14 @@ class EspressoRunner { async installTestApk () { let rebuild = this.forceEspressoRebuild; if (rebuild) { - logger.debug(`'forceEspressoRebuild' capability is enabled`); + this.log.debug(`'forceEspressoRebuild' capability is enabled`); } else if (await this.isAppPackageChanged()) { - logger.info(`Forcing Espresso server rebuild because of changed application package`); + this.log.info(`Forcing Espresso server rebuild because of changed application package`); rebuild = true; } if (rebuild && await fs.exists(this.modServerPath)) { - logger.debug(`Deleting the obsolete Espresso server package '${this.modServerPath}'`); + this.log.debug(`Deleting the obsolete Espresso server package '${this.modServerPath}'`); await fs.unlink(this.modServerPath); } if (!(await fs.exists(this.modServerPath))) { @@ -150,7 +151,7 @@ class EspressoRunner { await this.adb.sign(this.modServerPath); } if ((rebuild || !isSigned) && await this.adb.uninstallApk(TEST_APK_PKG)) { - logger.info('Uninstalled the obsolete Espresso server package from the device under test'); + this.log.info('Uninstalled the obsolete Espresso server package from the device under test'); } await this.installServer(); @@ -161,16 +162,16 @@ class EspressoRunner { if (this.espressoBuildConfig) { let buildConfigurationStr; if (await fs.exists(this.espressoBuildConfig)) { - logger.info(`Loading the build configuration from '${this.espressoBuildConfig}'`); + this.log.info(`Loading the build configuration from '${this.espressoBuildConfig}'`); buildConfigurationStr = await fs.readFile(this.espressoBuildConfig, 'utf8'); } else { - logger.info(`Loading the build configuration from 'espressoBuildConfig' capability`); + this.log.info(`Loading the build configuration from 'espressoBuildConfig' capability`); buildConfigurationStr = this.espressoBuildConfig; } try { buildConfiguration = JSON.parse(buildConfigurationStr); } catch (e) { - logger.error('Cannot parse the build configuration JSON', e); + this.log.error('Cannot parse the build configuration JSON', e); throw e; } } @@ -178,26 +179,26 @@ class EspressoRunner { replacement: '-', }); const serverPath = path.resolve(this.tmpDir, dirName); - logger.info(`Building espresso server in '${serverPath}'`); - logger.debug(`The build folder root could be customized by changing the 'tmpDir' capability`); + this.log.info(`Building espresso server in '${serverPath}'`); + this.log.debug(`The build folder root could be customized by changing the 'tmpDir' capability`); await fs.rimraf(serverPath); await mkdirp(serverPath); - logger.debug(`Copying espresso server template from ('${TEST_SERVER_ROOT}' to '${serverPath}')`); + this.log.debug(`Copying espresso server template from ('${TEST_SERVER_ROOT}' to '${serverPath}')`); await copyGradleProjectRecursively(TEST_SERVER_ROOT, serverPath); - logger.debug('Bulding espresso server'); - await new ServerBuilder({ + this.log.debug('Bulding espresso server'); + await new ServerBuilder(this.log, { serverPath, buildConfiguration, showGradleLog: this.showGradleLog, testAppPackage: this.appPackage, signingConfig: this.signingConfig }).build(); const apkPath = path.resolve(serverPath, 'app', 'build', 'outputs', 'apk', 'androidTest', 'debug', 'app-debug-androidTest.apk'); - logger.debug(`Copying built apk from '${apkPath}' to '${this.modServerPath}'`); + this.log.debug(`Copying built apk from '${apkPath}' to '${this.modServerPath}'`); await fs.copyFile(apkPath, this.modServerPath); } async cleanupSessionLeftovers () { - logger.debug('Performing cleanup of automation leftovers'); + this.log.debug('Performing cleanup of automation leftovers'); try { const {value} = (await axios({ @@ -206,8 +207,8 @@ class EspressoRunner { })).data; const activeSessionIds = value.map((sess) => sess.id); if (activeSessionIds.length) { - logger.debug(`The following obsolete sessions are still running: ${JSON.stringify(activeSessionIds)}`); - logger.debug('Cleaning up the obsolete sessions'); + this.log.debug(`The following obsolete sessions are still running: ${JSON.stringify(activeSessionIds)}`); + this.log.debug('Cleaning up the obsolete sessions'); await B.all(activeSessionIds.map((id) => axios({ url: `http://${this.host}:${this.systemPort}/session/${id}`, @@ -217,10 +218,10 @@ class EspressoRunner { // Let all sessions to be properly terminated before continuing await B.delay(1000); } else { - logger.debug('No obsolete sessions have been detected'); + this.log.debug('No obsolete sessions have been detected'); } } catch (e) { - logger.debug(`No obsolete sessions have been detected (${e.message})`); + this.log.debug(`No obsolete sessions have been detected (${e.message})`); } } @@ -241,7 +242,7 @@ class EspressoRunner { cmd.push(`${TEST_APK_PKG}/androidx.test.runner.AndroidJUnitRunner`); - logger.info(`Starting Espresso Server v${version} with cmd: adb ${cmd.join(' ')}`); + this.log.info(`Starting Espresso Server v${version} with cmd: adb ${cmd.join(' ')}`); let hasSocketError = false; // start the instrumentation process @@ -251,7 +252,7 @@ class EspressoRunner { }; this.instProcess = this.adb.createSubProcess(cmd); this.instProcess.on('exit', (code, signal) => { - logger.info(`Instrumentation process exited with code ${code} from signal ${signal}`); + this.log.info(`Instrumentation process exited with code ${code} from signal ${signal}`); this.jwproxy.instrumentationState.exited = true; }); this.instProcess.on('output', (stdout, stderr) => { @@ -261,7 +262,7 @@ class EspressoRunner { return; } - logger.debug(`[Instrumentation] ${line.trim()}`); + this.log.debug(`[Instrumentation] ${line.trim()}`); // A 'SocketException' indicates that we couldn't connect to the Espresso server, // because the INTERNET permission is not set if (line.toLowerCase().includes('java.net.socketexception')) { @@ -273,15 +274,15 @@ class EspressoRunner { const timer = new timing.Timer().start(); await this.instProcess.start(0); - logger.info(`Waiting up to ${this.serverLaunchTimeout}ms for Espresso server to be online`); + this.log.info(`Waiting up to ${this.serverLaunchTimeout}ms for Espresso server to be online`); try { await waitForCondition(async () => { if (hasSocketError) { - logger.errorAndThrow(`Espresso server has failed to start due to an unexpected exception. ` + + this.log.errorAndThrow(`Espresso server has failed to start due to an unexpected exception. ` + `Make sure the 'INTERNET' permission is requested in the Android manifest of your ` + `application under test ()`); } else if (this.jwproxy.instrumentationState.exited) { - logger.errorAndThrow(`Espresso server process has been unexpectedly terminated. ` + + this.log.errorAndThrow(`Espresso server process has been unexpectedly terminated. ` + `Check the Appium server log and the logcat output for more details`); } try { @@ -296,15 +297,15 @@ class EspressoRunner { }); } catch (e) { if (/Condition unmet/.test(e.message)) { - logger.errorAndThrow(`Timed out waiting for Espresso server to be ` + + this.log.errorAndThrow(`Timed out waiting for Espresso server to be ` + `online within ${this.serverLaunchTimeout}ms. The timeout value could be ` + `customized using 'espressoServerLaunchTimeout' capability`); } throw e; } - logger.info(`Espresso server is online. ` + + this.log.info(`Espresso server is online. ` + `The initialization process took ${timer.getDuration().asMilliSeconds.toFixed(0)}ms`); - logger.info('Starting the session'); + this.log.info('Starting the session'); await this.jwproxy.command('/session', 'POST', { capabilities: { @@ -317,17 +318,17 @@ class EspressoRunner { async recordTargetAppPackage () { await this.adb.shell([`echo "${this.appPackage}" > "${TARGET_PACKAGE_CONTAINER}"`]); - logger.info(`Recorded the target application package '${this.appPackage}' to ${TARGET_PACKAGE_CONTAINER}`); + this.log.info(`Recorded the target application package '${this.appPackage}' to ${TARGET_PACKAGE_CONTAINER}`); } async deleteSession () { - logger.debug('Deleting Espresso server session'); + this.log.debug('Deleting Espresso server session'); // rely on jwproxy's intelligence to know what we're talking about and // delete the current session try { await this.jwproxy.command('/', 'DELETE'); } catch (err) { - logger.warn(`Did not get confirmation Espresso deleteSession worked; ` + + this.log.warn(`Did not get confirmation Espresso deleteSession worked; ` + `Error was: ${err}`); } diff --git a/lib/server-builder.js b/lib/server-builder.js index 8a5ee09f2..26340c1e8 100644 --- a/lib/server-builder.js +++ b/lib/server-builder.js @@ -1,7 +1,6 @@ import { SubProcess } from 'teen_process'; -import { fs, logger, system } from '@appium/support'; +import { fs, system } from '@appium/support'; import _ from 'lodash'; -import log from './logger'; import path from 'path'; import { EOL } from 'os'; import { updateDependencyLines } from './utils'; @@ -26,8 +25,6 @@ const VERSION_KEYS = [ 'composeVersion' ]; -const gradleLog = logger.getLogger('Gradle'); - function buildServerSigningConfig (args) { return { zipAlign: true, @@ -39,7 +36,8 @@ function buildServerSigningConfig (args) { } class ServerBuilder { - constructor (args = {}) { + constructor (log, args = {}) { + this.log = log; this.serverPath = args.serverPath; this.showGradleLog = args.showGradleLog; @@ -151,7 +149,7 @@ class ServerBuilder { continue; } - log.info(`Adding the following ${propName} to build.gradle.kts: ${deps}`); + this.log.info(`Adding the following ${propName} to build.gradle.kts: ${deps}`); configuration = updateDependencyLines(configuration, propName, deps); } await fs.writeFile(buildPath, configuration, 'utf8'); @@ -159,7 +157,7 @@ class ServerBuilder { async runBuildProcess () { const {cmd, args} = this.getCommand(); - log.debug(`Beginning build with command '${cmd} ${args.join(' ')}' ` + + this.log.debug(`Beginning build with command '${cmd} ${args.join(' ')}' ` + `in directory '${this.serverPath}'`); const gradlebuild = new SubProcess(cmd, args, { cwd: this.serverPath, @@ -169,13 +167,13 @@ class ServerBuilder { let buildLastLines = []; const logMsg = `Output from Gradle ${this.showGradleLog ? 'will' : 'will not'} be logged`; - log.debug(`${logMsg}. To change this, use 'showGradleLog' desired capability`); + this.log.debug(`${logMsg}. To change this, use 'showGradleLog' desired capability`); gradlebuild.on('stream-line', (line) => { if (this.showGradleLog) { if (line.startsWith('[STDERR]')) { - gradleLog.warn(line); + this.log.warn(`[Gradle] ${line}`); } else { - gradleLog.info(line); + this.log.info(`[Gradle] ${line}`); } } buildLastLines.push(`${EOL}${line}`); @@ -190,7 +188,7 @@ class ServerBuilder { } catch (err) { let msg = `Unable to build Espresso server - ${err.message}\n` + `Gradle error message:${EOL}${buildLastLines}`; - log.errorAndThrow(msg); + this.log.errorAndThrow(msg); } } } diff --git a/package.json b/package.json index 43bbb544d..1c8d0fc33 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,7 @@ "!.DS_Store" ], "dependencies": { - "@appium/base-driver": "^8.2.4", + "@appium/base-driver": "^8.3.0", "@appium/support": "^2.55.3", "@babel/runtime": "^7.4.3", "appium-adb": "^9.1.0", diff --git a/test/unit/espresso-runner-specs.js b/test/unit/espresso-runner-specs.js index 3c0884672..643f757ea 100644 --- a/test/unit/espresso-runner-specs.js +++ b/test/unit/espresso-runner-specs.js @@ -3,6 +3,7 @@ import chaiAsPromised from 'chai-as-promised'; import { EspressoRunner, REQUIRED_PARAMS } from '../../lib/espresso-runner'; import { ADB } from 'appium-adb'; import sinon from 'sinon'; +import log from '../../lib/logger'; chai.should(); chai.use(chaiAsPromised); @@ -21,7 +22,7 @@ describe('espresso-runner', function () { function runConstructorTest (opts, missingParam) { it(`should error out if missing '${missingParam}' parameter`, function () { expect(function () { - new EspressoRunner(opts); + new EspressoRunner(log, opts); }).to.throw(`Option '${missingParam}' is required!`); }); } @@ -69,7 +70,7 @@ describe('espresso-runner', function () { }); const adb = ADB.createADB(); - const espresso = new EspressoRunner({ + const espresso = new EspressoRunner(log, { adb, tmpDir: 'tmp', host: 'localhost', systemPort: 4724, devicePort: 6790, appPackage: 'io.appium.example', forceEspressoRebuild: false @@ -93,7 +94,7 @@ describe('espresso-runner', function () { }); const adb = ADB.createADB(); - const espresso = new EspressoRunner({ + const espresso = new EspressoRunner(log, { adb, tmpDir: 'tmp', host: 'localhost', systemPort: 4724, devicePort: 6790, appPackage: 'io.appium.example', forceEspressoRebuild: false @@ -117,7 +118,7 @@ describe('espresso-runner', function () { }); const adb = ADB.createADB(); - const espresso = new EspressoRunner({ + const espresso = new EspressoRunner(log, { adb, tmpDir: 'tmp', host: 'localhost', systemPort: 4724, devicePort: 6790, appPackage: 'io.appium.example', forceEspressoRebuild: false @@ -144,7 +145,7 @@ describe('espresso-runner', function () { }); const adb = ADB.createADB(); - const espresso = new EspressoRunner({ + const espresso = new EspressoRunner(log, { adb, tmpDir: 'tmp', host: 'localhost', systemPort: 4724, devicePort: 6790, appPackage: 'io.appium.example', forceEspressoRebuild: false diff --git a/test/unit/server-builder-specs.js b/test/unit/server-builder-specs.js index c2636465c..7ef5bb5fa 100644 --- a/test/unit/server-builder-specs.js +++ b/test/unit/server-builder-specs.js @@ -5,6 +5,7 @@ import { GRADLE_URL_TEMPLATE, ServerBuilder, VERSION_KEYS } from '../../lib/server-builder'; import { updateDependencyLines } from '../../lib/utils'; +import log from '../../lib/logger'; chai.should(); chai.use(chaiAsPromised); @@ -15,12 +16,12 @@ describe('server-builder', function () { it('should not pass properties when no versions are specified', function () { const expected = {cmd: expectedCmd, args: ['app:assembleAndroidTest']}; - new ServerBuilder().getCommand().should.eql(expected); + new ServerBuilder(log).getCommand().should.eql(expected); }); it('should pass only specified versions as properties and pass them correctly', function () { const expected = {cmd: expectedCmd, args: ['-PappiumAndroidGradlePlugin=1.2.3', 'app:assembleAndroidTest']}; - let serverBuilder = new ServerBuilder({ + let serverBuilder = new ServerBuilder(log, { buildConfiguration: { toolsVersions: { androidGradlePlugin: '1.2.3' @@ -35,7 +36,7 @@ describe('server-builder', function () { VERSION_KEYS.should.not.contain(unknownKey); const expected = {cmd: expectedCmd, args: ['app:assembleAndroidTest']}; - const serverBuilder = new ServerBuilder({ + const serverBuilder = new ServerBuilder(log, { buildConfiguration: { toolsVersions: { [unknownKey]: '1.2.3' @@ -47,7 +48,7 @@ describe('server-builder', function () { it('should not pass gradle_version as property', function () { const expected = {cmd: expectedCmd, args: ['app:assembleAndroidTest']}; - const serverBuilder = new ServerBuilder({ + const serverBuilder = new ServerBuilder(log, { buildConfiguration: { toolsVersions: { gradle_version: '1.2.3' @@ -62,7 +63,7 @@ describe('server-builder', function () { const serverPath = 'server'; it('should set correct URL in gradle.properties', function () { const readFileResult = 'foo=1\ndistributionUrl=abc\nbar=2'; - let serverBuilder = new ServerBuilder({serverPath}); + let serverBuilder = new ServerBuilder(log, {serverPath}); let actualFileContent = serverBuilder.updateGradleDistUrl(readFileResult, '1.2.3'); actualFileContent.should.eql( @@ -73,7 +74,7 @@ describe('server-builder', function () { it('should keep other lines not affected', function () { const readFileResult = 'foo=1\ndistributionUrl=abc\nbar=2'; - let serverBuilder = new ServerBuilder({serverPath}); + let serverBuilder = new ServerBuilder(log, {serverPath}); let actualFileContent = serverBuilder.updateGradleDistUrl(readFileResult, '1.2.3'); actualFileContent.should.match(/^foo=1$/m); @@ -141,7 +142,7 @@ describe('server-builder', function () { }); it('should throw on single quotes in additional dependencies', async function () { - let serverBuilder = new ServerBuilder({serverPath}); + let serverBuilder = new ServerBuilder(log, {serverPath}); serverBuilder.additionalAppDependencies = ['foo.\':1.2.3']; await serverBuilder.insertAdditionalDependencies().should.be.eventually.rejectedWith( @@ -149,7 +150,7 @@ describe('server-builder', function () { }); it('should throw on dollar characters in additional dependencies', async function () { - let serverBuilder = new ServerBuilder({serverPath}); + let serverBuilder = new ServerBuilder(log, {serverPath}); serverBuilder.additionalAndroidTestDependencies = ['foo.\':1.2.3']; await serverBuilder.insertAdditionalDependencies().should.be.eventually.rejectedWith( @@ -157,7 +158,7 @@ describe('server-builder', function () { }); it('should throw on new lines in additional dependencies', async function () { - let serverBuilder = new ServerBuilder({serverPath}); + let serverBuilder = new ServerBuilder(log, {serverPath}); serverBuilder.additionalAppDependencies = ['foo.\n:1.2.3']; await serverBuilder.insertAdditionalDependencies().should.be.eventually.rejectedWith(