Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,8 @@ jobs:
The step which runs `github-action-benchmark` does followings:

1. Extract benchmark result from the output in `output.txt`
2. Switch branch to `gh-pages`
2. Switch branch to `gh-pages` (when `gh-repository` is set, the action instead shallow-clones only the
`gh-pages` branch of that repository)
3. Read existing benchmark results from `dev/bench/data.js`
4. Update `dev/bench/data.js` with the extracted benchmark result
5. Generate a commit to store the update in `gh-pages` branch
Expand Down
4 changes: 3 additions & 1 deletion src/write.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ async function storeDataJs(dataPath: string, data: DataJson) {
core.debug(`Overwrote ${dataPath} for adding new data`);
}

async function addIndexHtmlIfNeeded(additionalGitArguments: string[], dir: string, baseDir: string) {
export async function addIndexHtmlIfNeeded(additionalGitArguments: string[], dir: string, baseDir: string) {
const indexHtmlRelativePath = path.join(dir, 'index.html');
const indexHtmlFullPath = path.join(baseDir, indexHtmlRelativePath);
try {
Expand Down Expand Up @@ -367,6 +367,8 @@ async function writeBenchmarkToGitHubPagesWithRetry(

if (githubToken && !skipFetchGhPages && ghRepository) {
benchmarkBaseDir = './benchmark-data-repository';
// Shallow, single-branch clone: only the tip of the pages branch is needed. Full clones of large
// repositories can take 20+ minutes and widen the window for push contention with other runs.
await git.clone(
githubToken,
ghRepository,
Expand Down
36 changes: 35 additions & 1 deletion test/git.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { deepStrictEqual as eq, notDeepStrictEqual as neq, strict as A } from 'assert';
import { cmd, getServerUrl, pull, push, fetch } from '../src/git';
import { cmd, getServerUrl, pull, push, fetch, clone } from '../src/git';

interface ExecOptions {
listeners: {
Expand Down Expand Up @@ -217,4 +217,38 @@ describe('git', function () {
eq(args[1], userArgs.concat(['fetch', 'origin', 'my-branch:my-branch', 'opt1', 'opt2']));
});
});

describe('clone()', function () {
it('runs `git clone` with given repository, directory and options', async function () {
const stdout = await clone(
'this-is-token',
'github.com/user/other-repo',
'./benchmark-data-repository',
[],
'--branch',
'my-branch',
'--single-branch',
'--depth',
'1',
);
const args = fakedExec.lastArgs;

eq(stdout, 'this is test');
ok(args);
eq(args[0], 'git');
eq(
args[1],
userArgs.concat([
'clone',
'https://x-access-token:this-is-token@github.com/user/other-repo.git',
'./benchmark-data-repository',
'--branch',
'my-branch',
'--single-branch',
'--depth',
'1',
]),
);
});
});
});
223 changes: 122 additions & 101 deletions test/write.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import * as os from 'os';
import * as path from 'path';
import { promises as fs } from 'fs';
import * as cheerio from 'cheerio';
import markdownit from 'markdown-it';
import rimraf from 'rimraf';
import { Config } from '../src/config';
import { Benchmark } from '../src/extract';
import { DataJson, writeBenchmark } from '../src/write';
import { addIndexHtmlIfNeeded, DataJson, writeBenchmark } from '../src/write';
import { DEFAULT_INDEX_HTML } from '../src/default_index_html';
import { expect } from '@jest/globals';
import { FakedOctokit, fakedRepos } from './fakedOctokit';
import { wrapBodyWithBenchmarkTags } from '../src/comment/benchmarkCommentTags';
Expand Down Expand Up @@ -51,6 +53,7 @@ class GitSpy {
}
}
const gitSpy = new GitSpy();
const rmRFSpy: string[] = [];

interface RepositoryPayloadSubset {
private: boolean;
Expand Down Expand Up @@ -111,6 +114,12 @@ jest.mock('../src/git', () => ({
return '';
},
}));
jest.mock('@actions/io', () => ({
...jest.requireActual('@actions/io'),
async rmRF(inputPath: string) {
rmRFSpy.push(inputPath); // Record only; do not delete test fixtures on rollback
},
}));

describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBenchmark() - %s', function (serverUrl) {
const savedCwd = process.cwd();
Expand All @@ -123,6 +132,7 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe
jest.unmock('@actions/core');
jest.unmock('@actions/github');
jest.unmock('../src/git');
jest.unmock('@actions/io');
process.chdir(savedCwd);
});

Expand Down Expand Up @@ -936,6 +946,7 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe
});
afterEach(async function () {
gitSpy.clear();
rmRFSpy.length = 0;
delete (global as any).window;
for (const p of [
path.join('data-dir', 'data.js'),
Expand Down Expand Up @@ -1035,6 +1046,44 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe
return hist.filter((x: [GitFunc, unknown[]] | undefined): x is [GitFunc, unknown[]] => x !== undefined);
}

const otherRepoGitArgs = [
'--work-tree=./benchmark-data-repository',
'--git-dir=./benchmark-data-repository/.git',
];

function otherRepoGitHistory(cfg: { addIndexHtml?: boolean } = {}): [GitFunc, unknown[]][] {
const addIndexHtml = cfg.addIndexHtml ?? true;
const hist: Array<[GitFunc, unknown[]] | undefined> = [
[
'clone',
[
'dummy token',
'https://github.com/user/other-repo',
'./benchmark-data-repository',
[],
'--branch',
'gh-pages',
'--single-branch',
'--depth',
'1',
],
],
['cmd', [otherRepoGitArgs, 'add', path.join('data-dir', 'data.js')]],
addIndexHtml ? ['cmd', [otherRepoGitArgs, 'add', path.join('data-dir', 'index.html')]] : undefined,
[
'cmd',
[
otherRepoGitArgs,
'commit',
'-m',
'add Test benchmark (cargo) benchmark result for current commit id',
],
],
['push', ['dummy token', 'https://github.com/user/other-repo', 'gh-pages', otherRepoGitArgs]],
];
return hist.filter((x: [GitFunc, unknown[]] | undefined): x is [GitFunc, unknown[]] => x !== undefined);
}

const normalCases: Array<{
it: string;
config: Config;
Expand Down Expand Up @@ -1083,56 +1132,7 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe
benches: [bench('bench_fib_10', 135)],
},
gitServerUrl: serverUrl,
gitHistory: [
[
'clone',
[
'dummy token',
'https://github.com/user/other-repo',
'./benchmark-data-repository',
[],
'--branch',
'gh-pages',
'--single-branch',
'--depth',
'1',
],
],
[
'cmd',
[
['--work-tree=./benchmark-data-repository', '--git-dir=./benchmark-data-repository/.git'],
'add',
path.join('data-dir', 'data.js'),
],
],
[
'cmd',
[
['--work-tree=./benchmark-data-repository', '--git-dir=./benchmark-data-repository/.git'],
'add',
path.join('data-dir', 'index.html'),
],
],
[
'cmd',
[
['--work-tree=./benchmark-data-repository', '--git-dir=./benchmark-data-repository/.git'],
'commit',
'-m',
'add Test benchmark (cargo) benchmark result for current commit id',
],
],
[
'push',
[
'dummy token',
'https://github.com/user/other-repo',
'gh-pages',
['--work-tree=./benchmark-data-repository', '--git-dir=./benchmark-data-repository/.git'],
],
],
],
gitHistory: otherRepoGitHistory(),
expectedDataBaseDirectory: 'benchmark-data-repository',
},
{
Expand All @@ -1148,56 +1148,7 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe
benches: [bench('bench_fib_10', 135)],
},
gitServerUrl: serverUrl,
gitHistory: [
[
'clone',
[
'dummy token',
'https://github.com/user/other-repo',
'./benchmark-data-repository',
[],
'--branch',
'gh-pages',
'--single-branch',
'--depth',
'1',
],
],
[
'cmd',
[
['--work-tree=./benchmark-data-repository', '--git-dir=./benchmark-data-repository/.git'],
'add',
path.join('data-dir', 'data.js'),
],
],
[
'cmd',
[
['--work-tree=./benchmark-data-repository', '--git-dir=./benchmark-data-repository/.git'],
'add',
path.join('data-dir', 'index.html'),
],
],
[
'cmd',
[
['--work-tree=./benchmark-data-repository', '--git-dir=./benchmark-data-repository/.git'],
'commit',
'-m',
'add Test benchmark (cargo) benchmark result for current commit id',
],
],
[
'push',
[
'dummy token',
'https://github.com/user/other-repo',
'gh-pages',
['--work-tree=./benchmark-data-repository', '--git-dir=./benchmark-data-repository/.git'],
],
],
],
gitHistory: otherRepoGitHistory(),
expectedDataBaseDirectory: 'benchmark-data-repository',
},
{
Expand Down Expand Up @@ -1495,5 +1446,75 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe
ok(t.error.test(err.message), `'${err.message}' did not match to ${t.error}`);
}
});

