refactor: separate parser logic and cli args more clearly
RENAME
lib/server-args.js
lib/cli-args.js
-import fs from 'fs';
-import _ from 'lodash';
import { DEFAULT_BASE_PATH } from 'appium-base-driver';
+import { parseSecurityFeatures, parseDefaultCaps, parseInstallTypes } from './parser-helpers';
+import { INSTALL_TYPES, DEFAULT_APPIUM_HOME } from './driver-config';
-// serverArgs will be added to the `server` (default) subcommand
-function parseSecurityFeatures (features) {
- const splitter = (splitOn, str) => `${str}`.split(splitOn).map((s) => s.trim()).filter(Boolean);
- let parsedFeatures;
- try {
- parsedFeatures = splitter(',', features);
- } catch (err) {
- throw new Error('Could not parse value of --allow/deny-insecure. Should be ' +
- 'a list of strings separated by commas, or a path to a file ' +
- 'listing one feature name per line.');
- }
-
- if (parsedFeatures.length === 1 && fs.existsSync(parsedFeatures[0])) {
- // we might have a file which is a list of features
- try {
- const fileFeatures = fs.readFileSync(parsedFeatures[0], 'utf8');
- parsedFeatures = splitter('\n', fileFeatures);
- } catch (err) {
- throw new Error(`Attempted to read --allow/deny-insecure feature names ` +
- `from file ${parsedFeatures[0]} but got error: ${err.message}`);
- }
- }
-
- return parsedFeatures;
-}
-
-function parseDefaultCaps (caps) {
- try {
- // use synchronous file access, as `argparse` provides no way of either
- // awaiting or using callbacks. This step happens in startup, in what is
- // effectively command-line code, so nothing is blocked in terms of
- // sessions, so holding up the event loop does not incur the usual
- // drawbacks.
- if (fs.statSync(caps).isFile()) {
- caps = fs.readFileSync(caps, 'utf8');
- }
- } catch (err) {
- // not a file, or not readable
- }
- caps = JSON.parse(caps);
- if (!_.isPlainObject(caps)) {
- throw 'Invalid format for default capabilities';
- }
- return caps;
-}
-
-export default [
+// sharedArgs will be added to every subcommand
+const sharedArgs = [
+ [['-ah', '--home', '--appium-home'], {
+ required: false,
+ defaultValue: process.env.APPIUM_HOME || DEFAULT_APPIUM_HOME,
+ help: 'The path to the directory where Appium will keep installed drivers, plugins, and any other metadata necessary for its operation',
+ dest: 'appiumHome',
+ }],
+
+ [['--log-filters'], {
+ dest: 'logFilters',
+ defaultValue: null,
+ required: false,
+ help: 'Set the full path to a JSON file containing one or more log filtering rules',
+ example: '/home/rules.json',
+ }],
+];
+
+const serverArgs = [
[['--shell'], {
required: false,
defaultValue: null,
@@ -346,3 +318,77 @@ export default [
}],
];
+const globalDriverArgs = [
+ [['--json'], {
+ required: false,
+ defaultValue: false,
+ action: 'storeTrue',
+ help: 'Use JSON for output format',
+ nargs: 0,
+ dest: 'json'
+ }]
+];
+
+const driverListArgs = [
+ ...globalDriverArgs,
+ [['--installed'], {
+ required: false,
+ defaultValue: false,
+ action: 'storeTrue',
+ help: 'List only installed drivers',
+ nargs: 0,
+ dest: 'showInstalled'
+ }],
+ [['--updates'], {
+ required: false,
+ defaultValue: false,
+ action: 'storeTrue',
+ help: 'Show information about newer versions',
+ nargs: 0,
+ dest: 'showUpdates'
+ }]
+];
+
+const driverInstallArgs = [
+ ...globalDriverArgs,
+ [['driver'], {
+ type: 'string',
+ example: 'xcuitest',
+ help: 'Name of the driver to install',
+ }],
+ [['--source'], {
+ required: false,
+ defaultValue: null,
+ type: parseInstallTypes,
+ help: `Where to look for the driver if it is not one of Appium's verified ` +
+ `drivers. Possible values: ${JSON.stringify(INSTALL_TYPES)}`,
+ dest: 'installType'
+ }],
+];
+
+const driverUninstallArgs = [
+ ...globalDriverArgs,
+ [['driver'], {
+ type: 'string',
+ example: 'xcuitest',
+ help: 'Name of the driver to uninstall',
+ }],
+];
+
+const driverUpdateArgs = [
+ ...globalDriverArgs,
+ [['driver'], {
+ type: 'string',
+ example: 'xcuitest',
+ help: 'Name of the driver to update, or the word "all" to update all. To see available updates, run "appium driver list --installed --updates"',
+ }],
+];
+
+export {
+ sharedArgs,
+ serverArgs,
+ driverListArgs,
+ driverInstallArgs,
+ driverUninstallArgs,
+ driverUpdateArgs,
+};

MODIFY
lib/driver-config.js
lib/driver-config.js
@@ -4,6 +4,8 @@ import { fs, mkdirp } from 'appium-support';
import path from 'path';
import YAML from 'yaml';
+const DEFAULT_APPIUM_HOME = path.resolve(process.env.HOME, '.appium');
+
const CONFIG_FILE_NAME = 'drivers.yaml';
const CONFIG_SCHEMA_REV = 1;
@@ -195,5 +197,5 @@ export default class DriverConfig {
export {
INSTALL_TYPE_NPM, INSTALL_TYPE_GIT, INSTALL_TYPE_LOCAL, INSTALL_TYPE_GITHUB,
- INSTALL_TYPES
+ INSTALL_TYPES, DEFAULT_APPIUM_HOME
};

DELETE
lib/driver-parser.js
-import _ from 'lodash';
-import { INSTALL_TYPES } from './driver-config';
-
-const globalArgs = [
- [['--json'], {
- required: false,
- defaultValue: false,
- action: 'storeTrue',
- help: 'Use JSON for output format',
- nargs: 0,
- dest: 'json'
- }]
-];
-
-const listArgs = [
- ...globalArgs,
- [['--installed'], {
- required: false,
- defaultValue: false,
- action: 'storeTrue',
- help: 'List only installed drivers',
- nargs: 0,
- dest: 'showInstalled'
- }],
- [['--updates'], {
- required: false,
- defaultValue: false,
- action: 'storeTrue',
- help: 'Show information about newer versions',
- nargs: 0,
- dest: 'showUpdates'
- }]
-];
-
-const installArgs = [
- ...globalArgs,
- [['driver'], {
- type: 'string',
- example: 'xcuitest',
- help: 'Name of the driver to install',
- }],
- [['--source'], {
- required: false,
- defaultValue: null,
- type: parseInstallTypes,
- help: `Where to look for the driver if it is not one of Appium's verified ` +
- `drivers. Possible values: ${JSON.stringify(INSTALL_TYPES)}`,
- dest: 'installType'
- }],
-];
-
-const uninstallArgs = [
- ...globalArgs,
- [['driver'], {
- type: 'string',
- example: 'xcuitest',
- help: 'Name of the driver to uninstall',
- }],
-];
-
-const updateArgs = [
- ...globalArgs,
- [['driver'], {
- type: 'string',
- example: 'xcuitest',
- help: 'Name of the driver to update, or the word "all" to update all. To see available updates, run "appium driver list --installed --updates"',
- }],
-];
-
-function addDriverToParser (sharedArgs, subParsers, debug) {
- const driverParser = subParsers.addParser('driver', {
- addHelp: true,
- help: 'Access the driver management CLI commands',
- debug
- });
- const driverSubParsers = driverParser.addSubparsers({
- dest: 'driverCommand',
- debug
- });
- const parserSpecs = [
- {command: 'list', args: listArgs,
- help: 'List available and installed drivers'},
- {command: 'install', args: installArgs,
- help: 'Install a driver'},
- {command: 'uninstall', args: uninstallArgs,
- help: 'Uninstall a driver'},
- {command: 'update', args: updateArgs,
- help: 'Update installed drivers to the latest version'},
- ];
-
- for (const {command, args, help} of parserSpecs) {
- const parser = driverSubParsers.addParser(command, {help, debug});
- for (const [flags, opts] of [...sharedArgs, ...args]) {
- // addArgument mutates params so make sure to send in copies instead
- parser.addArgument([...flags], {...opts});
- }
- }
-}
-
-function parseInstallTypes (source) {
- if (!_.includes(INSTALL_TYPES, source)) {
- throw `Argument to --source was '${source}', which is not a valid ` +
- `driver source type. It must be one of ${JSON.stringify(INSTALL_TYPES)}`;
- }
-
- return source;
-}
-
-export {
- addDriverToParser
-};

MODIFY
lib/main.js
lib/main.js
@@ -6,8 +6,7 @@ import logger from './logger'; // logger needs to remain first of imports
import _ from 'lodash';
import { server as baseServer, routeConfiguringFunction as makeRouter } from 'appium-base-driver';
import { asyncify } from 'asyncbox';
-import { default as getParser } from './parser';
-import { getDefaultServerArgs } from './server-parser';
+import { default as getParser, getDefaultServerArgs } from './parser';
import { logger as logFactory, util } from 'appium-support';
import {
showConfig, checkNodeOk, validateServerArgs,

ADD
lib/parser-helpers.js
+import fs from 'fs';
+import _ from 'lodash';
+import { INSTALL_TYPES } from './driver-config';
+
+// serverArgs will be added to the `server` (default) subcommand
+function parseSecurityFeatures (features) {
+ const splitter = (splitOn, str) => `${str}`.split(splitOn).map((s) => s.trim()).filter(Boolean);
+ let parsedFeatures;
+ try {
+ parsedFeatures = splitter(',', features);
+ } catch (err) {
+ throw new Error('Could not parse value of --allow/deny-insecure. Should be ' +
+ 'a list of strings separated by commas, or a path to a file ' +
+ 'listing one feature name per line.');
+ }
+
+ if (parsedFeatures.length === 1 && fs.existsSync(parsedFeatures[0])) {
+ // we might have a file which is a list of features
+ try {
+ const fileFeatures = fs.readFileSync(parsedFeatures[0], 'utf8');
+ parsedFeatures = splitter('\n', fileFeatures);
+ } catch (err) {
+ throw new Error(`Attempted to read --allow/deny-insecure feature names ` +
+ `from file ${parsedFeatures[0]} but got error: ${err.message}`);
+ }
+ }
+
+ return parsedFeatures;
+}
+
+function parseDefaultCaps (caps) {
+ try {
+ // use synchronous file access, as `argparse` provides no way of either
+ // awaiting or using callbacks. This step happens in startup, in what is
+ // effectively command-line code, so nothing is blocked in terms of
+ // sessions, so holding up the event loop does not incur the usual
+ // drawbacks.
+ if (fs.statSync(caps).isFile()) {
+ caps = fs.readFileSync(caps, 'utf8');
+ }
+ } catch (err) {
+ // not a file, or not readable
+ }
+ caps = JSON.parse(caps);
+ if (!_.isPlainObject(caps)) {
+ throw 'Invalid format for default capabilities';
+ }
+ return caps;
+}
+
+function parseInstallTypes (source) {
+ if (!_.includes(INSTALL_TYPES, source)) {
+ throw `Argument to --source was '${source}', which is not a valid ` +
+ `driver source type. It must be one of ${JSON.stringify(INSTALL_TYPES)}`;
+ }
+
+ return source;
+}
+
+export {
+ parseSecurityFeatures,
+ parseDefaultCaps,
+ parseInstallTypes,
+};

MODIFY
lib/parser.js
lib/parser.js
import path from 'path';
import _ from 'lodash';
import { ArgumentParser } from 'argparse';
-import { addDriverToParser } from './driver-parser';
-import { addServerToParser } from './server-parser';
+import { sharedArgs, serverArgs, driverListArgs, driverInstallArgs, driverUninstallArgs,
+ driverUpdateArgs } from './cli-args';
import { rootDir } from './utils';
-const DEFAULT_APPIUM_HOME = path.resolve(process.env.HOME, '.appium');
-
-// sharedArgs will be added to every subcommand
-const sharedArgs = [
- [['-ah', '--home', '--appium-home'], {
- required: false,
- defaultValue: process.env.APPIUM_HOME || DEFAULT_APPIUM_HOME,
- help: 'The path to the directory where Appium will keep installed drivers, plugins, and any other metadata necessary for its operation',
- dest: 'appiumHome',
- }],
-
- [['--log-filters'], {
- dest: 'logFilters',
- defaultValue: null,
- required: false,
- help: 'Set the full path to a JSON file containing one or more log filtering rules',
- example: '/home/rules.json',
- }],
-];
-
function getParser (debug = false) {
const parser = new ArgumentParser({
version: require(path.resolve(rootDir, 'package.json')).version,
@@ -59,5 +39,58 @@ function getParser (debug = false) {
return parser;
}
+function addServerToParser (sharedArgs, subParsers, debug) {
+ const serverParser = subParsers.addParser('server', {
+ addHelp: true,
+ help: 'Run an Appium server',
+ debug
+ });
+
+ for (const [flags, opts] of [...sharedArgs, ...serverArgs]) {
+ // addArgument mutates arguments so make copies
+ serverParser.addArgument([...flags], {...opts});
+ }
+
+ return serverArgs;
+}
+
+function getDefaultServerArgs () {
+ let defaults = {};
+ for (let [, arg] of serverArgs) {
+ defaults[arg.dest] = arg.defaultValue;
+ }
+ return defaults;
+}
+
+function addDriverToParser (sharedArgs, subParsers, debug) {
+ const driverParser = subParsers.addParser('driver', {
+ addHelp: true,
+ help: 'Access the driver management CLI commands',
+ debug
+ });
+ const driverSubParsers = driverParser.addSubparsers({
+ dest: 'driverCommand',
+ debug
+ });
+ const parserSpecs = [
+ {command: 'list', args: driverListArgs,
+ help: 'List available and installed drivers'},
+ {command: 'install', args: driverInstallArgs,
+ help: 'Install a driver'},
+ {command: 'uninstall', args: driverUninstallArgs,
+ help: 'Uninstall a driver'},
+ {command: 'update', args: driverUpdateArgs,
+ help: 'Update installed drivers to the latest version'},
+ ];
+
+ for (const {command, args, help} of parserSpecs) {
+ const parser = driverSubParsers.addParser(command, {help, debug});
+ for (const [flags, opts] of [...sharedArgs, ...args]) {
+ // addArgument mutates params so make sure to send in copies instead
+ parser.addArgument([...flags], {...opts});
+ }
+ }
+}
+
export default getParser;
-export { getParser, DEFAULT_APPIUM_HOME };
+export { getParser, getDefaultServerArgs };

DELETE
lib/server-parser.js
-import serverArgs from './server-args';
-
-function addServerToParser (sharedArgs, subParsers, debug) {
- const serverParser = subParsers.addParser('server', {
- addHelp: true,
- help: 'Run an Appium server',
- debug
- });
-
- for (const [flags, opts] of [...sharedArgs, ...serverArgs]) {
- // addArgument mutates arguments so make copies
- serverParser.addArgument([...flags], {...opts});
- }
-
- return serverArgs;
-}
-
-function getDefaultServerArgs () {
- let defaults = {};
- for (let [, arg] of serverArgs) {
- defaults[arg.dest] = arg.defaultValue;
- }
- return defaults;
-}
-
-
-export {
- serverArgs,
- addServerToParser,
- getDefaultServerArgs,
-};

MODIFY
test/driver-e2e-specs.js
test/driver-e2e-specs.js
@@ -7,7 +7,7 @@ import chaiAsPromised from 'chai-as-promised';
import wd from 'wd';
import axios from 'axios';
import { main as appiumServer } from '../lib/main';
-import { DEFAULT_APPIUM_HOME } from '../lib/parser';
+import { DEFAULT_APPIUM_HOME } from '../lib/driver-config';
import { TEST_FAKE_APP, TEST_HOST, TEST_PORT } from './helpers';
import { BaseDriver } from 'appium-base-driver';
import { FakeDriver } from 'appium-fake-driver';

MODIFY
test/parser-specs.js
test/parser-specs.js
// transpile:mocha
-import { getParser, DEFAULT_APPIUM_HOME } from '../lib/parser';
-import { INSTALL_TYPES } from '../lib/driver-config';
+import { getParser } from '../lib/parser';
+import { INSTALL_TYPES, DEFAULT_APPIUM_HOME } from '../lib/driver-config';
import chai from 'chai';
const should = chai.should();

refactor: separate parser logic and cli args more clearly