refactor: address various code issues and move to ora for cli spinner
MODIFY
.travis.yml
.travis.yml
@@ -19,14 +19,14 @@ jobs:
name: node 10
before_script:
- npm outdated || true
- - node . driver install --source=npm appium-fake-driver@0.8.0
+ - node . driver install --source=npm appium-fake-driver
script: npm run test && npm run e2e-test
after_script: npm run coverage
- stage:
name: node 12
before_script:
- npm outdated || true
- - node . driver install --source=npm appium-fake-driver@0.8.0
+ - node . driver install --source=npm appium-fake-driver
script: npm run test && npm run e2e-test
node_js: "12"
- stage:

MODIFY
lib/cli-utils.js
lib/cli-utils.js
/* eslint-disable no-console */
+import ora from 'ora';
+
const JSON_SPACES = 4;
/***
@@ -25,50 +27,6 @@ function log (json, msg) {
!json && console.log(msg);
}
-/**
- * Start a CLI spinner
- * @param {string} msg - what to label the spinner with
- */
-function startSpinner (msg) {
- process.stdout.write(msg);
-
- let count = 1;
- const numDots = 5;
- const dotTime = 500;
- const cursorStart = msg.length;
-
- const interval = setInterval(() => {
- if (count % (numDots + 1) === 0) {
- process.stdout.cursorTo(cursorStart);
- for (let i = 0; i < numDots; i++) {
- process.stdout.write(' ');
- }
- process.stdout.cursorTo(cursorStart);
- } else {
- process.stdout.write('.');
- }
- count++;
- }, dotTime);
-
- return () => {
- clearInterval(interval);
- process.stdout.write('\n');
- };
-}
-
-/**
- * Conditionally start a spinner
- * @param {boolean} json - whether we are in json mode (and should therefore not log)
- * @param {string} msg - string to log
- */
-function logSpin (json, msg) {
- if (json) {
- return () => {};
- }
-
- return startSpinner(msg);
-}
-
/**
* Start a spinner, execute an async function, and then stop the spinner
* @param {boolean} json - whether we are in json mode (and should therefore not log)
@@ -76,21 +34,24 @@ function logSpin (json, msg) {
* @param {function} fn - function to wrap with spinning
*/
async function spinWith (json, msg, fn) {
- const stopSpinner = logSpin(json, msg);
+ if (json) {
+ return await fn();
+ }
+ const spinner = ora(msg).start();
let res;
try {
res = await fn();
- } finally {
- stopSpinner();
- }
+ spinner.succeed();
return res;
+ } catch (err) {
+ spinner.fail();
+ throw err;
+ }
}
export {
errAndQuit,
log,
- startSpinner,
spinWith,
- logSpin,
JSON_SPACES
};

