Skip to content

Commit

Permalink
fix failure on fork PRs
Browse files Browse the repository at this point in the history
For after checkout, always use local cloning and do not checkout
afterBranch as it will be automatically present in the local clone.

The git comparator also had to be adopted to always use the sha
instead of the git branch for after.

Add unit test fork.sh.
  • Loading branch information
alexkli committed Apr 29, 2022
1 parent 7a4d505 commit c789b52
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 25 deletions.
33 changes: 18 additions & 15 deletions lib/checkout.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,10 @@ async function cloneLocalAndCheckout(from, to, branch) {
debug(`local cloning ${gitRoot} into ${to}`);
await Git().clone(gitRoot, to, { '--local' : true });

debug(`checking out '${branch}'...`);
await Git(to).checkout(branch);
if (branch) {
debug(`checking out '${branch}'...`);
await Git(to).checkout(branch);
}
}

async function gitCheckoutBeforeAndAfter(gitDir, beforeBranch, afterBranch) {
Expand All @@ -131,32 +133,33 @@ async function gitCheckoutBeforeAndAfter(gitDir, beforeBranch, afterBranch) {
const tempDir = tmp.dirSync({unsafeCleanup: true}).name;
debug(`temporary directory: ${tempDir}`);

// before checkout --------------------------------------------------------------

const beforeDir = path.join(tempDir, "before");

// we can use the faster local clone only if the "branch" is present locally
if (await hasBranch(gitDir, beforeBranch)) {
debug(`[before] using local cloning because existing checkout already has before branch '${beforeBranch}'`);
await cloneLocalAndCheckout(gitDir, beforeDir, beforeBranch);
} else {
debug(`[before] using remote cloning, as existing checkout does not have before branch '${beforeBranch}'`);
// ...otherwise need to do a full remote clone
await cloneRemoteAndCheckout(gitDir, beforeDir, beforeBranch);
}

// after checkout --------------------------------------------------------------

const afterDir = path.join(tempDir, "after");

// we can use the faster local clone only if the "branch" is present locally
if (await hasBranch(gitDir, afterBranch)) {
await cloneLocalAndCheckout(gitDir, afterDir, afterBranch);

try {
// need to ensure before branch is available under local name
await Git(afterDir).branch([beforeBranch, "-t", `origin/${beforeBranch}`]);
} catch (e) {
debug("ignoring error", e);
}
} else {
// ...otherwise need to do a full remote clone
await cloneRemoteAndCheckout(gitDir, afterDir, afterBranch);
debug(`[after] using local cloning`);
await cloneLocalAndCheckout(gitDir, afterDir);
try {
// need to ensure before branch is available under local name
await Git(afterDir).branch([beforeBranch, "-t", `origin/${beforeBranch}`]);
} catch (e) {
debug("ignoring error", e);
}
debug(`[after] current branch: ${await currentBranch(afterDir)}`);

debug("folders ready");

Expand Down
2 changes: 1 addition & 1 deletion lib/comparators/git.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ module.exports = {
compare: async function(before, after) {
const beforeSize = await getSize(before.dir);
const afterSize = await getSize(after.dir);
const details = await largestCommits(after.dir, before.branch, after.branch);
const details = await largestCommits(after.dir, before.branch, after.sha);

return {
beforeSize,
Expand Down
34 changes: 26 additions & 8 deletions test/checkout.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,28 @@ async function run(dir) {
assert.equal(await Git(after.dir).revparse(["HEAD"]), commitSha);
}

function cleanEnvVars() {
delete process.env.CI;
delete process.env.GITHUB_ACTIONS;
delete process.env.GITHUB_BASE_REF;
delete process.env.GITHUB_HEAD_REF;
delete process.env.TRAVIS;
delete process.env.TRAVIS_PULL_REQUEST_BRANCH;
delete process.env.TRAVIS_BRANCH;
delete process.env.CIRCLECI;
delete process.env.CIRCLE_BRANCH;
}

describe("checkout", function() {

this.captureConsole = true;

beforeEach(function() {
delete process.env.CI;
delete process.env.GITHUB_ACTIONS;
delete process.env.GITHUB_HEAD_REF;
delete process.env.TRAVIS;
delete process.env.TRAVIS_PULL_REQUEST_BRANCH;
delete process.env.TRAVIS_BRANCH;
delete process.env.CIRCLECI;
delete process.env.CIRCLE_BRANCH;
cleanEnvVars();
});

afterEach(function() {
cleanEnvVars();
});

it("handles normal checkouts", async function() {
Expand All @@ -74,4 +83,13 @@ describe("checkout", function() {
process.env.GITHUB_ACTIONS = true;
await run("test/checkout/githubactions");
});

it("handles github action fork PR checkouts", async function() {
process.env.CI = "true";
process.env.GITHUB_ACTIONS = true;
process.env.GITHUB_BASE_REF = "main";
// this branch name isn't actually set in test/checkout/githubactions/checkout.sh
process.env.GITHUB_HEAD_REF = "some-branch";
await run("test/checkout/githubactions");
});
});
19 changes: 18 additions & 1 deletion test/cli.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,24 @@ describe("cli e2e", function() {

assert.strictEqual(lastExitCode, undefined);
assert(this.output.stdout.includes("'main' => 'new'"));
assert(this.output.stdout.includes("+ ✅ git: -0.0%"));
assert(this.output.stdout.includes("+ ✅ git: -0."));
});

it("fork PR (github actions)", async function() {
process.env.CI = "true";
process.env.GITHUB_ACTIONS = true;
process.env.GITHUB_BASE_REF = "main";
// this branch name isn't actually set in the checkout steps in the script
process.env.GITHUB_HEAD_REF = "branch";

await exec(path.join(PROJECT_DIR, "test/scripts/fork.sh"));
process.chdir("checkout");

await sizewatcher();

assert.strictEqual(lastExitCode, undefined);
assert(this.output.stdout.includes("'main' => 'branch'"));
assert(this.output.stdout.includes("git:"));
});

});
40 changes: 40 additions & 0 deletions test/scripts/fork.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#!/bin/sh

# simulate a fork PR in github actions

mkdir remote
cd remote
REMOTE=$PWD

# setup new git repo
git init --initial-branch=main
# needed for git run in CI context where none of this is set
git config --local user.email "you@example.com"
git config --local user.name "Your Name"

echo "hello" > file
git add file
git commit -a -m "initial commit"

# new local branch
git checkout -b branch
echo "hello2" > file
git commit -a -m "branch commit"

# need the commit hash in the js code, and it's different each run
git rev-parse HEAD > ../commit.hash
SHA=$(cat ../commit.hash)

# checkout like github actions
# actions/checkout@v2 and v3

cd ..
mkdir checkout
cd checkout

git init
git remote add origin $REMOTE
git config --local gc.auto 0
git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 \
origin +$SHA:refs/remotes/pull/1/merge
git checkout --progress --force refs/remotes/pull/1/merge

0 comments on commit c789b52

Please sign in to comment.