Skip to content

feat: list relevant audits on category failure #56

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 22, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 78 additions & 34 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,32 +47,47 @@ const belowThreshold = (id, expected, categories) => {
return actual < expected;
};

const getError = (id, expected, results) => {
const category = results.find((c) => c.id === id);
return `Expected category ${chalk.magenta(
const getError = (id, expected, categories, audits) => {
const category = categories.find((c) => c.id === id);

const categoryError = `Expected category ${chalk.cyan(
category.title,
)} to be greater or equal to ${chalk.green(expected)} but got ${chalk.red(
category.score !== null ? category.score : 'unknown',
)}`;

const categoryAudits = category.auditRefs
.filter(({ weight, id }) => weight > 0 && audits[id].score < 1)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only list audits that donated to the category score based on weight, and also ignore audits with a perfect score

.map((ref) => {
const audit = audits[ref.id];
return ` '${chalk.cyan(
audit.title,
)}' received a score of ${chalk.yellow(audit.score)}`;
})
.join('\n');

return { message: categoryError, details: categoryAudits };
};

const formatResults = ({ results, thresholds }) => {
const categories = Object.values(
results.lhr.categories,
).map(({ title, score, id }) => ({ title, score, id }));
).map(({ title, score, id, auditRefs }) => ({ title, score, id, auditRefs }));

const categoriesBelowThreshold = Object.entries(
thresholds,
).filter(([id, expected]) => belowThreshold(id, expected, categories));

const errors = categoriesBelowThreshold.map(([id, expected]) =>
getError(id, expected, categories),
getError(id, expected, categories, results.lhr.audits),
);

const summary = {
results: categories.map((cat) => ({
...cat,
...(thresholds[cat.id] ? { threshold: thresholds[cat.id] } : {}),
results: categories.map(({ title, score, id }) => ({
title,
score,
id,
...(thresholds[id] ? { threshold: thresholds[id] } : {}),
})),
};

Expand All @@ -86,8 +101,8 @@ const formatResults = ({ results, thresholds }) => {
const getUtils = ({ utils }) => {
const failBuild =
(utils && utils.build && utils.build.failBuild) ||
((message, { error }) => {
console.error(message, error.message);
((message, { error } = {}) => {
console.error(message, error && error.message);
process.exitCode = 1;
});

Expand Down Expand Up @@ -126,34 +141,53 @@ const runAudit = async ({ path, url, thresholds }) => {
return {
summary,
shortSummary,
error: errors.length > 0 ? new Error(`\n${errors.join('\n')}`) : false,
errors,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't format the message here so we can create both a short message and a detailed message later on.

};
}
} catch (error) {
return { error };
}
};

const prefixString = ({ path, url, str }) => {
if (path) {
return `\n${chalk.red('Error')} for directory '${chalk.cyan(
path,
)}':\n${str}`;
} else if (url) {
return `\n${chalk.red('Error')} for url '${chalk.cyan(url)}':\n${str}`;
} else {
return `\n${str}`;
}
};

const processResults = ({ summaries, errors }) => {
if (errors.length > 0) {
const error = errors.reduce(
(acc, { path, url, errors }) => {
const message = prefixString({
path,
url,
str: errors.map((e) => e.message).join('\n'),
});
const details = prefixString({
path,
url,
str: errors.map((e) => `${e.message}\n${e.details}`).join('\n'),
});

return {
message: `${acc.message}\n${message}`,
details: `${acc.details}\n${details}`,
};
},
{
message: '',
details: '',
},
);
return {
error: new Error(
errors
.map(({ path, url, error }) => {
if (path) {
return `\n${chalk.red('Error')} for directory '${chalk.magenta(
path,
)}': ${error.message}`;
}
if (url) {
return `\n${chalk.red('Error')} for url '${chalk.magenta(
url,
)}': ${error.message}`;
}
return `\n${error.message}`;
})
.join('\n'),
),
error,
};
} else {
return {
Expand Down Expand Up @@ -182,31 +216,41 @@ module.exports = {
inputs,
});

const errors = [];
const allErrors = [];
const summaries = [];
for (const { path, url, thresholds } of audits) {
const { error, summary, shortSummary } = await runAudit({
const { errors, summary, shortSummary } = await runAudit({
path,
url,
thresholds,
});
if (summary) {
console.log(summary);
}
if (error) {
errors.push({ path, url, error });
if (Array.isArray(errors) && errors.length > 0) {
allErrors.push({ path, url, errors });
} else {
summaries.push({ path, url, summary: shortSummary });
}
}

const { error, summary } = processResults({ summaries, errors, show });
const { error, summary } = processResults({
summaries,
errors: allErrors,
show,
});

if (error) {
throw error;
}
show({ summary });
} catch (error) {
failBuild(chalk.red('Failed with error:\n'), { error });
if (error.details) {
console.error(error.details);
failBuild(`${chalk.red('Failed with error:\n')}${error.message}`);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need the internal stack trace in this case.

} else {
failBuild(`${chalk.red('Failed with error:\n')}`, { error });
}
}
},
};