fix(plugins/slack): correctly send messages on html.finished message (#2971)
* fix(plugins/slack): correctly send messages on html.finished message Do not rely on `resultBaseUrl` to block messages to be sent to Slack. Instead, check if any of the supported hosting providers are correctly configured. If that is the case, then messages are sent _only after the upload is complete_. For all other cases, send the message anyway test(plugins/slack): add basic test scenarios - Check for missing options when calling `slack.open()` - Ensure that methods supposed to return lists always return them - Add some sanity checks to the summary, so that missing values would not crash * Skipping .nvmrc for now Co-authored-by: Peter Hedenskog <peter@soulgalore.com>
This commit is contained in:
Erick Wilder
GitHub
parent
53cf92276c
commit
4b0c9a7c6b
|
|
@ -18,7 +18,11 @@ module.exports = function(
|
|||
screenshotType
|
||||
) {
|
||||
const attachments = [];
|
||||
for (let url of dataCollector.getURLs()) {
|
||||
const urls =
|
||||
dataCollector && typeof dataCollector.getURLs === 'function'
|
||||
? dataCollector.getURLs()
|
||||
: [];
|
||||
for (let url of urls) {
|
||||
const results = dataCollector.getURLData(url);
|
||||
const base = results.data;
|
||||
const metrics = {
|
||||
|
|
|
|||
|
|
@ -57,7 +57,6 @@ class DataCollector {
|
|||
|
||||
addDataForUrl(url, typePath, data, runIndex) {
|
||||
this._addUrl(url);
|
||||
|
||||
if (runIndex !== undefined) {
|
||||
let runData = this.urlRunPages[url][runIndex] || {
|
||||
runIndex,
|
||||
|
|
|
|||
|
|
@ -86,9 +86,39 @@ function send(options, dataCollector, context, screenshotType) {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Detect configurations for a static website provider and return its name.
|
||||
*
|
||||
* @param {object} options
|
||||
* @return {string?} The provider name (if any could be detected)
|
||||
*/
|
||||
function staticPagesProvider(options) {
|
||||
const s3Options = options.s3;
|
||||
try {
|
||||
throwIfMissing(s3Options, ['bucketname'], 's3');
|
||||
if (s3Options.key || s3Options.secret) {
|
||||
throwIfMissing(s3Options, ['key', 'secret'], 's3');
|
||||
}
|
||||
return 's3';
|
||||
} catch (err) {
|
||||
log.debug('s3 is not configured');
|
||||
}
|
||||
|
||||
const gcsOptions = options.gcs;
|
||||
try {
|
||||
throwIfMissing(gcsOptions, ['projectId', 'key', 'bucketname'], 'gcs');
|
||||
return 'gcs';
|
||||
} catch (err) {
|
||||
log.debug('gcs is not configured');
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
open(context, options) {
|
||||
throwIfMissing(options.slack, ['hookUrl', 'userName'], 'slack');
|
||||
open(context, options = {}) {
|
||||
const slackOptions = options.slack || {};
|
||||
throwIfMissing(slackOptions, ['hookUrl', 'userName'], 'slack');
|
||||
this.dataCollector = new DataCollector(context);
|
||||
this.context = context;
|
||||
this.options = options;
|
||||
|
|
@ -143,15 +173,21 @@ module.exports = {
|
|||
}
|
||||
|
||||
case 'html.finished': {
|
||||
// if we have set a result base URL wait on
|
||||
// the content to be uploaded
|
||||
if (!this.options.resultBaseURL) {
|
||||
const provider = staticPagesProvider(this.options);
|
||||
if (!provider) {
|
||||
// Send the notification right away if no static website provider was configured
|
||||
return send(
|
||||
this.options,
|
||||
dataCollector,
|
||||
this.context,
|
||||
this.screenshotType
|
||||
);
|
||||
} else {
|
||||
log.info(
|
||||
'A static website hosting provider (%s) is configured. ' +
|
||||
'The Slack message should be sent after the HTML report is uploaded',
|
||||
provider
|
||||
);
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -56,16 +56,16 @@ module.exports = function(dataCollector, errors, resultUrls, name, options) {
|
|||
metric: h.size.format(get(base.pagexray, 'summary.transferSize.median'))
|
||||
}
|
||||
};
|
||||
const iterations = get(options, 'browsertime.iterations', 0);
|
||||
const browser = h.cap(get(options, 'browsertime.browser', 'unknown'));
|
||||
const pages = h.plural(dataCollector.getURLs().length, 'page');
|
||||
const testName = h.short(name || '', 30) || 'Unknown';
|
||||
const device = options.mobile ? 'mobile' : 'desktop';
|
||||
const runs = h.plural(iterations, 'run');
|
||||
|
||||
let summaryText =
|
||||
`${h.plural(dataCollector.getURLs().length, 'page')} analysed for ${h.short(
|
||||
name,
|
||||
30
|
||||
)} ` +
|
||||
`(${h.plural(options.browsertime.iterations, 'run')}, ` +
|
||||
`${h.cap(options.browsertime.browser)}/${
|
||||
options.mobile ? 'mobile' : 'desktop'
|
||||
}/${tsdbUtil.getConnectivity(options)})\n`;
|
||||
`${pages} analysed for ${testName} ` +
|
||||
`(${runs}, ${browser}/${device}/${tsdbUtil.getConnectivity(options)})\n`;
|
||||
|
||||
let message = '';
|
||||
if (resultUrls.hasBaseUrl()) {
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ docker push sitespeedio/sitespeed.io:$PACKAGE_VERSION-action
|
|||
docker build -t sitespeedio/sitespeed.io:$PACKAGE_VERSION-plus1 --build-arg version=$PACKAGE_VERSION --file docker/Dockerfile-plus1 .
|
||||
docker push sitespeedio/sitespeed.io:$PACKAGE_VERSION-plus1
|
||||
|
||||
# Update to latet version in the docs
|
||||
# Update to latest version in the docs
|
||||
bin/sitespeed.js --version | tr -d '\n' > docs/_includes/version/sitespeed.io.txt
|
||||
|
||||
# Generate the help for the docs
|
||||
|
|
|
|||
|
|
@ -0,0 +1,291 @@
|
|||
'use strict';
|
||||
const expect = require('chai').expect;
|
||||
const path = require('path');
|
||||
const fs = require('fs');
|
||||
|
||||
const resultUrls = require('../lib/core/resultsStorage/resultUrls');
|
||||
const messageMaker = require('../lib/support/messageMaker');
|
||||
const filterRegistry = require('../lib/support/filterRegistry');
|
||||
const intel = require('intel');
|
||||
const statsHelpers = require('../lib/support/statsHelpers');
|
||||
|
||||
const coachRunPath = path.resolve(__dirname, 'fixtures', 'coach.run-0.json');
|
||||
const coachRun = JSON.parse(fs.readFileSync(coachRunPath, 'utf8'));
|
||||
|
||||
const DataCollector = require('../lib/plugins/slack/dataCollector');
|
||||
|
||||
const defaultContextFactory = (context = {}) => {
|
||||
return Object.assign(
|
||||
{
|
||||
messageMaker,
|
||||
filterRegistry,
|
||||
intel,
|
||||
statsHelpers,
|
||||
resultUrls: resultUrls()
|
||||
},
|
||||
context
|
||||
);
|
||||
};
|
||||
|
||||
describe('slack', () => {
|
||||
describe('summary', () => {
|
||||
const getSummary = require('../lib/plugins/slack/summary');
|
||||
|
||||
it('should not hard crash without a name', () => {
|
||||
const dataCollector = new DataCollector(defaultContextFactory());
|
||||
const options = {
|
||||
browsertime: {
|
||||
browser: 'mosaic',
|
||||
iterations: 5
|
||||
}
|
||||
};
|
||||
const codeUnderTest = () =>
|
||||
getSummary(dataCollector, [], resultUrls(), undefined, options);
|
||||
expect(codeUnderTest).to.not.throw();
|
||||
|
||||
const summary = codeUnderTest();
|
||||
expect(summary.summaryText).to.have.string(
|
||||
'0 pages analysed for Unknown (5 runs, Mosaic/desktop/unknown)'
|
||||
);
|
||||
});
|
||||
|
||||
it('should not hard crash with undefined browsertime options', () => {
|
||||
const dataCollector = new DataCollector(defaultContextFactory());
|
||||
const options = {
|
||||
browsertime: undefined
|
||||
};
|
||||
const codeUnderTest = () =>
|
||||
getSummary(dataCollector, [], resultUrls(), '<TEST NAME>', options);
|
||||
expect(codeUnderTest).to.not.throw();
|
||||
|
||||
const summary = codeUnderTest();
|
||||
expect(summary.summaryText).to.have.string(
|
||||
'0 pages analysed for <TEST NAME> (0 runs, Unknown/desktop/unknown)'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('attachments', () => {
|
||||
const getAttachments = require('../lib/plugins/slack/attachements');
|
||||
|
||||
it('should always return a list', () => {
|
||||
expect(getAttachments()).to.be.an('array').that.is.empty;
|
||||
});
|
||||
});
|
||||
|
||||
describe('DataCollector', () => {
|
||||
it('should have an empty list of pages', () => {
|
||||
const context = defaultContextFactory();
|
||||
const collector = new DataCollector(context);
|
||||
expect(collector.getURLs()).to.be.an('array').that.is.empty;
|
||||
});
|
||||
|
||||
it('add data should add new page URL', () => {
|
||||
const context = defaultContextFactory();
|
||||
const collector = new DataCollector(context);
|
||||
|
||||
collector.addDataForUrl(
|
||||
'https://fake-site.sitespeed.io',
|
||||
'coach.run',
|
||||
{ coach: { pageSummary: coachRun } },
|
||||
undefined
|
||||
);
|
||||
|
||||
expect(collector.getURLs()).to.eql(['https://fake-site.sitespeed.io']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('index.open', () => {
|
||||
const plugin = require('../lib/plugins/slack');
|
||||
|
||||
it('without any valid option', () => {
|
||||
const undefinedOptions = () => {
|
||||
plugin.open(defaultContextFactory(), undefined);
|
||||
};
|
||||
expect(undefinedOptions).to.throw(
|
||||
/^Required option\(s\) .+(hookUrl|userName).+ need to be specified in namespace "slack"$/
|
||||
);
|
||||
|
||||
const emptyOptions = () => {
|
||||
plugin.open(defaultContextFactory(), {});
|
||||
};
|
||||
expect(emptyOptions).to.throw(
|
||||
/^Required option\(s\) .+(hookUrl|userName).+ need to be specified in namespace "slack"$/
|
||||
);
|
||||
|
||||
const undefinedSlackOptions = () => {
|
||||
plugin.open(defaultContextFactory(), { slack: undefined });
|
||||
};
|
||||
expect(undefinedSlackOptions).to.throw(
|
||||
/^Required option\(s\) .+(hookUrl|userName).+ need to be specified in namespace "slack"$/
|
||||
);
|
||||
|
||||
const emptySlackOptions = () => {
|
||||
plugin.open(defaultContextFactory(), { slack: {} });
|
||||
};
|
||||
expect(emptySlackOptions).to.throw(
|
||||
/^Required option\(s\) .+(hookUrl|userName).+ need to be specified in namespace "slack"$/
|
||||
);
|
||||
});
|
||||
|
||||
it('should require userName with hookUrl option', () => {
|
||||
const codeUnderTest = () => {
|
||||
plugin.open(defaultContextFactory(), {
|
||||
hookUrl: 'https://fake-slack.sitespeed.io/hook'
|
||||
});
|
||||
};
|
||||
expect(codeUnderTest).to.throw(
|
||||
/^Required option\(s\) .+(hookUrl|userName).+ need to be specified in namespace "slack"$/
|
||||
);
|
||||
});
|
||||
|
||||
it('should require hookUrl with userName option', () => {
|
||||
const codeUnderTest = () => {
|
||||
plugin.open(defaultContextFactory(), {
|
||||
userName: 'Sitespeed.io'
|
||||
});
|
||||
};
|
||||
expect(codeUnderTest).to.throw(
|
||||
/^Required option\(s\) .+(hookUrl|userName).+ need to be specified in namespace "slack"$/
|
||||
);
|
||||
});
|
||||
|
||||
it('should set plugin state for all valid options', () => {
|
||||
const options = Object.assign({}, plugin.config, {
|
||||
hookUrl: 'https://fake-slack.sitespeed.io/hook'
|
||||
});
|
||||
const context = defaultContextFactory();
|
||||
plugin.open(context, {
|
||||
slack: options
|
||||
});
|
||||
expect(plugin.options.slack).to.equal(options);
|
||||
expect(plugin.context).to.equal(context);
|
||||
});
|
||||
});
|
||||
|
||||
describe('index.processMessage', () => {
|
||||
const Slack = require('node-slack');
|
||||
const realSend = Slack.prototype.send;
|
||||
|
||||
function mockSend() {
|
||||
function mock() {
|
||||
mock.called = true;
|
||||
mock.callArgs = Array.from(arguments);
|
||||
}
|
||||
mock.called = false;
|
||||
mock.callArgs = [];
|
||||
Slack.prototype.send = mock;
|
||||
return mock;
|
||||
}
|
||||
|
||||
function pluginFactory(context, extraOptions = {}) {
|
||||
const plugin = require('../lib/plugins/slack');
|
||||
|
||||
const slackOptions = Object.assign({}, plugin.config, {
|
||||
hookUrl: 'https://fake-slack.sitespeed.io/hook'
|
||||
});
|
||||
const options = Object.assign(
|
||||
{
|
||||
slack: slackOptions,
|
||||
browsertime: {
|
||||
iterations: 1,
|
||||
browser: 'Chrome'
|
||||
}
|
||||
},
|
||||
extraOptions
|
||||
);
|
||||
plugin.open(context, options);
|
||||
plugin.processMessage({
|
||||
type: 'coach.pageSummary',
|
||||
url: 'http://fake-page.sitespeed.io/coach-run',
|
||||
data: coachRun,
|
||||
runIndex: undefined
|
||||
});
|
||||
return plugin;
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
Slack.prototype.send = realSend;
|
||||
});
|
||||
|
||||
it('should send message for html.finished message without baseUrl', () => {
|
||||
const context = defaultContextFactory({
|
||||
name: 'Simple test'
|
||||
});
|
||||
const plugin = pluginFactory(context);
|
||||
const mock = mockSend();
|
||||
plugin.processMessage({ type: 'html.finished' });
|
||||
|
||||
expect(mock.called).to.be.true;
|
||||
const params = mock.callArgs[0];
|
||||
expect(params.text).to.equal(
|
||||
'1 page analysed for Simple test (1 run, Chrome/desktop/unknown)\n*Site summary*\nPage transfer weight: N/A\n\n'
|
||||
);
|
||||
});
|
||||
|
||||
it('results URL should be part of the the report if baseUrl is provided', () => {
|
||||
const context = defaultContextFactory({
|
||||
name: 'Simple test'
|
||||
});
|
||||
context.resultUrls = resultUrls(
|
||||
'https://results.sitespeed.io/absolute/path'
|
||||
);
|
||||
const plugin = pluginFactory(context);
|
||||
const mock = mockSend();
|
||||
plugin.processMessage({ type: 'html.finished' });
|
||||
|
||||
expect(mock.called).to.be.true;
|
||||
const params = mock.callArgs[0];
|
||||
expect(params.text).to.equal(
|
||||
'1 page analysed for Simple test (1 run, Chrome/desktop/unknown)\n*Site summary* ' +
|
||||
'(<https://results.sitespeed.io/absolute/path/index.html |result>)\nPage transfer weight: N/A\n\n'
|
||||
);
|
||||
});
|
||||
|
||||
it('should not send message for html.finished with s3 configured', () => {
|
||||
const context = defaultContextFactory({
|
||||
name: 'S3 configured'
|
||||
});
|
||||
const plugin = pluginFactory(context, {
|
||||
s3: { bucketname: 'sitespeed_data_bucket' }
|
||||
});
|
||||
|
||||
const mock = mockSend();
|
||||
expect(Slack.prototype.send).to.equal(mock);
|
||||
|
||||
plugin.processMessage({ type: 'html.finished' });
|
||||
expect(mock.called).to.be.false;
|
||||
|
||||
plugin.processMessage({ type: 's3.finished' });
|
||||
expect(mock.called).to.be.true;
|
||||
expect(mock.callArgs[0].text).to.equal(
|
||||
'1 page analysed for S3 configured (1 run, Chrome/desktop/unknown)\n*Site summary*\nPage transfer weight: N/A\n\n'
|
||||
);
|
||||
});
|
||||
|
||||
it('should not send message for html.finished with gcs configured', () => {
|
||||
const context = defaultContextFactory({
|
||||
name: 'GCS'
|
||||
});
|
||||
const plugin = pluginFactory(context, {
|
||||
gcs: {
|
||||
bucketname: 'sitespeed_data_bucket',
|
||||
projectId: 123456789,
|
||||
key: 'sitespeed-1234567890'
|
||||
}
|
||||
});
|
||||
|
||||
const mock = mockSend();
|
||||
expect(Slack.prototype.send).to.equal(mock);
|
||||
|
||||
plugin.processMessage({ type: 'html.finished' });
|
||||
expect(mock.called).to.be.false;
|
||||
|
||||
plugin.processMessage({ type: 'gcs.finished' });
|
||||
expect(mock.called).to.be.true;
|
||||
expect(mock.callArgs[0].text).to.equal(
|
||||
'1 page analysed for GCS (1 run, Chrome/desktop/unknown)\n*Site summary*\nPage transfer weight: N/A\n\n'
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue