Skip to content

Commit

Permalink
Bug fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@pascalgn
pascalgn committed Feb 4, 2019
1 parent 1a3cd57 commit 9d65535
Show file tree
Hide file tree
Showing 4 changed files with 199 additions and 54 deletions.
118 changes: 75 additions & 43 deletions lib/api.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,18 @@ async function executeLocally(octokit, url, token) {
const m = url.match(URL_REGEXP);
if (m && m[3] === "pull") {
logger.debug("Getting PR data...");
const { data: pullRequest } = await octokit.pulls.get({
const { data: pull_request } = await octokit.pulls.get({
owner: m[1],
repo: m[2],
number: m[4]
});

await handlePullRequestUpdate(octokit, token, pullRequest);
const event = {
action: "opened",
pull_request
};

await executeGitHubAction(octokit, token, "pull_request", event);
} else if (m && m[3] === "tree") {
const event = {
ref: `refs/heads/${m[4]}`,
Expand All @@ -31,82 +36,75 @@ async function executeLocally(octokit, url, token) {
}
};

await handleBranchUpdate(octokit, token, event);
await executeGitHubAction(octokit, token, "push", event);
} else {
throw new ClientError(`invalid URL: ${url}`);
}
}

async function executeGitHubAction(octokit, token, eventName, eventData) {
logger.trace("Event name:", eventName);
logger.info("Event name:", eventName);
logger.trace("Event data:", eventData);

if (eventName === "push") {
await handleBranchUpdate(octokit, token, eventData);
await handleBranchUpdate(octokit, token, eventName, eventData);
} else if (eventName === "status") {
if (eventData.state === "success") {
await handleStatusUpdate(octokit, token, eventData);
} else {
logger.info("Event state ignored:", eventName, eventData.state);
throw new NeutralExitError();
}
await handleStatusUpdate(octokit, token, eventName, eventData);
} else if (eventName === "pull_request") {
if (RELEVANT_ACTIONS.includes(eventData.action)) {
await handlePullRequestUpdate(octokit, token, eventData.pull_request);
} else {
logger.info("Action ignored:", eventName, eventData.action);
throw new NeutralExitError();
}
await handlePullRequestUpdate(octokit, token, eventName, eventData);
} else if (eventName === "pull_request_review") {
if (eventData.action === "submitted") {
await handlePullRequestUpdate(octokit, token, eventData.pull_request);
} else {
logger.info("Action ignored:", eventName, eventData.action);
throw new NeutralExitError();
}
await handlePullRequestReviewUpdate(octokit, token, eventName, eventData);
} else {
throw new ClientError(`invalid event type: ${eventName}`);
}
}

async function handlePullRequestUpdate(octokit, token, pullRequest) {
logger.trace("PR:", pullRequest);

if (pullRequest.state !== "open") {
logger.info("PR is not open:", pullRequest.state);
async function handlePullRequestUpdate(octokit, token, eventName, event) {
const { action } = event;
if (!RELEVANT_ACTIONS.includes(action)) {
logger.info("Action ignored:", eventName, action);
throw new NeutralExitError();
}

const repo = pullRequest.head.repo.full_name;
const cloneUrl = `https://x-access-token:${token}@github.com/${repo}.git`;

const head = await tmpdir(path =>
update(octokit, path, cloneUrl, pullRequest)
);
await updateAndMergePullRequest(octokit, token, event.pull_request);
}

await merge(octokit, pullRequest, head);
async function handlePullRequestReviewUpdate(octokit, token, eventName, event) {
const { action } = event;
if (action === "submitted") {
await updateAndMergePullRequest(octokit, token, event.pull_request);
} else {
logger.info("Action ignored:", eventName, action);
throw new NeutralExitError();
}
}

async function handleStatusUpdate(octokit, token, event) {
logger.trace("Event:", event);
async function handleStatusUpdate(octokit, token, eventName, event) {
const { state } = event;
if (state !== "success") {
logger.info("Event state ignored:", eventName, state);
throw new NeutralExitError();
}

for (const branch of event.branches) {
logger.debug("Listing pull requests for", branch, "...");
logger.debug("Listing pull requests for", branch.name, "...");
const { data: pullRequests } = await octokit.pulls.list({
owner: event.repository.owner.login,
repo: event.repository.name,
state: "open",
head: `${event.repository.owner.login}:${branch}`,
head: `${event.repository.owner.login}:${branch.name}`,
sort: "updated",
direction: "desc",
per_page: MAX_PR_COUNT
});

logger.trace("PR list:", pullRequests);

let updated = 0;
for (const pullRequest of pullRequests) {
try {
await handlePullRequestUpdate(octokit, token, pullRequest);
await updateAndMergePullRequest(octokit, token, pullRequest);
++updated;
} catch (e) {
if (e instanceof NeutralExitError) {
logger.trace("PR update has been skipped.");
Expand All @@ -115,10 +113,31 @@ async function handleStatusUpdate(octokit, token, event) {
}
}
}

if (updated === 0) {
logger.info("No PRs have been updated");
throw new NeutralExitError();
}
}
}

async function handleBranchUpdate(octokit, token, event) {
async function updateAndMergePullRequest(octokit, token, pullRequest) {
if (pullRequest.state !== "open") {
logger.info("PR is not open:", pullRequest.state);
throw new NeutralExitError();
}

const repo = pullRequest.head.repo.full_name;
const cloneUrl = `https://x-access-token:${token}@github.com/${repo}.git`;

const head = await tmpdir(path =>
update(octokit, path, cloneUrl, pullRequest)
);

await merge(octokit, pullRequest, head);
}

async function handleBranchUpdate(octokit, token, eventName, event) {
const { ref } = event;
if (!ref.startsWith("refs/heads/")) {
logger.info("Push does not reference a branch:", ref);
Expand All @@ -141,14 +160,22 @@ async function handleBranchUpdate(octokit, token, event) {

logger.trace("PR list:", pullRequests);

logger.info("Open PRs:", pullRequests.length);
if (pullRequests.length > 0) {
logger.info("Open PRs:", pullRequests.length);
} else {
logger.info("No open PRs for", branch);
throw new NeutralExitError();
}

const repo = `${event.repository.owner.name}/${event.repository.name}`;
const cloneUrl = `https://x-access-token:${token}@github.com/${repo}.git`;

let updated = 0;

for (const pullRequest of pullRequests) {
try {
await tmpdir(path => update(octokit, path, cloneUrl, pullRequest));
updated++;
} catch (e) {
if (e instanceof NeutralExitError) {
logger.trace("PR update has been skipped.");
Expand All @@ -158,7 +185,12 @@ async function handleBranchUpdate(octokit, token, event) {
}
}

logger.info("PRs based on", branch, "have been updated");
if (updated > 0) {
logger.info(updated, "PRs based on", branch, "have been updated");
} else {
logger.info("No PRs based on", branch, "have been updated");
throw new NeutralExitError();
}
}

module.exports = { executeLocally, executeGitHubAction };
67 changes: 57 additions & 10 deletions lib/git.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ class ExitError extends Error {

const FETCH_DEPTH = 10;

const COMMON_ARGS = [
"-c",
"user.name=GitHub",
"-c",
"[email protected]"
];

function git(cwd, ...args) {
const stdio = [
"ignore",
Expand All @@ -23,7 +30,11 @@ function git(cwd, ...args) {
: `git ${args.join(" ")}`;
logger.debug("Executing", command);
return new Promise((resolve, reject) => {
const proc = spawn("git", args.filter(a => a !== null), { cwd, stdio });
const proc = spawn(
"git",
COMMON_ARGS.concat(args.filter(a => a !== null)),
{ cwd, stdio }
);
const buffers = [];
proc.stdout.on("data", data => buffers.push(data));
proc.on("error", () => {
Expand Down Expand Up @@ -72,10 +83,33 @@ async function fetch(dir, branch) {

async function fetchUntilMergeBase(dir, branch, timeout) {
const maxTime = new Date().getTime() + timeout;
const ref = `refs/remotes/origin/${branch}`;
while (new Date().getTime() < maxTime) {
const base = await mergeBase(dir, branch);
const base = await mergeBase(dir, "HEAD", ref);
if (base) {
return base;
const bases = [base];
const parents = await mergeCommits(dir, ref);
let fetchMore = false;
for (const parent of parents.flat()) {
const b = await mergeBase(dir, parent, ref);
if (b) {
if (!bases.includes(b)) {
bases.push(b);
}
} else {
// we found a commit which does not have a common ancestor with
// the branch we want to merge, so we need to fetch more
fetchMore = true;
break;
}
}
if (!fetchMore) {
const commonBase = await mergeBase(dir, ...bases);
if (!commonBase) {
throw new Error(`failed to find common base for ${bases}`);
}
return commonBase;
}
}
await fetchDeepen(dir);
}
Expand All @@ -86,14 +120,19 @@ async function fetchDeepen(dir) {
await git(dir, "fetch", "--quiet", "--deepen", FETCH_DEPTH);
}

async function mergeBase(dir, branch) {
async function mergeBase(dir, ...refs) {
if (refs.length === 1) {
return refs[0];
} else if (refs.length < 1) {
throw new Error("empty refs!");
}
let todo = refs;
try {
return await git(
dir,
"merge-base",
"HEAD",
`refs/remotes/origin/${branch}`
);
while (todo.length > 1) {
const base = await git(dir, "merge-base", todo[0], todo[1]);
todo = [base].concat(todo.slice(2));
}
return todo[0];
} catch (e) {
if (e instanceof ExitError && e.code === 1) {
return null;
Expand All @@ -103,6 +142,13 @@ async function mergeBase(dir, branch) {
}
}

async function mergeCommits(dir, ref) {
return (await git(dir, "rev-list", "--parents", `${ref}..HEAD`))
.split(/\n/g)
.map(line => line.split(/ /g).slice(1))
.filter(commit => commit.length > 1);
}

async function head(dir) {
return await git(dir, "show-ref", "--head", "-s", "/HEAD");
}
Expand Down Expand Up @@ -134,6 +180,7 @@ module.exports = {
fetchUntilMergeBase,
fetchDeepen,
mergeBase,
mergeCommits,
head,
sha,
rebase,
Expand Down
3 changes: 2 additions & 1 deletion lib/merge.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ const { NeutralExitError, logger } = require("./common");
async function merge(octokit, pullRequest, head) {
if (
pullRequest.mergeable_state !== "clean" &&
pullRequest.mergeable_state !== "unknown"
pullRequest.mergeable_state !== "unknown" &&
pullRequest.mergeable_state !== undefined
) {
logger.info("PR not ready to be merged:", pullRequest.mergeable_state);
throw new NeutralExitError();
Expand Down
65 changes: 65 additions & 0 deletions test/git.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,68 @@ test("fetchUntilMergeBase finds the correct merge base", async () => {
expect(await git.fetchUntilMergeBase(ws, "master", 10000)).toBe(base);
});
}, 15000);

test("fetchUntilMergeBase finds the earliest merge base 1", async () => {
await tmpdir(async path => {
const origin = `${path}/origin`;
await init(origin);
await commit(origin, "base %d", 10);
const base = await git.head(origin);
await git.git(origin, "branch", "br1");
await commit(origin, "master %d", 10);
await git.git(origin, "checkout", "br1");
await commit(origin, "br1 before merge %d", 5);
await git.git(origin, "merge", "--no-ff", "master");
await commit(origin, "br1 after merge %d", 10);
await git.git(origin, "checkout", "master");
await commit(origin, "master after merge %d", 10);

const ws = `${path}/ws`;
await git.clone(`file://${path}/origin`, ws, "br1");
await git.fetch(ws, "master");
expect(await git.fetchUntilMergeBase(ws, "master", 10000)).toBe(base);
});
}, 15000);

test("fetchUntilMergeBase finds the earliest merge base 2", async () => {
await tmpdir(async path => {
const origin = `${path}/origin`;
await init(origin);
await commit(origin, "base a%d", 5);
const base = await git.head(origin);
await commit(origin, "base b%d", 5);
await git.git(origin, "branch", "br1");
await commit(origin, "master %d", 10);
await git.git(origin, "checkout", "br1");
await commit(origin, "br1 before merge %d", 5);
await git.git(origin, "merge", "--no-ff", "master");
await commit(origin, "br1 after merge %d", 10);
await git.git(origin, "checkout", "master");
await commit(origin, "master after merge %d", 10);
await git.git(origin, "checkout", "-b", "br2", base);
await commit(origin, "br2");
await git.git(origin, "checkout", "br1");
await git.git(origin, "merge", "--no-ff", "br2");

const ws = `${path}/ws`;
await git.clone(`file://${path}/origin`, ws, "br1");
await git.fetch(ws, "master");
expect(await git.fetchUntilMergeBase(ws, "master", 10000)).toBe(base);
});
}, 15000);

test("mergeCommits returns the correct commits", async () => {
await tmpdir(async path => {
await init(path);
await commit(path, "master %d", 2);
const head1 = await git.head(path);
await git.git(path, "checkout", "-b", "branch", "HEAD^");
const head2 = await git.head(path);
await git.git(path, "merge", "--no-ff", "master");

const commits = await git.mergeCommits(path, "HEAD^");
expect(commits).toHaveLength(1);
expect(commits[0][0]).toBe(head2);
expect(commits[0][1]).toBe(head1);
});
});

0 comments on commit 9d65535

Please sign in to comment.