diff --git a/README.md b/README.md index 9e446ee02..84ad615e7 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/src/write.ts b/src/write.ts index ac7283e37..4658d913b 100644 --- a/src/write.ts +++ b/src/write.ts @@ -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 { @@ -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, diff --git a/test/git.spec.ts b/test/git.spec.ts index 91e09b85b..bf8667f02 100644 --- a/test/git.spec.ts +++ b/test/git.spec.ts @@ -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: { @@ -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', + ]), + ); + }); + }); }); diff --git a/test/write.spec.ts b/test/write.spec.ts index f7be69acd..2b8a97f23 100644 --- a/test/write.spec.ts +++ b/test/write.spec.ts @@ -1,3 +1,4 @@ +import * as os from 'os'; import * as path from 'path'; import { promises as fs } from 'fs'; import * as cheerio from 'cheerio'; @@ -5,7 +6,8 @@ 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'; @@ -51,6 +53,7 @@ class GitSpy { } } const gitSpy = new GitSpy(); +const rmRFSpy: string[] = []; interface RepositoryPayloadSubset { private: boolean; @@ -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(); @@ -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); }); @@ -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'), @@ -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; @@ -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', }, { @@ -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', }, { @@ -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 }), + ]); + 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([]); }); });