diff --git a/.github/workflows/test-fail.yml b/.github/workflows/test-fail.yml deleted file mode 100644 index 843bf7fd..00000000 --- a/.github/workflows/test-fail.yml +++ /dev/null @@ -1,49 +0,0 @@ -name: Test that the action fails if benchmark increase exceeds threshold -on: - workflow_dispatch: - pull_request: -jobs: - run-action-exceed-threshold: - runs-on: ubuntu-latest - steps: - # run the test using the local action in repo - - uses: actions/checkout@v4 - - name: "build" - run: | - npm install - npm run build - - uses: ./ # local action in repo - # First add the data from part 1 - with: - name: 'Test fail if exceed threshold' - input-data-path: ./test/data/extract/testFailPart1.json - github-token: ${{ secrets.GITHUB_TOKEN }} - auto-push: true - gh-pages-branch: gh-pages-test - group-by: 'os' - schema: 'name,os' - - uses: ./ # local action in repo - # Then add the data from part 2, which represents - # more than a 200% increase. - with: - name: 'Test fail if exceed threshold' - input-data-path: ./test/data/extract/testFailPart2.json - github-token: ${{ secrets.GITHUB_TOKEN }} - auto-push: true - gh-pages-branch: gh-pages-test - group-by: 'os' - schema: 'name,os' - fail-on-alert: true - alert-threshold: 200% - # ensure that the above job fails - check-action-fails-on-exceed-threshold: - if: ${{ always() }} - runs-on: ubuntu-latest - needs: [run-action-exceed-threshold] - steps: - - name: Successful - expected failure but job exited successfully - if: ${{ !(contains(needs.*.result, 'failure')) }} - run: exit 1 - - name: Failing - expected failure - if: ${{ (contains(needs.*.result, 'failure')) }} - run: exit 0 diff --git a/src/default_index.html b/src/default_index.html index 50fce36f..a6f54efe 100644 --- a/src/default_index.html +++ b/src/default_index.html @@ -68,10 +68,19 @@ display: flex; gap: 25px; } + .checklist { + padding-right: 10px; + } .checklist-title { font-weight: bold; text-align: center; } + .select-all { + background: grey; + color: white; + margin-right: 2px; + width: 100%; + } .benchmark-set { margin: 8px 0; width: 100%; @@ -119,6 +128,9 @@
+ Number of charts displayed: +
+
@@ -393,8 +405,42 @@ elem.setAttribute(key.replace(' ', ''), value); }); } + function updateSelectAllStatus(add, selectAll, max) { + let numSelected = selectAll.getAttribute('num-selected'); + numSelected = numSelected ? Number(numSelected) : 0; + numSelected = numSelected ? numSelected : 0; + + if (add) { + numSelected += 1; + } else if (!add && numSelected === 0) { + numSelected = 0; + } else { + numSelected -= 1; + } + + selectAll.setAttribute('num-selected', numSelected); + + // set the state + if (numSelected === 0) { + selectAll.checked = false; + selectAll.indeterminate = false; + } else if (numSelected === max) { + selectAll.checked = true; + selectAll.indeterminate = false; + } else { + selectAll.checked = false; + selectAll.indeterminate = true; + } + } + + function updateNumShown(numShown) { + const elem = document.getElementById('num-charts-shown'); + elem.textContent = numShown; + } function setChartHiddenStatus(hide, key, value) { + value = value ? value : 'undefined'; + value = typeof value === 'number' ? value.toString() : value; const charts = document.getElementsByClassName('benchmark-graphs'); const filteredCharts = Array.from(charts).filter((chart) => { const chartValue = chart.getAttribute(key.replace(' ', '')); @@ -422,22 +468,52 @@ } chart.setAttribute('hidden-by', hiddenBy); }); + + // update number of charts hidden + const numShown = Array.from(charts).filter((chart) => { + return chart.style.getPropertyValue('display') !== 'none'; + }).length; + + updateNumShown(numShown); } - function getUniqueValuesForKey(groupKeys, groupBy) { - // get the unique values for each group + // get the unique values for each group + function getUniqueValuesByKey(groupKeys, groupBy) { const uniqueValues = {}; groupBy.forEach((key) => { - const keys = groupKeys.map((k) => k[key]); - const set = [...new Set(keys)]; - uniqueValues[key] = set; + uniqueValues[key] = [...new Set(groupKeys.map((k) => k[key]))]; }); return uniqueValues; } - function createFilterInterface(elem, groupKeys, groupBy) { - elem.className = 'filter-interface'; - const uniqueValues = getUniqueValuesForKey(groupKeys, groupBy); + function createFilterInterface(elem, uniqueValues) { + const fragment = document.createDocumentFragment(); + + // button to clear all filters + const clearFilters = document.getElementById('clear-filters-btn-div'); + clearFilters.innerHTML = ''; + const btn = document.createElement('button'); + btn.setAttribute('id', 'clear-filters-btn'); + btn.textContent = 'Clear all filters'; + btn.disabled = true; + btn.addEventListener('click', function () { + Object.entries(uniqueValues).forEach(([key, values]) => { + values.forEach((value) => { + const checkbox = document.getElementById(`${key}-${value}`); + checkbox.checked = true; + const hidden = false; + + // hide the chart + setChartHiddenStatus(hidden, key, value); + }); + const numSelected = values.length; + const selectAll = document.getElementById(`${key}-select-all`); + selectAll.setAttribute('num-selected', numSelected); + selectAll.checked = true; + selectAll.indeterminate = false; + }); + }); + clearFilters.appendChild(btn); Object.entries(uniqueValues).forEach(([key, values]) => { const checklist = document.createElement('div'); @@ -448,7 +524,43 @@ title.textContent = key; checklist.appendChild(title); + // select all / deselect all + const selectAllWrapper = document.createElement('div'); + selectAllWrapper.className = 'select-all'; + + const selectAll = document.createElement('input'); + selectAll.setAttribute('id', `${key}-select-all`); + selectAll.setAttribute('type', 'checkbox'); + const checkboxId = `${key}-select-all-checkbox`; + const selectAllLabel = document.createElement('label'); + selectAllLabel.setAttribute('for', checkboxId); + selectAllLabel.textContent = 'select all'; + selectAllWrapper.appendChild(selectAll); + selectAllWrapper.appendChild(selectAllLabel); + checklist.appendChild(selectAllWrapper); + + selectAll.addEventListener('change', function () { + values.forEach((value) => { + const checkbox = document.getElementById(`${key}-${value}`); + checkbox.checked = this.checked; + const hidden = !this.checked; + + // hide the chart + setChartHiddenStatus(hidden, key, value); + + // enable clear filters button + if (hidden) { + btn.disabled = false; + } + }); + const numSelected = this.checked ? values.length : 0; + selectAll.setAttribute('num-selected', numSelected); + }); + values.forEach((value) => { + // select all + updateSelectAllStatus(true, selectAll, values.length); + // generate the filter interface from the unique values const wrapper = document.createElement('div'); wrapper.className = 'checklist-entry'; @@ -476,11 +588,8 @@ checkBox.addEventListener('change', function () { const key = this.getAttribute('key'); const value = this.getAttribute('value'); - if (this.checked) { - setChartHiddenStatus(false, key, value); - } else { - setChartHiddenStatus(true, key, value); - } + setChartHiddenStatus(!this.checked, key, value); + updateSelectAllStatus(this.checked, selectAll, values.length); }); wrapper.appendChild(checkBox); wrapper.appendChild(label); @@ -488,8 +597,9 @@ checklist.appendChild(wrapper); }); - elem.appendChild(checklist); + fragment.appendChild(checklist); }); + elem.appendChild(fragment); } function populateRefPrDropdown(refsOpt, prsOpt) { const refs = refsOpt ? refsOpt : []; @@ -607,8 +717,10 @@ const groupKeys = Object.keys(groupedData).map((keyString) => parseKey(keyString, groupBy)); // create the interface at the top of the page for filtering - createFilterInterface(filterElem, groupKeys, groupBy); + const uniqueValues = getUniqueValuesByKey(groupKeys, groupBy); + createFilterInterface(filterElem, uniqueValues); + let numShown = 0; // create a chart for each group Object.entries(groupedData).forEach(([groupKeyString, filteredTraces]) => { // build the title @@ -622,11 +734,14 @@ const chartElem = chartFilteredTraces(setElem, title, filteredTraces); if (chartElem) { addAttributesToChartElement(chartElem, groupKey); + numShown += 1; } }); main.appendChild(filterElem); main.appendChild(setElem); + + updateNumShown(numShown); } async function checkFileAvailable(url) { const response = await fetch(url, { method: 'HEAD' }).catch((e) => { diff --git a/src/write.ts b/src/write.ts index 43e7492d..6ba24e43 100644 --- a/src/write.ts +++ b/src/write.ts @@ -108,16 +108,17 @@ function getComparePathAndSha(): [string, string] | undefined { return [comparePath, sha]; } -async function getPrevBench(benchName: string, config: Config): Promise { +async function getPrevBench(benchmarkRepoDir: string, config: Config): Promise { // TODO: error handling const comparePathAndSha = getComparePathAndSha(); if (!comparePathAndSha) { return null; } + const { basePath, name } = config; const [comparePath, compareSha] = comparePathAndSha; - const data = await loadDataJson(path.join(config.basePath, comparePath)); + const data = await loadDataJson(path.join(benchmarkRepoDir, basePath, comparePath)); - const suite = data.entries[benchName]; + const suite = data.entries[name]; if (suite === undefined) { return null; @@ -492,7 +493,11 @@ function isRemoteRejectedError(err: unknown): err is Error { return false; } -async function writeBenchmarkToGitHubPagesWithRetry(bench: Benchmark, config: Config, retry: number) { +async function writeBenchmarkToGitHubPagesWithRetry( + bench: Benchmark, + config: Config, + retry: number, +): Promise { const { name, ghPagesBranch, @@ -520,16 +525,16 @@ async function writeBenchmarkToGitHubPagesWithRetry(bench: Benchmark, config: Co } } - let benchmarkBaseDir = './'; + let benchmarkRepoDir = './'; let extraGitArguments: string[] = []; if (githubToken && !skipFetchGhPages && ghRepository) { - benchmarkBaseDir = './benchmark-data-repository'; - await git.clone(githubToken, ghRepository, benchmarkBaseDir); + benchmarkRepoDir = './benchmark-data-repository'; + await git.clone(githubToken, ghRepository, benchmarkRepoDir); rollbackActions.push(async () => { - await io.rmRF(benchmarkBaseDir); + await io.rmRF(benchmarkRepoDir); }); - extraGitArguments = [`--work-tree=${benchmarkBaseDir}`, `--git-dir=${benchmarkBaseDir}/.git`]; + extraGitArguments = [`--work-tree=${benchmarkRepoDir}`, `--git-dir=${benchmarkRepoDir}/.git`]; await git.checkout(ghPagesBranch, extraGitArguments); } else if (!skipFetchGhPages && (isPrivateRepo === false || githubToken)) { await git.pull(githubToken, ghPagesBranch); @@ -546,21 +551,22 @@ async function writeBenchmarkToGitHubPagesWithRetry(bench: Benchmark, config: Co githubToken: !!githubToken, }); } + const prevBench = await getPrevBench(benchmarkRepoDir, config); // `benchmarkDataDirPath` is an absolute path at this stage, - // so we need to convert it to relative to be able to prepend the `benchmarkBaseDir` + // so we need to convert it to relative to be able to prepend the `benchmarkRepoDir` const dataEntry = getDataEntry(); if (!dataEntry) { // sometimes we don't want to push the benchmark data (e.g. on the merge queue). // in this case, skip the below. console.warn('No data path could be built'); - return; + return prevBench; } let dataRelativePath = getDataPath(dataEntry); dataRelativePath = path.join(basePath, dataRelativePath); - const dataPath = path.join(benchmarkBaseDir, dataRelativePath); + const dataPath = path.join(benchmarkRepoDir, dataRelativePath); await io.mkdirP(path.dirname(dataPath)); @@ -571,13 +577,13 @@ async function writeBenchmarkToGitHubPagesWithRetry(bench: Benchmark, config: Co // handle the listing const listingPath = path.join(basePath, 'listing.json'); - const listing = await loadListing(benchmarkBaseDir, listingPath); + const listing = await loadListing(benchmarkRepoDir, listingPath); - await updateAndStoreListing(benchmarkBaseDir, listingPath, listing, dataEntry); + await updateAndStoreListing(benchmarkRepoDir, listingPath, listing, dataEntry); await git.cmd(extraGitArguments, 'add', listingPath); await git.cmd(extraGitArguments, 'add', dataRelativePath); - await addIndexHtmlIfNeeded(extraGitArguments, benchmarkBaseDir, path.join(basePath, 'index.html')); + await addIndexHtmlIfNeeded(extraGitArguments, benchmarkRepoDir, path.join(basePath, 'index.html')); await git.cmd(extraGitArguments, 'commit', '-m', `add ${name} benchmark result for ${bench.commit.id}`); if (githubToken && autoPush) { @@ -606,8 +612,7 @@ async function writeBenchmarkToGitHubPagesWithRetry(bench: Benchmark, config: Co core.warning( `Retrying to generate a commit and push to remote ${ghPagesBranch} with retry count ${retry}...`, ); - await writeBenchmarkToGitHubPagesWithRetry(bench, config, retry - 1); // Recursively retry - return; + return await writeBenchmarkToGitHubPagesWithRetry(bench, config, retry - 1); // Recursively retry } else { core.warning(`Failed to add benchmark data to '${name}' data: ${JSON.stringify(bench)}`); throw new Error( @@ -620,9 +625,11 @@ async function writeBenchmarkToGitHubPagesWithRetry(bench: Benchmark, config: Co `Auto-push to ${ghPagesBranch} is skipped because it requires both 'github-token' and 'auto-push' inputs`, ); } + + return prevBench; } -async function writeBenchmarkToGitHubPages(bench: Benchmark, config: Config) { +async function writeBenchmarkToGitHubPages(bench: Benchmark, config: Config): Promise { const { ghPagesBranch, skipFetchGhPages, ghRepository, githubToken } = config; if (!ghRepository) { if (!skipFetchGhPages) { @@ -631,8 +638,7 @@ async function writeBenchmarkToGitHubPages(bench: Benchmark, config: Config) { await git.cmd([], 'switch', ghPagesBranch); } try { - await writeBenchmarkToGitHubPagesWithRetry(bench, config, 10); - return; + return await writeBenchmarkToGitHubPagesWithRetry(bench, config, 10); } catch (e) { console.warn(e); throw e; @@ -678,8 +684,7 @@ export async function writeBenchmark(bench: Benchmark, config: Config) { prevBench = await writeBenchmarkToExternalJson(bench, externalDataJsonPath, config); } else { console.log('Writing to GitHub Pages'); - prevBench = await getPrevBench(name, config); - await writeBenchmarkToGitHubPages(bench, config); + prevBench = await writeBenchmarkToGitHubPages(bench, config); } // Put this after `git push` for reducing possibility to get conflict on push. Since sending diff --git a/test/data/write/benchmark-data-repository/data-dir/original_data.json b/test/data/write/benchmark-data-repository/data-dir/original_data.json index 271fc949..e0ce3b2b 100644 --- a/test/data/write/benchmark-data-repository/data-dir/original_data.json +++ b/test/data/write/benchmark-data-repository/data-dir/original_data.json @@ -16,7 +16,7 @@ }, "date": 1574927127603, "tool": "cargo", - "benches": [{ "name": "bench_fib_10", "range": "± 20", "unit": "ns/iter", "value": 100 }] + "benches": [{ "name": "bench_fib_10", "range": "± 20", "unit": "ns/iter", "value": 100, "os": "ubuntu-latest" }] } ] } diff --git a/test/data/write/benchmark-data-repository/data-dir/pr/original_data.json b/test/data/write/benchmark-data-repository/data-dir/pr/original_data.json new file mode 100644 index 00000000..d9d7bae0 --- /dev/null +++ b/test/data/write/benchmark-data-repository/data-dir/pr/original_data.json @@ -0,0 +1,23 @@ +{ + "lastUpdate": 1574927128603, + "repoUrl": "https://github.com/user/repo", + "entries": { + "Test benchmark": [ + { + "commit": { + "author": { "email": "dummy@example.com", "name": "User", "username": "user" }, + "committer": { "email": "dummy@example.com", "name": "User", "username": "user" }, + "distinct": false, + "id": "pr commit id", + "message": "dummy message", + "timestamp": "dummy stamp", + "tree_id": "dummy tree id", + "url": "https://github.com/user/repo/commit/pr commit id" + }, + "date": 1574927127604, + "tool": "cargo", + "benches": [{ "name": "bench_fib_10", "range": "± 20", "unit": "ns/iter", "value": 500, "os": "ubuntu-latest" }] + } + ] + } +} diff --git a/test/write.spec.ts b/test/write.spec.ts index 9e64bfe9..472994fd 100644 --- a/test/write.spec.ts +++ b/test/write.spec.ts @@ -920,6 +920,7 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe path.join('with-index-html', 'data.json'), path.join('benchmark-data-repository', 'data-dir', 'data.json'), path.join('benchmark-data-repository', 'data-dir', 'listing.json'), + path.join('benchmark-data-repository', 'data-dir', 'pr', '10.json'), path.join('benchmark-data-repository', 'data-dir', 'refs'), path.join('benchmark-data-repository', 'data-dir', 'index.html'), path.join('benchmark-data-repository', 'new-data-dir'), @@ -1369,6 +1370,57 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe `This comment was automatically generated by [workflow](${serverUrl}/user/repo/actions?query=workflow%3AWorkflow%20name) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`, ], }, + { + it: 'raises an alert when exceeding threshold 2.0 for push event with gh repository and base path', + config: { ...defaultCfg, ghRepository: 'https://github.com/user/other-repo' }, + added: { + commit: commit('current commit id'), + date: lastUpdate, + benches: [bench('bench_fib_10', 500), bench('bench_fib_20', 25000)], // Exceeds 2.0 threshold + bigger_is_better: false, + }, + gitServerUrl: serverUrl, + gitHistory: gitHistory(), + expectedDataBaseDirectory: 'benchmark-data-repository', + error: [ + '# :warning: **Performance Alert** :warning:', + '', + "Possible performance regression was detected for benchmark **'Test benchmark'**.", + 'Benchmark result of this commit is worse than the previous benchmark result exceeding threshold `2`.', + '', + '| Benchmark suite | Current: current commit id | Previous: prev commit id | Ratio |', + '|-|-|-|-|', + '| `bench_fib_10` | `500` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `5` |', + '', + `This comment was automatically generated by [workflow](${serverUrl}/user/repo/actions?query=workflow%3AWorkflow%20name) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`, + ], + }, + { + it: 'raises an alert when exceeding threshold 2.0 for merge group event with gh repository and base path', + config: { ...defaultCfg, ghRepository: 'https://github.com/user/other-repo' }, + payloadType: PayloadType.MergeGroup, + added: { + commit: commit('current commit id'), + date: lastUpdate, + benches: [bench('bench_fib_10', 500), bench('bench_fib_20', 25000)], // Exceeds 2.0 threshold + bigger_is_better: false, + }, + gitServerUrl: serverUrl, + gitHistory: gitHistory(), + expectedDataBaseDirectory: 'benchmark-data-repository', + error: [ + '# :warning: **Performance Alert** :warning:', + '', + "Possible performance regression was detected for benchmark **'Test benchmark'**.", + 'Benchmark result of this commit is worse than the previous benchmark result exceeding threshold `2`.', + '', + '| Benchmark suite | Current: current commit id | Previous: prev commit id | Ratio |', + '|-|-|-|-|', + '| `bench_fib_10` | `500` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `5` |', + '', + `This comment was automatically generated by [workflow](${serverUrl}/user/repo/actions?query=workflow%3AWorkflow%20name) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`, + ], + }, { it: 'raises an alert when exceeding threshold 2.0 for merge group event', config: defaultCfg, @@ -1419,6 +1471,32 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe `This comment was automatically generated by [workflow](${serverUrl}/user/repo/actions?query=workflow%3AWorkflow%20name) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`, ], }, + { + it: 'raises an alert when exceeding threshold 2.0 for pull request event with gh-repository and base path', + config: { ...defaultCfg, ghRepository: 'https://github.com/user/other-repo' }, + payloadType: PayloadType.PullRequest, + added: { + commit: commit('current commit id'), + date: lastUpdate, + benches: [bench('bench_fib_10', 500), bench('bench_fib_20', 25000)], // Exceeds 2.0 threshold + bigger_is_better: false, + }, + gitServerUrl: serverUrl, + gitHistory: gitHistory(), + expectedDataBaseDirectory: 'benchmark-data-repository', + error: [ + '# :warning: **Performance Alert** :warning:', + '', + "Possible performance regression was detected for benchmark **'Test benchmark'**.", + 'Benchmark result of this commit is worse than the previous benchmark result exceeding threshold `2`.', + '', + '| Benchmark suite | Current: current commit id | Previous: prev commit id | Ratio |', + '|-|-|-|-|', + '| `bench_fib_10` | `500` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `5` |', + '', + `This comment was automatically generated by [workflow](${serverUrl}/user/repo/actions?query=workflow%3AWorkflow%20name) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`, + ], + }, { it: 'writes data for pull request event', config: defaultCfg, @@ -1455,6 +1533,7 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe const originalDataJson = path.join(dataDirPath, 'original_data.json'); const indexHtml = path.join(dataDirPath, 'index.html'); + // copy data to append to/compare against if (await isFile(originalDataJson)) { await fs.mkdir(path.dirname(dataJs), { recursive: true }); await fs.copyFile(originalDataJson, dataJs);