it('re-clones other repository after rollback when push is rejected', async function () {
gitSpy.pushFailure = '... [remote rejected] ...';
gitSpy.pushFailureCount = 1;
const config = { ...defaultCfg, ghRepository: 'https://github.com/user/other-repo' };
const added: Benchmark = {
commit: commit('current commit id'),
date: lastUpdate,
tool: 'cargo',
benches: [bench('bench_fib_10', 135)],
};

const dataDirPath = path.join('benchmark-data-repository', config.benchmarkDataDirPath);
await fs.copyFile(path.join(dataDirPath, 'original_data.js'), path.join(dataDirPath, 'data.js'));

await writeBenchmark(added, config);

expect(gitSpy.history).toEqual([
...otherRepoGitHistory(),
['cmd', [otherRepoGitArgs, 'reset', '--hard', 'HEAD~1']],
// index.html created by the first attempt survives because rmRF is mocked, so it is not added again
// (recreating a missing index.html is covered by the addIndexHtmlIfNeeded() tests below)
...otherRepoGitHistory({ addIndexHtml: false }),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
]);
expect(rmRFSpy).toEqual(['./benchmark-data-repository']);
});
});
});

describe('addIndexHtmlIfNeeded()', function () {
const gitArgs = ['--work-tree=./repo', '--git-dir=./repo/.git'];
const dir = 'data-dir';
const indexHtmlRelativePath = path.join(dir, 'index.html');
let baseDir: string;

beforeEach(async function () {
baseDir = await fs.mkdtemp(path.join(os.tmpdir(), 'add-index-html-'));
await fs.mkdir(path.join(baseDir, dir));
});

afterEach(async function () {
gitSpy.clear();
await fs.rm(baseDir, { recursive: true, force: true });
});

it('creates and adds index.html when missing, and recreates it after it is removed', async function () {
const indexHtmlFullPath = path.join(baseDir, indexHtmlRelativePath);
const addIndexHtml = ['cmd', [gitArgs, 'add', indexHtmlRelativePath]];

await addIndexHtmlIfNeeded(gitArgs, dir, baseDir);

expect(await fs.readFile(indexHtmlFullPath, 'utf8')).toBe(DEFAULT_INDEX_HTML);
expect(gitSpy.history).toEqual([addIndexHtml]);

// Simulate a fresh re-clone that does not contain index.html
await fs.unlink(indexHtmlFullPath);
await addIndexHtmlIfNeeded(gitArgs, dir, baseDir);

expect(await fs.readFile(indexHtmlFullPath, 'utf8')).toBe(DEFAULT_INDEX_HTML);
expect(gitSpy.history).toEqual([addIndexHtml, addIndexHtml]);
});

it('neither overwrites nor adds an existing index.html', async function () {
const indexHtmlFullPath = path.join(baseDir, indexHtmlRelativePath);
await fs.writeFile(indexHtmlFullPath, 'custom index.html', 'utf8');

await addIndexHtmlIfNeeded(gitArgs, dir, baseDir);

expect(await fs.readFile(indexHtmlFullPath, 'utf8')).toBe('custom index.html');
expect(gitSpy.history).toEqual([]);
});
});
Loading