Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #348 from github/simon-engledew/checkout-wrong-heads
Only report the first CheckoutWrongHead lint error
Simon Engledew authored and GitHub committed Jan 4, 2021

Unverified

No user is associated with the committer email.
2 parents 230cb9b + 034bf31 commit b1e0b46
Showing 6 changed files with 128 additions and 10 deletions.
11 changes: 7 additions & 4 deletions lib/actions-util.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/actions-util.js.map

Large diffs are not rendered by default.

49 changes: 49 additions & 0 deletions lib/actions-util.test.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/actions-util.test.js.map

Large diffs are not rendered by default.

60 changes: 60 additions & 0 deletions src/actions-util.test.ts
@@ -336,6 +336,8 @@ test("validateWorkflow() when on.pull_request for mismatched wildcard branches",
});

test("validateWorkflow() when HEAD^2 is checked out", (t) => {
process.env.GITHUB_JOB = "test";

const errors = actionsutil.validateWorkflow({
on: ["push", "pull_request"],
jobs: { test: { steps: [{ run: "git checkout HEAD^2" }] } },
@@ -432,3 +434,61 @@ on:

t.deepEqual(errors, []);
});

test("validateWorkflow() should only report the current job's CheckoutWrongHead", (t) => {
process.env.GITHUB_JOB = "test";

const errors = actionsutil.validateWorkflow(
yaml.safeLoad(`
name: "CodeQL"
on:
push:
branches: [master]
pull_request:
# The branches below must be a subset of the branches above
branches: [master]
jobs:
test:
steps:
- run: "git checkout HEAD^2"
test2:
steps:
- run: "git checkout HEAD^2"
test3:
steps: []
`)
);

t.deepEqual(errors, [actionsutil.WorkflowErrors.CheckoutWrongHead]);
});

test("validateWorkflow() should not report a different job's CheckoutWrongHead", (t) => {
process.env.GITHUB_JOB = "test3";

const errors = actionsutil.validateWorkflow(
yaml.safeLoad(`
name: "CodeQL"
on:
push:
branches: [master]
pull_request:
# The branches below must be a subset of the branches above
branches: [master]
jobs:
test:
steps:
- run: "git checkout HEAD^2"
test2:
steps:
- run: "git checkout HEAD^2"
test3:
steps: []
`)
);

t.deepEqual(errors, []);
});
14 changes: 10 additions & 4 deletions src/actions-util.ts
@@ -211,17 +211,23 @@ export const WorkflowErrors = toCodedErrors({
export function validateWorkflow(doc: Workflow): CodedError[] {
const errors: CodedError[] = [];

// .jobs[key].steps[].run
for (const job of Object.values(doc?.jobs || {})) {
if (Array.isArray(job?.steps)) {
for (const step of job?.steps) {
const jobName = process.env.GITHUB_JOB;

if (jobName) {
const job = doc?.jobs?.[jobName];

const steps = job?.steps;

if (Array.isArray(steps)) {
for (const step of steps) {
// this was advice that we used to give in the README
// we actually want to run the analysis on the merge commit
// to produce results that are more inline with expectations
// (i.e: this is what will happen if you merge this PR)
// and avoid some race conditions
if (step?.run === "git checkout HEAD^2") {
errors.push(WorkflowErrors.CheckoutWrongHead);
break;
}
}
}

0 comments on commit b1e0b46

Please sign in to comment.