Skip to content

Commit

Permalink
Merge pull request #74 from ktrz/fix/manual-and-scheduled-runs
Browse files Browse the repository at this point in the history 
Fix manual and scheduled runs
  • Loading branch information
@ktrz
ktrz authored Oct 12, 2021
2 parents cf314e1 + e834399 commit 14b2070
Show file tree
Hide file tree
Showing 6 changed files with 183 additions and 126 deletions.
99 changes: 74 additions & 25 deletions src/extract.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ interface Commit {
url: string;
}

interface PullRequest {
[key: string]: any;
number: number;
html_url?: string;
body?: string;
}

export interface Benchmark {
commit: Commit;
date: number;
Expand Down Expand Up @@ -138,39 +145,81 @@ function getHumanReadableUnitValue(seconds: number): [number, string] {
}
}

function getCommit(): Commit {
/* eslint-disable @typescript-eslint/camelcase */
if (github.context.payload.head_commit) {
return github.context.payload.head_commit;
}

const pr = github.context.payload.pull_request;
if (!pr) {
throw new Error(
`No commit information is found in payload: ${JSON.stringify(github.context.payload, null, 2)}`,
);
}

function getCommitFromPullRequestPayload(pr: PullRequest): Commit {
// On pull_request hook, head_commit is not available
const message: string = pr.title;
const id: string = pr.head.sha;
const timestamp: string = pr.head.repo.updated_at;
const url = `${pr.html_url}/commits/${id}`;
const name: string = pr.head.user.login;
const username: string = pr.head.user.login;
const user = {
name,
username: name, // XXX: Fallback, not correct
name: username, // XXX: Fallback, not correct
username,
};

return {
author: user,
committer: user,
id,
message,
timestamp,
url,
message: pr.title,
timestamp: pr.head.repo.updated_at,
url: `${pr.html_url}/commits/${id}`,
};
/* eslint-enable @typescript-eslint/camelcase */
}

async function getCommitFromGitHubAPIRequest(githubToken: string): Promise<Commit> {
const octocat = new github.GitHub(githubToken);

const { status, data } = await octocat.repos.getCommit({
owner: github.context.repo.owner,
repo: github.context.repo.repo,
ref: github.context.ref,
});

if (!(status === 200 || status === 304)) {
throw new Error(`Could not fetch the head commit. Received code: ${status}`);
}

const { commit } = data;

return {
author: {
name: commit.author.name,
username: data.author.login,
email: commit.author.email,
},
committer: {
name: commit.committer.name,
username: data.committer.login,
email: commit.committer.email,
},
id: data.sha,
message: commit.message,
timestamp: commit.author.date,
url: data.html_url,
};
}

async function getCommit(githubToken?: string): Promise<Commit> {
/* eslint-disable @typescript-eslint/camelcase */
if (github.context.payload.head_commit) {
return github.context.payload.head_commit;
}

const pr = github.context.payload.pull_request;

if (pr) {
return getCommitFromPullRequestPayload(pr);
}

if (!githubToken) {
throw new Error(
`No commit information is found in payload: ${JSON.stringify(
github.context.payload,
null,
2,
)}. Also, no 'github-token' provided, could not fallback to GitHub API Request.`,
);
}

return getCommitFromGitHubAPIRequest(githubToken);
}

function extractCargoResult(output: string): BenchmarkResult[] {
Expand Down Expand Up @@ -417,7 +466,7 @@ function extractCatch2Result(output: string): BenchmarkResult[] {

export async function extractResult(config: Config): Promise<Benchmark> {
const output = await fs.readFile(config.outputFilePath, 'utf8');
const { tool } = config;
const { tool, githubToken } = config;
let benches: BenchmarkResult[];

switch (tool) {
Expand Down Expand Up @@ -447,7 +496,7 @@ export async function extractResult(config: Config): Promise<Benchmark> {
throw new Error(`No benchmark result was found in ${config.outputFilePath}. Benchmark output was '${output}'`);
}

const commit = getCommit();
const commit = await getCommit(githubToken);

return {
commit,
Expand Down
11 changes: 2 additions & 9 deletions src/git.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,8 @@ export async function cmd(...args: string[]): Promise<string> {
}

function getRemoteUrl(token: string): string {
/* eslint-disable @typescript-eslint/camelcase */
const fullName = github.context.payload.repository?.full_name;
/* eslint-enable @typescript-eslint/camelcase */

if (!fullName) {
throw new Error(`Repository info is not available in payload: ${JSON.stringify(github.context.payload)}`);
}

return `https://x-access-token:${token}@github.com/${fullName}.git`;
const { repo, owner } = github.context.repo;
return `https://x-access-token:${token}@github.com/${owner}/${repo}.git`;
}

export async function push(token: string, branch: string, ...options: string[]): Promise<string> {
Expand Down
34 changes: 19 additions & 15 deletions src/write.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,16 @@ function findAlerts(curSuite: Benchmark, prevSuite: Benchmark, threshold: number
return alerts;
}

function getCurrentRepo() {
const repo = github.context.payload.repository;
if (!repo) {
throw new Error(
`Repository information is not available in payload: ${JSON.stringify(github.context.payload, null, 2)}`,
);
}
return repo;
function getCurrentRepoMetadata() {
const { repo, owner } = github.context.repo;
return {
name: repo,
owner: {
login: owner,
},
// eslint-disable-next-line @typescript-eslint/camelcase
html_url: `https://github.com/${owner}/${repo}`,
};
}

function floatStr(n: number) {
Expand All @@ -137,9 +139,9 @@ function strVal(b: BenchmarkResult): string {
}

function commentFooter(): string {
const repo = getCurrentRepo();
const repoMetadata = getCurrentRepoMetadata();
// eslint-disable-next-line @typescript-eslint/camelcase
const repoUrl = repo.html_url ?? '';
const repoUrl = repoMetadata.html_url ?? '';
const actionUrl = repoUrl + '/actions?query=workflow%3A' + encodeURIComponent(github.context.workflow);

return `This comment was automatically generated by [workflow](${actionUrl}) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`;
Expand Down Expand Up @@ -219,13 +221,13 @@ function buildAlertComment(
async function leaveComment(commitId: string, body: string, token: string) {
core.debug('Sending comment:\n' + body);

const repo = getCurrentRepo();
const repoMetadata = getCurrentRepoMetadata();
// eslint-disable-next-line @typescript-eslint/camelcase
const repoUrl = repo.html_url ?? '';
const repoUrl = repoMetadata.html_url ?? '';
const client = new github.GitHub(token);
const res = await client.repos.createCommitComment({
owner: repo.owner.login,
repo: repo.name,
owner: repoMetadata.owner.login,
repo: repoMetadata.name,
// eslint-disable-next-line @typescript-eslint/camelcase
commit_sha: commitId,
body,
Expand Down Expand Up @@ -313,7 +315,8 @@ function addBenchmarkToDataJson(
maxItems: number | null,
): Benchmark | null {
// eslint-disable-next-line @typescript-eslint/camelcase
const htmlUrl = github.context.payload.repository?.html_url ?? '';
const repoMetadata = getCurrentRepoMetadata();
const htmlUrl = repoMetadata.html_url ?? '';

let prevBench: Benchmark | null = null;
data.lastUpdate = Date.now();
Expand Down Expand Up @@ -369,6 +372,7 @@ async function writeBenchmarkToGitHubPagesWithRetry(
maxItemsInChart,
} = config;
const dataPath = path.join(benchmarkDataDirPath, 'data.js');
// FIXME: This payload is not available on `schedule:` or `workflow_dispatch:` events.
const isPrivateRepo = github.context.payload.repository?.private ?? false;

if (!skipFetchGhPages && (!isPrivateRepo || githubToken)) {
Expand Down
77 changes: 75 additions & 2 deletions test/extract.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,30 @@ const dummyWebhookPayload = {
url: 'https://github.com/dummy/repo',
},
} as { [key: string]: any };
let dummyCommitData = {};
class DummyGitHub {
repos = {
getCommit: () => {
return {
status: 200,
data: dummyCommitData,
};
},
};
}
const dummyGitHubContext = {
payload: dummyWebhookPayload,
repo: {
owner: 'dummy',
repo: 'repo',
},
ref: 'abcd1234',
};

mock('@actions/github', { context: dummyGitHubContext });
mock('@actions/github', {
context: dummyGitHubContext,
GitHub: DummyGitHub,
});

const { extractResult } = require('../src/extract');

Expand Down Expand Up @@ -320,7 +339,61 @@ describe('extractResult()', function() {
A.equal(commit.url, 'https://github.com/dummy/repo/pull/1/commits/abcdef0123456789');
});

it('raises an error when commit information is not found in webhook payload', async function() {
it('collects the commit information from current head via REST API as fallback when githubToken is provided', async function() {
dummyGitHubContext.payload = {};
dummyCommitData = {
author: {
login: 'testAuthorLogin',
},
committer: {
login: 'testCommitterLogin',
},
commit: {
author: {
name: 'test author',
date: 'author updated at timestamp',
email: 'author@testdummy.com',
},
committer: {
name: 'test committer',
// We use the `author.date` instead.
// date: 'committer updated at timestamp',
email: 'committer@testdummy.com',
},
message: 'test message',
},
sha: 'abcd1234',
html_url: 'https://github.com/dymmy/repo/commit/abcd1234',
};
const outputFilePath = path.join(__dirname, 'data', 'extract', 'go_output.txt');
const config = {
tool: 'go',
outputFilePath,
githubToken: 'abcd1234',
};

const { commit } = await extractResult(config);

const expectedCommit = {
id: 'abcd1234',
message: 'test message',
timestamp: 'author updated at timestamp',
url: 'https://github.com/dymmy/repo/commit/abcd1234',
author: {
name: 'test author',
username: 'testAuthorLogin',
email: 'author@testdummy.com',
},
committer: {
name: 'test committer',
username: 'testCommitterLogin',
email: 'committer@testdummy.com',
},
};
A.deepEqual(commit, expectedCommit);
});

it('raises an error when commit information is not found in webhook payload and no githubToken is provided', async function() {
dummyGitHubContext.payload = {};
const outputFilePath = path.join(__dirname, 'data', 'extract', 'go_output.txt');
const config = {
Expand Down
40 changes: 6 additions & 34 deletions test/git.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,14 @@ class FakedExec {

const fakedExec = new FakedExec();
const gitHubContext = {
payload: {
repository: {
full_name: 'user/repo',
},
repo: {
repo: 'repo',
owner: 'user',
},
} as {
payload: {
repository: {
full_name: string;
} | null;
repo: {
repo: string;
owner: string;
};
};

Expand Down Expand Up @@ -126,10 +124,6 @@ describe('git', function() {
});

describe('push()', function() {
afterEach(function() {
gitHubContext.payload.repository = { full_name: 'user/repo' };
});

it('runs `git push` with given branch and options', async function() {
const stdout = await push('this-is-token', 'my-branch', 'opt1', 'opt2');
const args = fakedExec.lastArgs;
Expand All @@ -149,22 +143,9 @@ describe('git', function() {
]),
);
});

it('raises an error when repository info is not included in payload', async function() {
gitHubContext.payload.repository = null;
await A.rejects(
() => push('this-is-token', 'my-branch', 'opt1', 'opt2'),
/^Error: Repository info is not available in payload/,
);
eq(fakedExec.lastArgs, null);
});
});

describe('pull()', function() {
afterEach(function() {
gitHubContext.payload.repository = { full_name: 'user/repo' };
});

it('runs `git pull` with given branch and options with token', async function() {
const stdout = await pull('this-is-token', 'my-branch', 'opt1', 'opt2');
const args = fakedExec.lastArgs;
Expand Down Expand Up @@ -193,14 +174,5 @@ describe('git', function() {
eq(args[0], 'git');
eq(args[1], userArgs.concat(['pull', 'origin', 'my-branch', 'opt1', 'opt2']));
});

it('raises an error when repository info is not included in payload', async function() {
gitHubContext.payload.repository = null;
await A.rejects(
() => pull('this-is-token', 'my-branch', 'opt1', 'opt2'),
/^Error: Repository info is not available in payload/,
);
eq(fakedExec.lastArgs, null);
});
});
});
Loading

0 comments on commit 14b2070

Please sign in to comment.