mirror
/
sitespeed.io
mirror of https://github.com/sitespeedio/sitespeed.io.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Refactor grafana cli options (#2984) 

* test(cli): add basic coverage to test cli help output

- Add basic coverated when invoking the command without arguments
- Basic checks for grafana help

* feat(cli): delegate command line options configuration to the Grafana plugin

It uses the name of the plugin as a namespace for all options. Plugin authors _may_ use this
in the future to show helpful messages in the default sitespeed CLI help output.

- Move CLI options to its own module to not pollute the plugin's `index.js` file.
- Change the plugin interface to define a `.cliOptions` property or method containing the `yargs`
  configuration to be used by the CLI code
- Add code coverage for the existing plugin API to ensure no regressions are introduced for Grafana
- Move parts of the `yargs`-related code to `cli/util.js`. Tests to ensure the initial proposition for
  plugin-based CLI options are included.

Co-authored-by: Peter Hedenskog <peter@soulgalore.com>
This commit is contained in:
Erick Wilder 2020-05-11 20:31:52 +02:00 committed by GitHub
parent ec951bf221
commit 8cf3c24722
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 452 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
lib/cli/cli.js
View File

@ -10,7 +10,9 @@ const set = require('lodash.set');
const get = require('lodash.get');
const findUp = require('find-up');
const toArray = require('../support/util').toArray;
const grafanaConfig = require('../plugins/grafana/index').config;
const grafanaPlugin = require('../plugins/grafana/index');
const graphiteConfig = require('../plugins/graphite/index').config;
const influxdbConfig = require('../plugins/influxdb/index').config;
const browsertimeConfig = require('../plugins/browsertime/index').config;
@ -833,45 +835,12 @@ module.exports.parseCommandLine = function parseCommandLine() {
describe:
'Discard URLs not matching the provided regular expression (ex: "/some/path/", "://some\\.domain/"). Can be provided multiple times.',
group: 'Crawler'
})
/**
Grafana cli option
*/
.option('grafana.host', {
describe: 'The Grafana host used when sending annotations.',
group: 'Grafana'
})
.option('grafana.port', {
default: grafanaConfig.port,
describe: 'The Grafana port used when sending annotations to Grafana.',
group: 'Grafana'
})
.option('grafana.auth', {
describe:
'The Grafana auth/bearer value used when sending annotations to Grafana. See http://docs.grafana.org/http_api/auth/#authentication-api',
group: 'Grafana'
})
.option('grafana.annotationTitle', {
describe: 'Add a title to the annotation sent for a run.',
group: 'Grafana'
})
.option('grafana.annotationMessage', {
describe:
'Add an extra message that will be attached to the annotation sent for a run. The message is attached after the default message and can contain HTML.',
group: 'Grafana'
})
.option('grafana.annotationTag', {
describe:
'Add a extra tag to the annotation sent for a run. Repeat the --grafana.annotationTag option for multiple tags. Make sure they do not collide with the other tags.',
group: 'Grafana'
})
.option('grafana.annotationScreenshot', {
default: false,
type: 'boolean',
describe:
'Include screenshot (from Browsertime/WebPageTest) in the annotation. You need to specify a --resultBaseURL for this to work.',
group: 'Grafana'
})
});
// Grafana CLI options
cliUtil.registerPluginOptions(parsed, grafanaPlugin);
parsed
/**
Graphite cli option
*/

64
lib/cli/util.js
View File

