Tidy up ready for review

This commit is contained in:
Simon Engledew 2020-11-24 17:21:25 +00:00
parent 1020df5700
commit 92ffb08081
No known key found for this signature in database
GPG key ID: 84302E7B02FE8BCE
6 changed files with 92 additions and 106 deletions

57
lib/actions-util.js generated
View file

@ -107,30 +107,20 @@ var MissingTriggers;
MissingTriggers[MissingTriggers["Push"] = 1] = "Push"; MissingTriggers[MissingTriggers["Push"] = 1] = "Push";
MissingTriggers[MissingTriggers["PullRequest"] = 2] = "PullRequest"; MissingTriggers[MissingTriggers["PullRequest"] = 2] = "PullRequest";
})(MissingTriggers || (MissingTriggers = {})); })(MissingTriggers || (MissingTriggers = {}));
exports.ErrCheckoutWrongHead = { function toCodedErrors(errors) {
message: `Git checkout HEAD^2 is no longer necessary. Please remove this line.`, return Object.entries(errors).reduce((acc, [key, value]) => {
code: "CheckoutWrongHead", acc[key] = { message: value, code: key };
}; return acc;
exports.ErrMismatchedBranches = { }, {});
message: `Please make sure that every branch in on.pull_request is also in on.push so that CodeQL can establish a baseline.`, }
code: "MismatchedBranches", exports.WorkflowErrors = toCodedErrors({
}; MismatchedBranches: `Please make sure that every branch in on.pull_request is also in on.push so that CodeQL can establish a baseline.`,
exports.ErrMissingHooks = { MissingHooks: `Please specify on.push and on.pull_request hooks.`,
message: `Please specify on.push and on.pull_request hooks.`, MissingPullRequestHook: `Please specify an on.pull_request hook so CodeQL is run against new pull requests.`,
code: "MissingHooks", MissingPushHook: `Please specify an on.push hook so CodeQL can establish a baseline.`,
}; PathsSpecified: `Please do not specify paths at on.pull.`,
exports.ErrMissingPushHook = { CheckoutWrongHead: `Git checkout HEAD^2 is no longer necessary. Please remove this line.`,
message: `Please specify an on.push hook so CodeQL can establish a baseline.`, });
code: "MissingPushHook",
};
exports.ErrMissingPullRequestHook = {
message: `Please specify an on.pull_request hook so CodeQL is run against new pull requests.`,
code: "MissingPullRequestHook",
};
exports.ErrPathsSpecified = {
message: `Please do not specify paths at on.pull.`,
code: "PathsSpecified",
};
function validateWorkflow(doc) { function validateWorkflow(doc) {
var _a, _b, _c, _d; var _a, _b, _c, _d;
const errors = []; const errors = [];
@ -138,7 +128,7 @@ function validateWorkflow(doc) {
for (const job of Object.values(((_a = doc) === null || _a === void 0 ? void 0 : _a.jobs) || {})) { for (const job of Object.values(((_a = doc) === null || _a === void 0 ? void 0 : _a.jobs) || {})) {
for (const step of ((_b = job) === null || _b === void 0 ? void 0 : _b.steps) || []) { for (const step of ((_b = job) === null || _b === void 0 ? void 0 : _b.steps) || []) {
if (((_c = step) === null || _c === void 0 ? void 0 : _c.run) === "git checkout HEAD^2") { if (((_c = step) === null || _c === void 0 ? void 0 : _c.run) === "git checkout HEAD^2") {
errors.push(exports.ErrCheckoutWrongHead); errors.push(exports.WorkflowErrors.CheckoutWrongHead);
} }
} }
} }
@ -177,7 +167,7 @@ function validateWorkflow(doc) {
else { else {
const paths = (_d = doc.on.push) === null || _d === void 0 ? void 0 : _d.paths; const paths = (_d = doc.on.push) === null || _d === void 0 ? void 0 : _d.paths;
if (Array.isArray(paths) && paths.length > 0) { if (Array.isArray(paths) && paths.length > 0) {
errors.push(exports.ErrPathsSpecified); errors.push(exports.WorkflowErrors.PathsSpecified);
} }
} }
if (doc.on.push) { if (doc.on.push) {
@ -186,23 +176,23 @@ function validateWorkflow(doc) {
const pull_request = doc.on.pull_request.branches || []; const pull_request = doc.on.pull_request.branches || [];
const intersects = pull_request.filter((value) => !push.includes(value)); const intersects = pull_request.filter((value) => !push.includes(value));
if (intersects.length > 0) { if (intersects.length > 0) {
errors.push(exports.ErrMismatchedBranches); errors.push(exports.WorkflowErrors.MismatchedBranches);
} }
} }
else if (push.length > 0) { else if (push.length > 0) {
errors.push(exports.ErrMismatchedBranches); errors.push(exports.WorkflowErrors.MismatchedBranches);
} }
} }
} }
switch (missing) { switch (missing) {
case MissingTriggers.PullRequest | MissingTriggers.Push: case MissingTriggers.PullRequest | MissingTriggers.Push:
errors.push(exports.ErrMissingHooks); errors.push(exports.WorkflowErrors.MissingHooks);
break; break;
case MissingTriggers.PullRequest: case MissingTriggers.PullRequest:
errors.push(exports.ErrMissingPullRequestHook); errors.push(exports.WorkflowErrors.MissingPullRequestHook);
break; break;
case MissingTriggers.Push: case MissingTriggers.Push:
errors.push(exports.ErrMissingPushHook); errors.push(exports.WorkflowErrors.MissingPushHook);
break; break;
} }
return errors; return errors;
@ -221,7 +211,10 @@ async function getWorkflowErrors() {
} }
exports.getWorkflowErrors = getWorkflowErrors; exports.getWorkflowErrors = getWorkflowErrors;
function formatWorkflowErrors(errors) { function formatWorkflowErrors(errors) {
return `${errors.length} issue${errors.length === 1 ? " was" : "s were"} detected with this workflow: ${errors.map((e) => e.message).join(", ")}`; const issuesWere = errors.length === 1 ? "issue was" : "issues were";
return `${errors.length} ${issuesWere} detected with this workflow: ${errors
.map((e) => e.message)
.join(", ")}`;
} }
exports.formatWorkflowErrors = formatWorkflowErrors; exports.formatWorkflowErrors = formatWorkflowErrors;
function formatWorkflowCause(errors) { function formatWorkflowCause(errors) {

File diff suppressed because one or more lines are too long

View file

@ -70,20 +70,20 @@ ava_1.default("prepareEnvironment() when a local run", (t) => {
}); });
ava_1.default("validateWorkflow() when on is missing", (t) => { ava_1.default("validateWorkflow() when on is missing", (t) => {
const errors = actionsutil.validateWorkflow({}); const errors = actionsutil.validateWorkflow({});
t.deepEqual(errors, [actionsutil.ErrMissingHooks]); t.deepEqual(errors, [actionsutil.WorkflowErrors.MissingHooks]);
}); });
ava_1.default("validateWorkflow() when on.push is missing", (t) => { ava_1.default("validateWorkflow() when on.push is missing", (t) => {
const errors = actionsutil.validateWorkflow({ on: {} }); const errors = actionsutil.validateWorkflow({ on: {} });
console.log(errors); console.log(errors);
t.deepEqual(errors, [actionsutil.ErrMissingHooks]); t.deepEqual(errors, [actionsutil.WorkflowErrors.MissingHooks]);
}); });
ava_1.default("validateWorkflow() when on.push is an array missing pull_request", (t) => { ava_1.default("validateWorkflow() when on.push is an array missing pull_request", (t) => {
const errors = actionsutil.validateWorkflow({ on: ["push"] }); const errors = actionsutil.validateWorkflow({ on: ["push"] });
t.deepEqual(errors, [actionsutil.ErrMissingPullRequestHook]); t.deepEqual(errors, [actionsutil.WorkflowErrors.MissingPullRequestHook]);
}); });
ava_1.default("validateWorkflow() when on.push is an array missing push", (t) => { ava_1.default("validateWorkflow() when on.push is an array missing push", (t) => {
const errors = actionsutil.validateWorkflow({ on: ["pull_request"] }); const errors = actionsutil.validateWorkflow({ on: ["pull_request"] });
t.deepEqual(errors, [actionsutil.ErrMissingPushHook]); t.deepEqual(errors, [actionsutil.WorkflowErrors.MissingPushHook]);
}); });
ava_1.default("validateWorkflow() when on.push is valid", (t) => { ava_1.default("validateWorkflow() when on.push is valid", (t) => {
const errors = actionsutil.validateWorkflow({ const errors = actionsutil.validateWorkflow({
@ -104,7 +104,7 @@ ava_1.default("validateWorkflow() when on.push should not have a path", (t) => {
pull_request: { branches: ["main"] }, pull_request: { branches: ["main"] },
}, },
}); });
t.deepEqual(errors, [actionsutil.ErrPathsSpecified]); t.deepEqual(errors, [actionsutil.WorkflowErrors.PathsSpecified]);
}); });
ava_1.default("validateWorkflow() when on.push is a correct object", (t) => { ava_1.default("validateWorkflow() when on.push is a correct object", (t) => {
const errors = actionsutil.validateWorkflow({ const errors = actionsutil.validateWorkflow({
@ -126,7 +126,7 @@ ava_1.default("validateWorkflow() when on.push is mismatched", (t) => {
pull_request: { branches: ["feature"] }, pull_request: { branches: ["feature"] },
}, },
}); });
t.deepEqual(errors, [actionsutil.ErrMismatchedBranches]); t.deepEqual(errors, [actionsutil.WorkflowErrors.MismatchedBranches]);
}); });
ava_1.default("validateWorkflow() when on.push is not mismatched", (t) => { ava_1.default("validateWorkflow() when on.push is not mismatched", (t) => {
const errors = actionsutil.validateWorkflow({ const errors = actionsutil.validateWorkflow({
@ -144,7 +144,7 @@ ava_1.default("validateWorkflow() when on.push is mismatched for pull_request",
pull_request: { branches: ["main", "feature"] }, pull_request: { branches: ["main", "feature"] },
}, },
}); });
t.deepEqual(errors, [actionsutil.ErrMismatchedBranches]); t.deepEqual(errors, [actionsutil.WorkflowErrors.MismatchedBranches]);
}); });
ava_1.default("validateWorkflow() when on.pull_request for every branch but push specifies branches", (t) => { ava_1.default("validateWorkflow() when on.pull_request for every branch but push specifies branches", (t) => {
const errors = actionsutil.validateWorkflow({ const errors = actionsutil.validateWorkflow({
@ -153,32 +153,32 @@ ava_1.default("validateWorkflow() when on.pull_request for every branch but push
pull_request: null, pull_request: null,
}, },
}); });
t.deepEqual(errors, [actionsutil.ErrMismatchedBranches]); t.deepEqual(errors, [actionsutil.WorkflowErrors.MismatchedBranches]);
}); });
ava_1.default("validateWorkflow() when HEAD^2 is checked out", (t) => { ava_1.default("validateWorkflow() when HEAD^2 is checked out", (t) => {
const errors = actionsutil.validateWorkflow({ const errors = actionsutil.validateWorkflow({
on: ["push", "pull_request"], on: ["push", "pull_request"],
jobs: { test: { steps: [{ run: "git checkout HEAD^2" }] } }, jobs: { test: { steps: [{ run: "git checkout HEAD^2" }] } },
}); });
t.deepEqual(errors, [actionsutil.ErrCheckoutWrongHead]); t.deepEqual(errors, [actionsutil.WorkflowErrors.CheckoutWrongHead]);
}); });
ava_1.default("formatWorkflowErrors() when there is one error", (t) => { ava_1.default("formatWorkflowErrors() when there is one error", (t) => {
const message = actionsutil.formatWorkflowErrors([ const message = actionsutil.formatWorkflowErrors([
actionsutil.ErrCheckoutWrongHead, actionsutil.WorkflowErrors.CheckoutWrongHead,
]); ]);
t.true(message.startsWith("1 issue was detected with this workflow:")); t.true(message.startsWith("1 issue was detected with this workflow:"));
}); });
ava_1.default("formatWorkflowErrors() when there are multiple errors", (t) => { ava_1.default("formatWorkflowErrors() when there are multiple errors", (t) => {
const message = actionsutil.formatWorkflowErrors([ const message = actionsutil.formatWorkflowErrors([
actionsutil.ErrCheckoutWrongHead, actionsutil.WorkflowErrors.CheckoutWrongHead,
actionsutil.ErrPathsSpecified, actionsutil.WorkflowErrors.PathsSpecified,
]); ]);
t.true(message.startsWith("2 issues were detected with this workflow:")); t.true(message.startsWith("2 issues were detected with this workflow:"));
}); });
ava_1.default("formatWorkflowCause()", (t) => { ava_1.default("formatWorkflowCause()", (t) => {
const message = actionsutil.formatWorkflowCause([ const message = actionsutil.formatWorkflowCause([
actionsutil.ErrCheckoutWrongHead, actionsutil.WorkflowErrors.CheckoutWrongHead,
actionsutil.ErrPathsSpecified, actionsutil.WorkflowErrors.PathsSpecified,
]); ]);
t.deepEqual(message, "CheckoutWrongHead,PathsSpecified"); t.deepEqual(message, "CheckoutWrongHead,PathsSpecified");
t.deepEqual(actionsutil.formatWorkflowCause(undefined), undefined); t.deepEqual(actionsutil.formatWorkflowCause(undefined), undefined);

File diff suppressed because one or more lines are too long

View file

@ -85,7 +85,7 @@ test("prepareEnvironment() when a local run", (t) => {
test("validateWorkflow() when on is missing", (t) => { test("validateWorkflow() when on is missing", (t) => {
const errors = actionsutil.validateWorkflow({}); const errors = actionsutil.validateWorkflow({});
t.deepEqual(errors, [actionsutil.ErrMissingHooks]); t.deepEqual(errors, [actionsutil.WorkflowErrors.MissingHooks]);
}); });
test("validateWorkflow() when on.push is missing", (t) => { test("validateWorkflow() when on.push is missing", (t) => {
@ -93,19 +93,19 @@ test("validateWorkflow() when on.push is missing", (t) => {
console.log(errors); console.log(errors);
t.deepEqual(errors, [actionsutil.ErrMissingHooks]); t.deepEqual(errors, [actionsutil.WorkflowErrors.MissingHooks]);
}); });
test("validateWorkflow() when on.push is an array missing pull_request", (t) => { test("validateWorkflow() when on.push is an array missing pull_request", (t) => {
const errors = actionsutil.validateWorkflow({ on: ["push"] }); const errors = actionsutil.validateWorkflow({ on: ["push"] });
t.deepEqual(errors, [actionsutil.ErrMissingPullRequestHook]); t.deepEqual(errors, [actionsutil.WorkflowErrors.MissingPullRequestHook]);
}); });
test("validateWorkflow() when on.push is an array missing push", (t) => { test("validateWorkflow() when on.push is an array missing push", (t) => {
const errors = actionsutil.validateWorkflow({ on: ["pull_request"] }); const errors = actionsutil.validateWorkflow({ on: ["pull_request"] });
t.deepEqual(errors, [actionsutil.ErrMissingPushHook]); t.deepEqual(errors, [actionsutil.WorkflowErrors.MissingPushHook]);
}); });
test("validateWorkflow() when on.push is valid", (t) => { test("validateWorkflow() when on.push is valid", (t) => {
@ -132,7 +132,7 @@ test("validateWorkflow() when on.push should not have a path", (t) => {
}, },
}); });
t.deepEqual(errors, [actionsutil.ErrPathsSpecified]); t.deepEqual(errors, [actionsutil.WorkflowErrors.PathsSpecified]);
}); });
test("validateWorkflow() when on.push is a correct object", (t) => { test("validateWorkflow() when on.push is a correct object", (t) => {
@ -161,7 +161,7 @@ test("validateWorkflow() when on.push is mismatched", (t) => {
}, },
}); });
t.deepEqual(errors, [actionsutil.ErrMismatchedBranches]); t.deepEqual(errors, [actionsutil.WorkflowErrors.MismatchedBranches]);
}); });
test("validateWorkflow() when on.push is not mismatched", (t) => { test("validateWorkflow() when on.push is not mismatched", (t) => {
@ -183,7 +183,7 @@ test("validateWorkflow() when on.push is mismatched for pull_request", (t) => {
}, },
}); });
t.deepEqual(errors, [actionsutil.ErrMismatchedBranches]); t.deepEqual(errors, [actionsutil.WorkflowErrors.MismatchedBranches]);
}); });
test("validateWorkflow() when on.pull_request for every branch but push specifies branches", (t) => { test("validateWorkflow() when on.pull_request for every branch but push specifies branches", (t) => {
@ -194,7 +194,7 @@ test("validateWorkflow() when on.pull_request for every branch but push specifie
}, },
}); });
t.deepEqual(errors, [actionsutil.ErrMismatchedBranches]); t.deepEqual(errors, [actionsutil.WorkflowErrors.MismatchedBranches]);
}); });
test("validateWorkflow() when HEAD^2 is checked out", (t) => { test("validateWorkflow() when HEAD^2 is checked out", (t) => {
@ -203,28 +203,28 @@ test("validateWorkflow() when HEAD^2 is checked out", (t) => {
jobs: { test: { steps: [{ run: "git checkout HEAD^2" }] } }, jobs: { test: { steps: [{ run: "git checkout HEAD^2" }] } },
}); });
t.deepEqual(errors, [actionsutil.ErrCheckoutWrongHead]); t.deepEqual(errors, [actionsutil.WorkflowErrors.CheckoutWrongHead]);
}); });
test("formatWorkflowErrors() when there is one error", (t) => { test("formatWorkflowErrors() when there is one error", (t) => {
const message = actionsutil.formatWorkflowErrors([ const message = actionsutil.formatWorkflowErrors([
actionsutil.ErrCheckoutWrongHead, actionsutil.WorkflowErrors.CheckoutWrongHead,
]); ]);
t.true(message.startsWith("1 issue was detected with this workflow:")); t.true(message.startsWith("1 issue was detected with this workflow:"));
}); });
test("formatWorkflowErrors() when there are multiple errors", (t) => { test("formatWorkflowErrors() when there are multiple errors", (t) => {
const message = actionsutil.formatWorkflowErrors([ const message = actionsutil.formatWorkflowErrors([
actionsutil.ErrCheckoutWrongHead, actionsutil.WorkflowErrors.CheckoutWrongHead,
actionsutil.ErrPathsSpecified, actionsutil.WorkflowErrors.PathsSpecified,
]); ]);
t.true(message.startsWith("2 issues were detected with this workflow:")); t.true(message.startsWith("2 issues were detected with this workflow:"));
}); });
test("formatWorkflowCause()", (t) => { test("formatWorkflowCause()", (t) => {
const message = actionsutil.formatWorkflowCause([ const message = actionsutil.formatWorkflowCause([
actionsutil.ErrCheckoutWrongHead, actionsutil.WorkflowErrors.CheckoutWrongHead,
actionsutil.ErrPathsSpecified, actionsutil.WorkflowErrors.PathsSpecified,
]); ]);
t.deepEqual(message, "CheckoutWrongHead,PathsSpecified"); t.deepEqual(message, "CheckoutWrongHead,PathsSpecified");

View file

@ -135,44 +135,37 @@ enum MissingTriggers {
PullRequest = 2, PullRequest = 2,
} }
interface WorkflowError { interface CodedError {
message: string; message: string;
code: string; code: string;
} }
export const ErrCheckoutWrongHead = { function toCodedErrors(errors: {
message: `Git checkout HEAD^2 is no longer necessary. Please remove this line.`, [key: string]: string;
code: "CheckoutWrongHead", }): { [key: string]: CodedError } {
}; return Object.entries(errors).reduce((acc, [key, value]) => {
export const ErrMismatchedBranches = { acc[key] = { message: value, code: key };
message: `Please make sure that every branch in on.pull_request is also in on.push so that CodeQL can establish a baseline.`, return acc;
code: "MismatchedBranches", }, {} as ReturnType<typeof toCodedErrors>);
}; }
export const ErrMissingHooks = {
message: `Please specify on.push and on.pull_request hooks.`,
code: "MissingHooks",
};
export const ErrMissingPushHook = {
message: `Please specify an on.push hook so CodeQL can establish a baseline.`,
code: "MissingPushHook",
};
export const ErrMissingPullRequestHook = {
message: `Please specify an on.pull_request hook so CodeQL is run against new pull requests.`,
code: "MissingPullRequestHook",
};
export const ErrPathsSpecified = {
message: `Please do not specify paths at on.pull.`,
code: "PathsSpecified",
};
export function validateWorkflow(doc: Workflow): WorkflowError[] { export const WorkflowErrors = toCodedErrors({
const errors: WorkflowError[] = []; MismatchedBranches: `Please make sure that every branch in on.pull_request is also in on.push so that CodeQL can establish a baseline.`,
MissingHooks: `Please specify on.push and on.pull_request hooks.`,
MissingPullRequestHook: `Please specify an on.pull_request hook so CodeQL is run against new pull requests.`,
MissingPushHook: `Please specify an on.push hook so CodeQL can establish a baseline.`,
PathsSpecified: `Please do not specify paths at on.pull.`,
CheckoutWrongHead: `Git checkout HEAD^2 is no longer necessary. Please remove this line.`,
});
export function validateWorkflow(doc: Workflow): CodedError[] {
const errors: CodedError[] = [];
// .jobs[key].steps[].run // .jobs[key].steps[].run
for (const job of Object.values(doc?.jobs || {})) { for (const job of Object.values(doc?.jobs || {})) {
for (const step of job?.steps || []) { for (const step of job?.steps || []) {
if (step?.run === "git checkout HEAD^2") { if (step?.run === "git checkout HEAD^2") {
errors.push(ErrCheckoutWrongHead); errors.push(WorkflowErrors.CheckoutWrongHead);
} }
} }
} }
@ -209,7 +202,7 @@ export function validateWorkflow(doc: Workflow): WorkflowError[] {
} else { } else {
const paths = doc.on.push?.paths; const paths = doc.on.push?.paths;
if (Array.isArray(paths) && paths.length > 0) { if (Array.isArray(paths) && paths.length > 0) {
errors.push(ErrPathsSpecified); errors.push(WorkflowErrors.PathsSpecified);
} }
} }
@ -222,32 +215,30 @@ export function validateWorkflow(doc: Workflow): WorkflowError[] {
(value) => !push.includes(value) (value) => !push.includes(value)
); );
if (intersects.length > 0) { if (intersects.length > 0) {
errors.push(ErrMismatchedBranches); errors.push(WorkflowErrors.MismatchedBranches);
} }
} else if (push.length > 0) { } else if (push.length > 0) {
errors.push(ErrMismatchedBranches); errors.push(WorkflowErrors.MismatchedBranches);
} }
} }
} }
switch (missing) { switch (missing) {
case MissingTriggers.PullRequest | MissingTriggers.Push: case MissingTriggers.PullRequest | MissingTriggers.Push:
errors.push(ErrMissingHooks); errors.push(WorkflowErrors.MissingHooks);
break; break;
case MissingTriggers.PullRequest: case MissingTriggers.PullRequest:
errors.push(ErrMissingPullRequestHook); errors.push(WorkflowErrors.MissingPullRequestHook);
break; break;
case MissingTriggers.Push: case MissingTriggers.Push:
errors.push(ErrMissingPushHook); errors.push(WorkflowErrors.MissingPushHook);
break; break;
} }
return errors; return errors;
} }
export async function getWorkflowErrors(): Promise< export async function getWorkflowErrors(): Promise<CodedError[] | undefined> {
WorkflowError[] | undefined
> {
const workflow = await getWorkflow(); const workflow = await getWorkflow();
if (workflow === undefined) { if (workflow === undefined) {
@ -263,15 +254,17 @@ export async function getWorkflowErrors(): Promise<
return workflowErrors; return workflowErrors;
} }
export function formatWorkflowErrors(errors: WorkflowError[]): string { export function formatWorkflowErrors(errors: CodedError[]): string {
return `${errors.length} issue${ const issuesWere = errors.length === 1 ? "issue was" : "issues were";
errors.length === 1 ? " was" : "s were"
} detected with this workflow: ${errors.map((e) => e.message).join(", ")}`; return `${
errors.length
} ${issuesWere} detected with this workflow: ${errors
.map((e) => e.message)
.join(", ")}`;
} }
export function formatWorkflowCause( export function formatWorkflowCause(errors?: CodedError[]): undefined | string {
errors?: WorkflowError[]
): undefined | string {
if (errors === undefined) { if (errors === undefined) {
return undefined; return undefined;
} }