MODIFY
lib/driver-cli.js
lib/driver-cli.js
+/* eslint-disable no-console */
+
import _ from 'lodash';
import path from 'path';
import { KNOWN_DRIVERS } from './drivers';
@@ -7,6 +9,15 @@ import NPM from './npm';
import { errAndQuit, log, spinWith, JSON_SPACES } from './cli-utils';
import { util, fs } from 'appium-support';
+const REQ_DRIVER_FIELDS = ['driverName', 'automationName', 'platformNames', 'mainClass'];
+
+/**
+ * Run a subcommand of the 'appium driver' type. Each subcommand has its own set of arguments which
+ * can be represented as a JS object.
+ *
+ * @param [object] args - JS object where the key is the parameter name (as defined in
+ * driver-parser.js)
+ */
async function runDriverCommand (args) {
// TODO driver config file should be locked while any of these commands are
// running to prevent weird situations
@@ -23,7 +34,7 @@ async function runDriverCommand (args) {
}
if (json) {
- console.log(JSON.stringify(jsonResult, null, JSON_SPACES)); // eslint-disable-line no-console
+ console.log(JSON.stringify(jsonResult, null, JSON_SPACES));
}
}
@@ -31,12 +42,12 @@ class DriverCommand {
constructor ({config, json}) {
this.config = config;
- this.json = json;
+ this.isJsonOutput = json;
this.npm = new NPM(this.config.appiumHome);
}
async execute (args) {
- if (!this[args.driverCommand]) {
+ if (!_.isFunction(DriverCommand.prototype[args.driverCommand])) {
throw new Error(`Cannot handle driver command ${args.driverCommand}`);
}
return await this[args.driverCommand](args);
@@ -44,26 +55,24 @@ class DriverCommand {
async list ({showInstalled, showUpdates}) {
const lsMsg = `Listing ${showInstalled ? 'installed' : 'available'} drivers`;
- const drivers = {};
-
- for (const name of Object.keys(this.config.installedDrivers)) {
- drivers[name] = this.config.installedDrivers[name];
- drivers[name].installed = true;
- }
-
- if (!showInstalled) {
- for (const name of Object.keys(KNOWN_DRIVERS)) {
+ const installedNames = Object.keys(this.config.installedDrivers);
+ const knownNames = Object.keys(KNOWN_DRIVERS);
+ const drivers = [...installedNames, ...knownNames].reduce((drivers, name) => {
if (!drivers[name]) {
- drivers[name] = {
- pkgName: KNOWN_DRIVERS[name],
- installed: false,
- };
- }
+ if (installedNames.includes(name)) {
+ drivers[name] = {...this.config.installedDrivers[name], installed: true};
+ } else if (!showInstalled) {
+ drivers[name] = {pkgName: KNOWN_DRIVERS[name], installed: false};
}
}
+ return drivers;
+ }, {});
- await spinWith(this.json, lsMsg, async () => {
- if (showUpdates) {
+ // if we want to show whether updates are available, put that behind a spinner
+ await spinWith(this.isJsonOutput, lsMsg, async () => {
+ if (!showUpdates) {
+ return;
+ }
for (const [, data] of _.toPairs(drivers)) {
const {version, pkgName, installType} = data;
data.updateVersion = null;
@@ -74,10 +83,11 @@ class DriverCommand {
}
}
}
- }
});
- if (this.json) {
+ // if we're just getting the data, short circuit return here since we don't need to do any
+ // formatting logic
+ if (this.isJsonOutput) {
return drivers;
}
@@ -104,20 +114,21 @@ class DriverCommand {
` [${updateVersion} available]`.magenta :
'';
- console.log(`- ${name.yellow}${installTxt}${updateTxt}`); // eslint-disable-line no-console
+ console.log(`- ${name.yellow}${installTxt}${updateTxt}`);
}
return drivers;
}
async install ({driver, installType}) {
- log(this.json, `Attempting to find and install driver '${driver}'`);
+ log(this.isJsonOutput, `Attempting to find and install driver '${driver}'`);
- let driverData, installSpec = driver;
+ let driverData;
+ let installSpec = driver;
if (installType === INSTALL_TYPE_LOCAL) {
const msg = 'Linking driver from local path';
- const pkgJsonData = await spinWith(this.json, msg, async () => (
+ const pkgJsonData = await spinWith(this.isJsonOutput, msg, async () => (
await this.npm.linkPackage(installSpec))
);
driverData = this.getDriverFields(pkgJsonData);
@@ -175,9 +186,9 @@ class DriverCommand {
await this.config.addDriver(driverName, driverData);
// log info for the user
- log(this.json, `Driver ${driverName}@${driverData.version} successfully installed`.green);
- log(this.json, `- automationName: ${driverData.automationName.green}`);
- log(this.json, `- platformNames: ${JSON.stringify(driverData.platformNames).green}`);
+ log(this.isJsonOutput, `Driver ${driverName}@${driverData.version} successfully installed`.green);
+ log(this.isJsonOutput, `- automationName: ${driverData.automationName.green}`);
+ log(this.isJsonOutput, `- platformNames: ${JSON.stringify(driverData.platformNames).green}`);
return this.config.installedDrivers;
}
@@ -192,7 +203,7 @@ class DriverCommand {
const specMsg = npmSpec === driver ? '' : ` using NPM install spec '${npmSpec}'`;
const msg = `Installing '${driver}'${specMsg}`;
try {
- const pkgJsonData = await spinWith(this.json, msg, async () => (
+ const pkgJsonData = await spinWith(this.isJsonOutput, msg, async () => (
await this.npm.installPackage(path.resolve(this.config.appiumHome, installPath), pkgName, pkgVer)
));
const driverData = this.getDriverFields(pkgJsonData);
@@ -203,6 +214,15 @@ class DriverCommand {
}
}
+ /**
+ * Take an NPM module's package.json and extract Appium driver information from a special
+ * 'appium' field in the JSON data. We need this information to e.g. determine which class to
+ * load as the main driver class, or to be able to detect incompatibilities between driver and
+ * appium versions.
+ *
+ * @param [object] pkgJsonData - the package.json data for a driver module, as if it had been
+ * straightforwardly 'require'd
+ */
getDriverFields (pkgJsonData) {
if (!pkgJsonData.appium) {
throw new Error(`Installed driver did not have an 'appium' section in its ` +
@@ -212,8 +232,7 @@ class DriverCommand {
const {appium, name, version} = pkgJsonData;
const {driverName, automationName, platformNames, mainClass} = appium;
- const reqFields = ['driverName', 'automationName', 'platformNames', 'mainClass'];
- const missingFields = reqFields.reduce((acc, field) => (
+ const missingFields = REQ_DRIVER_FIELDS.reduce((acc, field) => (
appium[field] ? acc : [...acc, field]
), []);
@@ -234,7 +253,7 @@ class DriverCommand {
} finally {
await this.config.removeDriver(driver);
}
- log(this.json, `Successfully uninstalled driver '${driver}'`.green);
+ log(this.isJsonOutput, `Successfully uninstalled driver '${driver}'`.green);
return true;
}

MODIFY
lib/driver-config.js
lib/driver-config.js
@@ -30,7 +30,7 @@ export default class DriverConfig {
async read () {
await mkdirp(this.appiumHome); // ensure appium home exists
try {
- const yamlData = YAML.parse(await fs.readFile(this.configFile, 'utf-8'));
+ const yamlData = YAML.parse(await fs.readFile(this.configFile, 'utf8'));
// in the future if we need to do anything specific based on schema
// revision, we can check it as follows
// const schemaRev = yamlData.schemaRev;
@@ -44,6 +44,11 @@ export default class DriverConfig {
`cache file (${this.configFile}). Ensure it exists and is ` +
`readable. Specific error: ${err.message}`);
}
+ if (!await fs.access(this.appiumHome)) {
+ // if appium home exists but the dir is not readable/writable
+ throw new Error(`Appium could not read or write from the Appium Home directory ` +
+ `(${this.appiumHome}). Please ensure it is writable.`);
+ }
}
return this.installedDrivers;
}
@@ -115,7 +120,9 @@ export default class DriverConfig {
const problemSummaries = [];
for (const [driverName, problems] of _.toPairs(foundProblems)) {
- if (!_.isEmpty(problems)) {
+ if (_.isEmpty(problems)) {
+ continue;
+ }
// remove this driver from the list since it's not valid
delete drivers[driverName];
problemSummaries.push(`Driver ${driverName} had errors and will not ` +
@@ -125,7 +132,6 @@ export default class DriverConfig {
`${JSON.stringify(problem.val)})`);
}
}
- }
if (!_.isEmpty(problemSummaries)) {
this.log(`Appium encountered one or more errors while validating ` +
@@ -140,7 +146,7 @@ export default class DriverConfig {
async write () {
const yamlData = {schemaRev: CONFIG_SCHEMA_REV, drivers: this.installedDrivers};
- await fs.writeFile(this.configFile, YAML.stringify(yamlData), 'utf-8');
+ await fs.writeFile(this.configFile, YAML.stringify(yamlData), 'utf8');
}
async addDriver (driverName, driverData) {

MODIFY
lib/main.js
lib/main.js
@@ -21,7 +21,7 @@ import registerNode from './grid-register';
import { inspectObject } from './utils';
-async function preflightChecks (parser, args, driverConfig, throwInsteadOfExit = false) {
+async function preflightChecks ({parser, args, driverConfig, throwInsteadOfExit = false}) {
try {
checkNodeOk();
if (args.longStacktrace) {
@@ -130,7 +130,7 @@ async function main (args = null) {
let appiumDriver = new AppiumDriver(args);
appiumDriver.driverConfig = new DriverConfig(args.appiumHome);
- await preflightChecks(parser, args, appiumDriver.driverConfig, throwInsteadOfExit);
+ await preflightChecks({parser, args, driverConfig: appiumDriver.driverConfig, throwInsteadOfExit});
await logStartupInfo(parser, args);
let routeConfiguringFunction = makeRouter(appiumDriver);
let server = await baseServer({

MODIFY
lib/npm.js
lib/npm.js
@@ -19,7 +19,9 @@ export default class NPM {
}
// ensure the directory we want to install inside of exists
await mkdirp(cwd);
- execOpts.cwd = cwd;
+
+ // make sure we perform the current operation in cwd
+ execOpts = {...execOpts, cwd};
args.unshift(cmd);
if (json) {

MODIFY
package.json
package.json
"lodash": "^4.17.11",
"longjohn": "^0.2.12",
"npmlog": "4.x",
+ "ora": "^4.0.4",
"semver": "^7.0.0",
"source-map-support": "0.x",
"teen_process": "1.x",

refactor: address various code issues and move to ora for cli spinner