@ -5,6 +5,29 @@
const fs = require('fs');
const path = require('path');
const toArray = require('../support/util').toArray;
const format = require('util').format;
/**
*
* @param {Object<string, any>} options The plugin options to normalise
*/
function sanitizePluginOptions(options) {
// Allow .cliOptions to be defined either as a method, returning an object
// or an object propety directly (computed or not)
const cliOptions = typeof options === 'function' ? options() : options;
const isValidType =
typeof cliOptions === 'object' &&
cliOptions !== null &&
cliOptions.constructor === Object;
if (!isValidType) {
const current = format(cliOptions);
throw new Error(`Invalid CLI options defined for plugin: ${current}`);
}
return cliOptions;
}
module.exports = {
getURLs(urls) {
@ -89,5 +112,46 @@ module.exports = {
}
}
return urlMetaData;
},
/**
* Retrieve a mapping of option names and their corressponding default values based on a plugin CLI configuration
*
* @param {Object<string, require('yargs').Options>} cliOptions a map of option names: yargs option
*/
pluginDefaults(cliOptions) {
let config = {};
try {
Object.entries(sanitizePluginOptions(cliOptions)).forEach(values => {
const [key, options] = values;
if (typeof options.default !== 'undefined') {
config[key] = options.default;
}
});
} catch (err) {
// In case of invalid values, just assume an empty object.
config = {};
}
return config;
},
/**
* Configure yargs options for a given plugin defining a `cliOptions` method or property. The names
* of configuration options are namespaced with the plugin name.
*
* @param {require('yargs').Argv} parsed yargs instance
* @param {object} plugin a sitespeed plugin instance
*/
registerPluginOptions(parsed, plugin) {
if (typeof plugin.name !== 'function' || !plugin.name()) {
throw new Error(
'Missing name() method for plugin registering CLI options'
);
}
const cliOptions = sanitizePluginOptions(plugin.cliOptions);
Object.entries(cliOptions).forEach(value => {
const [key, yargsOptions] = value;
parsed.option(`${plugin.name()}.${key}`, yargsOptions);
});
}
};

37
lib/plugins/grafana/cli.js Normal file
View File

@ -0,0 +1,37 @@
module.exports = {
host: {
describe: 'The Grafana host used when sending annotations.',
group: 'Grafana'
},
port: {
default: 80,
describe: 'The Grafana port used when sending annotations to Grafana.',
group: 'Grafana'
},
auth: {
describe:
'The Grafana auth/bearer value used when sending annotations to Grafana. See http://docs.grafana.org/http_api/auth/#authentication-api',
group: 'Grafana'
},
annotationTitle: {
describe: 'Add a title to the annotation sent for a run.',
group: 'Grafana'
},
annotationMessage: {
describe:
'Add an extra message that will be attached to the annotation sent for a run. The message is attached after the default message and can contain HTML.',
group: 'Grafana'
},
annotationTag: {
describe:
'Add a extra tag to the annotation sent for a run. Repeat the --grafana.annotationTag option for multiple tags. Make sure they do not collide with the other tags.',
group: 'Grafana'
},
annotationScreenshot: {
default: false,
type: 'boolean',
describe:
'Include screenshot (from Browsertime/WebPageTest) in the annotation. You need to specify a --resultBaseURL for this to work.',
group: 'Grafana'
}
};

32
lib/plugins/grafana/index.js
View File

@ -1,11 +1,25 @@
'use strict';
const path = require('path');
const sendAnnotations = require('./send-annotation');
const tsdbUtil = require('../../support/tsdbUtil');
const throwIfMissing = require('../../support/util').throwIfMissing;
const defaultConfig = {
port: 80
};
const cliUtil = require('../../cli/util');
module.exports = {
name() {
return path.basename(__dirname);
},
/**
* Define `yargs` options with their respective default values. When displayed by the CLI help message
* all options are namespaced by its plugin name.
*
* @return {Object<string, require('yargs').Options} an object mapping the name of the option and its yargs configuration
*/
get cliOptions() {
return require(path.resolve(__dirname, 'cli.js'));
},
open(context, options) {
throwIfMissing(options.grafana, ['host', 'port'], 'grafana');
this.options = options;
@ -18,6 +32,7 @@ module.exports = {
this.alias = {};
this.wptExtras = {};
},
processMessage(message, queue) {
if (message.type === 'webpagetest.pageSummary') {
this.wptExtras[message.url] = {};
@ -86,5 +101,14 @@ module.exports = {
);
}
},
config: defaultConfig
/**
* At the time of this writing, this property's usage could be verified only by the CLI portion of the codebase.
* Instead of introducting a breaking change in the plugin interface, this is kept.
*
* @todo Inspect the code base and plugin dependencies to ensure this property can be removed (if necessary)
*/
get config() {
return cliUtil.pluginDefaults(this.cliOptions);
}
};

82
test/cliTests.js Normal file
View File

@ -0,0 +1,82 @@
'use strict';
const expect = require('chai').expect;
const path = require('path');
const util = require('util');
const execFile = util.promisify(require('child_process').execFile);
function runSitespeed(options = []) {
const cli = path.join(path.resolve(__dirname), '../bin/sitespeed.js');
return execFile('node', [cli].concat(options));
}
describe('cli', () => {
context('no arguments', () => {
it('should report error', async () => {
let stderr;
let exitCode = 0;
try {
await runSitespeed();
} catch (err) {
stderr = err.stderr;
exitCode = err.code;
}
expect(stderr).not.to.be.undefined;
expect(exitCode).not.to.equal(0);
expect(stderr).to.include('sitespeed.js [options] <url>/<file>');
expect(stderr).to.include('One or multiple URLs or scripts');
});
});
describe('--help summary', async () => {
let stdout;
before(async () => {
const run = await runSitespeed(['--help']);
stdout = run.stdout;
});
it('should include banner in stdout with --help and not hard exit', async () => {
expect(stdout).not.to.be.undefined;
expect(stdout).to.include('sitespeed.js [options] <url>/<file>');
expect(stdout).to.not.include('One or multiple URLs or scripts');
});
it('should contain grafana options', () => {
expect(stdout).to.contain('--grafana.host');
expect(stdout).to.contain(
'The Grafana host used when sending annotations'
);
expect(stdout).to.contain('--grafana.port');
expect(stdout).to.contain(
'The Grafana port used when sending annotations to Grafana'
);
expect(stdout).to.contain('--grafana.auth');
expect(stdout).to.contain(
'The Grafana auth/bearer value used when sending annotations to Grafana. See http://docs.grafana.org/http_api/auth/#authentication-api'
);
expect(stdout).to.contain('--grafana.annotationTitle');
expect(stdout).to.contain(
'Add a title to the annotation sent for a run.'
);
expect(stdout).to.contain('--grafana.annotationMessage');
expect(stdout).to.contain(
'Add an extra message that will be attached to the annotation sent for a run. The message is attached after the default message and can contain HTML.'
);
expect(stdout).to.contain('--grafana.annotationTag');
expect(stdout).to.contain(
'Add a extra tag to the annotation sent for a run. Repeat the --grafana.annotationTag option for multiple tags. Make sure they do not collide with the other tags.'
);
expect(stdout).to.contain('--grafana.annotationScreenshot');
expect(stdout).to.contain(
'Include screenshot (from Browsertime/WebPageTest) in the annotation. You need to specify a --resultBaseURL for this to work.'
);
});
});
});

213
test/cliUtilTests.js
View File

@ -15,6 +15,7 @@ describe('cliUtil', function() {
expect(urls[3] === 'https://www.sitespeed.io/documentation/faq');
});
});
describe('getAliases', function() {
it('should extract aliases', function() {
let aliases = cliUtil.getAliases(['test/fixtures/sitespeed-urls.txt']);
@ -38,4 +39,216 @@ describe('cliUtil', function() {
).to.be.undefined;
});
});
describe('pluginDefaults', () => {
it('should yield an empty object for invalid values', () => {
expect(cliUtil.pluginDefaults()).to.eql({});
expect(cliUtil.pluginDefaults(null)).to.eql({});
expect(cliUtil.pluginDefaults(1)).to.eql({});
expect(cliUtil.pluginDefaults(true)).to.eql({});
expect(cliUtil.pluginDefaults(false)).to.eql({});
expect(cliUtil.pluginDefaults('trust me!')).to.eql({});
});
it('should yield a map of defaults based on config names and its defaults', () => {
const cliOptions = {
propName: {
default: 'value'
}
};
expect(cliUtil.pluginDefaults(cliOptions).propName).to.eql('value');
});
it('should not include options without an explicit default set', () => {
const cliOptions = {
propName: {
default: 'value'
},
otherProp: {
description: 'No default set'
}
};
expect(cliUtil.pluginDefaults(cliOptions).otherProp).to.be.undefined;
});
});
describe('registerPluginOptions', () => {
const mockYargs = () => ({
calls: [],
option() {
this.calls.push(Array.from(arguments));
}
});
it('should not setup options with invalid values', () => {
const plugin = {
name() {
return 'test';
}
};
const codeUnderTest = () =>
cliUtil.registerPluginOptions(mockYargs(), plugin);
// Undefined cliOptions
expect(codeUnderTest).to.throw(
`Invalid CLI options defined for plugin: undefined`
);
plugin.cliOptions = null;
expect(codeUnderTest).to.throw(
`Invalid CLI options defined for plugin: null`
);
plugin.cliOptions = true;
expect(codeUnderTest).to.throw(
`Invalid CLI options defined for plugin: true`
);
plugin.cliOptions = false;
expect(codeUnderTest).to.throw(
`Invalid CLI options defined for plugin: false`
);
plugin.cliOptions = 1;
expect(codeUnderTest).to.throw(
`Invalid CLI options defined for plugin: 1`
);
plugin.cliOptions =
'trust me! I am a valid options definition :puss_in_boots_eyes:';
expect(codeUnderTest).to.throw(
`Invalid CLI options defined for plugin: trust me! I am a valid options definition :puss_in_boots_eyes:`
);
});
it('must have an explicit name defined in the plugin', () => {
function codeUnderTest() {
cliUtil.registerPluginOptions(mockYargs(), {
processMessage() {
// Apparently a safe plugin definition
return undefined;
}
});
}
expect(codeUnderTest).to.throw(
`Missing name() method for plugin registering CLI options`
);
});
it('should call yargs.options() when cliOptions is defined as method', () => {
const fakeYargs = mockYargs();
const plugin = {
name() {
return 'test';
},
cliOptions() {
return {
prop1: {
default: 80
},
'nested.config': {
description: 'Any help text'
}
};
}
};
cliUtil.registerPluginOptions(fakeYargs, plugin);
expect(fakeYargs.calls)
.to.be.an('array')
.with.lengthOf(2);
const expectedProp1 = ['test.prop1', { default: 80 }];
expect(fakeYargs.calls.find(call => call[0] === 'test.prop1')).to.eql(
expectedProp1
);
const expectedNested = [
'test.nested.config',
{ description: 'Any help text' }
];
expect(
fakeYargs.calls.find(call => call[0] === 'test.nested.config')
).to.eql(expectedNested);
});
it('should call yargs.options() when cliOptions is defined as computed property', () => {
const fakeYargs = mockYargs();
const plugin = {
name() {
return 'test';
},
get cliOptions() {
return {
prop1: {
default: 80
},
'nested.config': {
description: 'Any help text'
}
};
}
};
cliUtil.registerPluginOptions(fakeYargs, plugin);
expect(fakeYargs.calls)
.to.be.an('array')
.with.lengthOf(2);
const expectedProp1 = ['test.prop1', { default: 80 }];
expect(fakeYargs.calls.find(call => call[0] === 'test.prop1')).to.eql(
expectedProp1
);
const expectedNested = [
'test.nested.config',
{ description: 'Any help text' }
];
expect(
fakeYargs.calls.find(call => call[0] === 'test.nested.config')
).to.eql(expectedNested);
});
it('should call yargs.options() when cliOptions is defined as regular property', () => {
const fakeYargs = mockYargs();
const plugin = {
name() {
return 'test';
},
cliOptions: {
prop1: {
default: 80
},
'nested.config': {
description: 'Any help text'
}
}
};
cliUtil.registerPluginOptions(fakeYargs, plugin);
expect(fakeYargs.calls)
.to.be.an('array')
.with.lengthOf(2);
const expectedProp1 = ['test.prop1', { default: 80 }];
expect(fakeYargs.calls.find(call => call[0] === 'test.prop1')).to.eql(
expectedProp1
);
const expectedNested = [
'test.nested.config',
{ description: 'Any help text' }
];
expect(
fakeYargs.calls.find(call => call[0] === 'test.nested.config')
).to.eql(expectedNested);
});
});
});

19
test/grafanaTests.js Normal file
View File

@ -0,0 +1,19 @@
const expect = require('chai').expect;
describe('grafana', () => {
function plugin() {
return require('../lib/plugins/grafana');
}
describe('plugin', () => {
it('should have a .config property with default host port value', () => {
// Regression: ensure the original interface is not broken
expect(plugin().config).to.have.property('port', 80);
});
it('should include missing annotationScreenshot default value', () => {
// Regression. All defaults should be exposed
expect(plugin().config).to.have.property('annotationScreenshot', false);
});
});
});