From 5cce1158e4aec07aa1fbe7b74151021e64f8e4db Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Thu, 10 Sep 2020 09:58:41 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20NEW:=20Add=20`render-json-path`=20o?= =?UTF-8?q?ption=20(#4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A JSON file, with configuration for how test suites and groups are rendered. Also, improve chart formatting. --- .github/workflows/ci.yml | 2 +- .github/workflows/pytest-config.json | 16 ++ .github/workflows/pytest.yml | 2 +- .gitignore | 1 + README.md | 64 +++++-- action.yml | 13 +- src/assets/benchmark.css | 195 +++++++++++++--------- src/assets/funcs.js | 56 ++++++- src/assets/index.html | 5 +- src/assets/main.js | 79 ++++++--- src/config.ts | 56 ++++--- src/extract.ts | 2 +- src/git.ts | 1 + src/write.ts | 37 ++-- test/config.ts | 50 ++++-- test/data/write/with-index-html/config.js | 1 + test/write.ts | 22 +-- 17 files changed, 401 insertions(+), 201 deletions(-) create mode 100644 .github/workflows/pytest-config.json create mode 100644 test/data/write/with-index-html/config.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 70bbed4e2..ff14d8163 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,10 +62,10 @@ jobs: with: name: Python Benchmark output-file-path: examples/pytest/output.json + render-json-path: .github/workflows/pytest-config.json skip-fetch-gh-pages: true fail-on-alert: true commit-msg-append: append - one-chart-groups: group1 - run: node ./scripts/ci_validate_modification.js before_data.js 'Python Benchmark' only-alert-with-cache: diff --git a/.github/workflows/pytest-config.json b/.github/workflows/pytest-config.json new file mode 100644 index 000000000..a7c4cc0a4 --- /dev/null +++ b/.github/workflows/pytest-config.json @@ -0,0 +1,16 @@ +{ + "suites": { + "Python Benchmark with pytest-benchmark": { + "header": "Test Suite Title", + "description": "Description of test suite." + } + }, + "groups": { + "group 1": { + "header": "Group 1 Title", + "description": "Description of group 1.", + "single_chart": true, + "xAxis": "id" + } + } +} diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 0812c0ce1..5caa8169e 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -22,10 +22,10 @@ jobs: with: name: Python Benchmark with pytest-benchmark output-file-path: examples/pytest/output.json + render-json-path: .github/workflows/pytest-config.json # Use personal access token instead of GITHUB_TOKEN due to https://github.community/t5/GitHub-Actions/Github-action-not-triggering-gh-pages-upon-push/td-p/26869/highlight/false github-token: ${{ secrets.GITHUB_TOKEN }} auto-push: true - one-chart-groups: group1 # Show alert with commit comment on detecting possible performance regression alert-threshold: '200%' comment-on-alert: true diff --git a/.gitignore b/.gitignore index 618097274..c0c7ed47c 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ /coverage .vscode/ src/assets/data.js +src/assets/config.js diff --git a/README.md b/README.md index bd687b8ad..5de346ee0 100644 --- a/README.md +++ b/README.md @@ -3,18 +3,51 @@ This is a fork of `github-action-benchmark`, optimised for `pytest-benchmark` only. It is hoped that eventually some/all of the features will be fed upstream: -1. Allow for tests to divided into groups (see [pytest-benchmark markers](https://pytest-benchmark.readthedocs.io/en/latest/usage.html#markers)), with sub-headings. -2. `one-chart-groups` allow for specific groups to be plotted on the same chart. -3. Capture of CPU information (shown in tooltip), useful for cross-referencing any changes in results against. -4. Add top-level `extra` key to captured data, which include the Python version. -5. Round values in tooltip to 5 significant figures -6. Add `chart-xaxis`, to allow chart x-axis to be commit date or ID -7. Add `commit-msg-append` option , useful for adding e.g. `[ci skip]` to commit message, but not having it in results titles -8. Removed capture of commit author/committer, since it can be obtained from the commit id/url -9. Split original HTML index text into files in `src/assets`folder, this allows for, -10. Add `npm run serve`, for local testing of results page -11. Add `overwrite-assets` option, as to whether these assets should be overridden. -12. Add `metadata` option, to capture additional information from the github action run (like dependency/docker versions). +1. More metadata is saved about the benchmark run, for later comparison: + - CPU data (cores, processors, speed) is specifically saved, per commit, in the `cpu` key (uses [systeminformation](https://www.npmjs.com/package/systeminformation) npm package). + - The python version is extracted from the pytest-benchmark data in an `extra` key + - A `metadata` action option is available to save additional data + - This data is all shown in the data point's tooltip + +2. The group name is saved for each test (see [pytest-benchmark markers](https://pytest-benchmark.readthedocs.io/en/latest/usage.html#markers)), or `null` if no group is given. + In the web-page rendering, tests are then arranged by the group they are in, which can be given a sub-heading, description, etc, and also a group can be "consolidated" into a single chart (with handling of differing data points). + +3. A new `render-json-path` allows for a JSON file to be copied, which is used to configure the rendered web-page, e.g. for certain test suites and groups: + + ```json + { + "suites": { + "name": { + "header": "Test Suite Title", + "description": "Description of test suite." + } + }, + "groups": { + "group1": { + "header": "Group 1 Title", + "description": "Description of group 1.", + "single_chart": true, + "xAxis": "date", + "backgroundFill": false + } + } + } + ``` + +4. Split original `index.html` into multiple HTML/JS files in the `src/assets` folder. This allows for easier testing and development of the output web-page. + This has additionally allowed for: + - Adding `npm run serve`, for local development of output web-page (using [light-server](https://www.npmjs.com/package/light-server)) + - Adding `overwrite-assets` option, to specify whether any existing assets should be overridden during a commit to `gh-pages`. + +5. Improve formatting of charts: + - Color cycling for consolidated charts + - For legend, extract common test name prefix as title + - Data point tooltips: rounding values to 5 significant figures and better formatting of dates etc. + - Addition of the `chartjs-plugin-zoom`. + +6. Add `commit-msg-append` option , useful for adding e.g. `[ci skip]` to commit message, but not having it as part of the test suite key in the data JSON. +7. Removed capture of commit author/committer, since it can be obtained from the commit id/url, and just bloats the data JSON. +8. Renamed `max-items-in-chart` -> `max-data-items`, to better describe its function of truncating the saved data during a commit. ## Development Notes @@ -29,12 +62,13 @@ You can also get the test coverage: `npm run coverage` To fix linting issues: `npm run fix` -For developing the webpage and associated JS/CSS, you can serve the assets: `npm serve`. -Note, that this requires you place a `data.js` object in `src/assets`. +For developing the webpage and associated JS/CSS, you can serve the assets: `npm run serve`. +Note, that this requires you place a `data.js` and `config.js` object in `src/assets`. To "release" a GH actions version, you need to create a branch containing all the compiled JS + node_modules. To do this (or to update a release branch), first make sure the branch has been created, -then you can run the script: `./scripts/prepare-release.sh branch-name`. +then you can run the script: `./scripts/prepare-release.sh branch-name`, usually with a branch named e.g. `v2`. +After this the branch should be tagged. --- diff --git a/action.yml b/action.yml index b446b19a1..6eba7a20b 100644 --- a/action.yml +++ b/action.yml @@ -66,17 +66,12 @@ inputs: external-data-json-path: description: 'JSON data file for storing benchmark results. When this input is set, github-action-benchmark no longer uses Git branch to store data. Instead, it reads and appends benchmark data from/to the file. User must store the file anywhere' required: false - max-items-in-chart: - description: 'Max data points in a benchmark chart to avoid making the chart too busy. Value must be unsigned integer. No limit by default' + max-data-items: + description: 'Max data points to store per test suite. No limit by default, otherwise data will be truncated.' required: false - chart-xaxis: - description: 'X-axis labels to use, commit "id" or "date"' + render-json-path: + description: 'A path to a file which contains the configuration JSON for rendering results.' required: false - default: id - one-chart-groups: - description: 'Benchmark groups (comma-delimited) whose tests should be plotted together on a single chart' - required: false - default: '' overwrite-assets: description: 'Whether to overwrite existing HTML/JS/CSS files on GitHub Pages' required: false diff --git a/src/assets/benchmark.css b/src/assets/benchmark.css index 712608b25..98df2b4f3 100644 --- a/src/assets/benchmark.css +++ b/src/assets/benchmark.css @@ -1,84 +1,113 @@ html { - font-family: BlinkMacSystemFont,-apple-system,"Segoe UI",Roboto,Oxygen,Ubuntu,Cantarell,"Fira Sans","Droid Sans","Helvetica Neue",Helvetica,Arial,sans-serif; - -webkit-font-smoothing: antialiased; - background-color: #fff; - font-size: 16px; - } - body { - color: #4a4a4a; - margin: 8px; - font-size: 1em; - font-weight: 400; - } - header { - margin-bottom: 8px; - display: flex; - flex-direction: column; - } - main { - width: 100%; - display: flex; - flex-direction: column; - } - a { - color: #3273dc; - cursor: pointer; - text-decoration: none; - } - a:hover { - color: #000; - } - button { - color: #fff; - background-color: #3298dc; - border-color: transparent; - cursor: pointer; - text-align: center; - } - button:hover { - background-color: #2793da; - flex: none; - } - .spacer { - flex: auto; - } - .small { - font-size: 0.75rem; - } - footer { - margin-top: 16px; - display: flex; - align-items: center; - } - .header-label { - margin-right: 4px; - } - .benchmark-set { - margin: 8px 0; - width: 100%; - display: flex; - flex-direction: column; - } - .benchmark-title { - font-size: 3rem; - font-weight: 600; - word-break: break-word; - text-align: center; - } - .benchmark-group { - font-size: 2rem; - font-weight: 400; - word-break: break-word; - text-align: center; - } - .benchmark-graphs { - display: flex; - flex-direction: row; - justify-content: space-around; - align-items: center; - flex-wrap: wrap; - width: 100%; - } - .benchmark-chart { - max-width: 1000px; - } + font-family: BlinkMacSystemFont, -apple-system, "Segoe UI", Roboto, Oxygen, Ubuntu, Cantarell, "Fira Sans", "Droid Sans", "Helvetica Neue", Helvetica, Arial, sans-serif; + -webkit-font-smoothing: antialiased; + background-color: #fff; + font-size: 16px; +} + +body { + color: #4a4a4a; + margin: 8px; + font-size: 1em; + font-weight: 400; +} + +header { + margin-bottom: 8px; + display: flex; + flex-direction: column; +} + +main { + width: 100%; + display: flex; + flex-direction: column; +} + +a { + color: #3273dc; + cursor: pointer; + text-decoration: none; +} + +a:hover { + color: #000; +} + +button { + color: #fff; + background-color: #3298dc; + border-color: transparent; + cursor: pointer; + text-align: center; +} + +button:hover { + background-color: #2793da; + flex: none; +} + +.spacer { + flex: auto; +} + +.small { + font-size: 0.75rem; +} + +footer { + margin-top: 16px; + display: flex; + align-items: center; +} + +.header-label { + margin-right: 4px; +} + +.benchmark-set { + margin: 8px 0; + width: 100%; + display: flex; + flex-direction: column; +} + +.benchmark-title { + font-size: 3rem; + font-weight: 600; + word-break: break-word; + text-align: center; +} + +.benchmark-description { + font-size: 1rem; + text-align: center; +} + +.benchmark-group-title { + font-size: 2rem; + font-weight: 400; + word-break: break-word; + text-align: left; + margin-left: 2%; +} + +.benchmark-group-description { + font-size: 1rem; + text-align: left; + margin-left: 2%; + margin-bottom: 2rem; +} + +.benchmark-graphs { + display: flex; + flex-direction: row; + justify-content: space-around; + align-items: center; + flex-wrap: wrap; + width: 100%; +} + +.benchmark-chart { + max-width: 1000px; +} diff --git a/src/assets/funcs.js b/src/assets/funcs.js index 9b82b9892..c918ec2e3 100644 --- a/src/assets/funcs.js +++ b/src/assets/funcs.js @@ -1,5 +1,8 @@ +// DOM independent functions module + 'use strict'; +// convert the entries to to a map: {group-key: {test-name: data, ...}, ...} export function collectBenchesPerTestCasePerGroup(entries) { const map = new Map(); for (const entry of entries) { @@ -30,23 +33,45 @@ function* cycle(iterable) { } } -export function renderGraph(canvas, dataset, labels, xAxis, alpha = 60, labelString = 'iter/sec') { + +var longestCommonPrefix = function(strs) { + let prefix = "" + if(strs === null || strs.length === 0) return prefix + + for (let i=0; i < strs[0].length; i++){ + const char = strs[0][i] // loop through all characters of the very first string. + + for (let j = 1; j < strs.length; j++){ + // loop through all other strings in the array + if(strs[j][i] !== char) return prefix + } + prefix = prefix + char + } + + return prefix +} + + +export function renderGraph(canvas, dataset, labels, xAxis, alpha = 60, fill = true, labelString = 'iter/sec') { const colorCycle = cycle(['#1f77b4', '#ff7f0e', '#2ca02c', '#d62728', '#9467bd', '#8c564b', '#e377c2', '#7f7f7f', '#bcbd22', '#17becf']); + // if all test names start with a common prefix, then show this as a title + const tests_prefix = dataset.length > 1 ? longestCommonPrefix(dataset.map(s => s.name)): '' + const data = { labels, datasets: dataset.map(s => { const color = colorCycle.next().value; return { - label: s.name, + label: s.name.slice(tests_prefix.length), data: s.data.map(d => d ? d.bench.value : null), borderColor: color, - // fill: false, + fill: fill, backgroundColor: color + `${alpha}`, // Add alpha for #rrggbbaa spanGaps: true, } - }) + }).sort((a, b) => { return a.label > b.label; }) }; const xAxes = { scaleLabel: { @@ -75,6 +100,18 @@ export function renderGraph(canvas, dataset, labels, xAxis, alpha = 60, labelStr xAxes: [xAxes], yAxes: [yAxes], }, + title: { + position: 'top', + text: tests_prefix, + display: tests_prefix.length > 0, + }, + legend: { + position: 'top', + align: 'center', + // TODO legend titles only available in chartjs v3 + // rather then chart title + // title: {text: 'hallo', display: true}, + }, tooltips: { callbacks: { afterTitle: items => { @@ -122,6 +159,17 @@ export function renderGraph(canvas, dataset, labels, xAxis, alpha = 60, labelStr const url = data.commit.url; window.open(url, '_blank'); }, + plugins: { + zoom: { + // pan: { + // enabled: true + // }, + zoom: { + enabled: true, + drag: true + } + } + } }; new Chart(canvas, { type: 'line', diff --git a/src/assets/index.html b/src/assets/index.html index 3aaccace2..53b6e42be 100644 --- a/src/assets/index.html +++ b/src/assets/index.html @@ -28,8 +28,11 @@ - + + + + diff --git a/src/assets/main.js b/src/assets/main.js index 33df8e16a..7c5094b37 100644 --- a/src/assets/main.js +++ b/src/assets/main.js @@ -1,10 +1,16 @@ +// The main interface to the page DOM + 'use strict'; import { collectBenchesPerTestCasePerGroup, renderGraph } from "./funcs.js"; (function () { const data = setupData(window.BENCHMARK_DATA); - renderAllCharts(data); + const config = { + suites: window.CONFIGURATION_DATA.suites || {}, + groups: window.CONFIGURATION_DATA.groups || {}, + }; + renderAllCharts(data, config); })(); function setupData(data) { @@ -27,35 +33,59 @@ function setupData(data) { // Prepare data points for charts return Object.keys(data.entries).map(name => ({ name, - xAxis: data.xAxis, - oneChartGroups: data.oneChartGroups, dataSet: collectBenchesPerTestCasePerGroup(data.entries[name]), })); } -function renderBenchSetGroups(name, benchSets, main, xAxis, oneChartGroups) { +/** + * Returns the sum of all numbers passed to the function. + * @param name name of the entry set + * @param Object benchSets map of {group-key: data, ...} + * @param Object configEntry map of configuration for the entry + * @param Object configGroups map of configuration per group: {group-key: data, ...} + */ +function renderBenchSetGroups(domElement, name, benchSets, configEntry, configGroups) { const setElem = document.createElement('div'); setElem.className = 'benchmark-set'; - main.appendChild(setElem); + domElement.appendChild(setElem); + + // add the entry title and description + const titleElem = document.createElement('h1'); + titleElem.className = 'benchmark-title'; + titleElem.textContent = configEntry.header || name; + setElem.appendChild(titleElem); + if (configEntry.description) { + const descriptElem = document.createElement('p'); + descriptElem.className = 'benchmark-description'; + descriptElem.textContent = configEntry.description; + setElem.appendChild(descriptElem); + } - const nameElem = document.createElement('h1'); - nameElem.className = 'benchmark-title'; - nameElem.textContent = name; - setElem.appendChild(nameElem); + for (const [groupKey, benchSet] of benchSets.entries()) { - for (const [groupName, benchSet] of benchSets.entries()) { + const configGroup = configGroups[groupKey] || {}; + const groupHeader = configGroup.header || groupKey; - if (groupName) { + // add a group title and description + if (groupHeader) { const nameElem = document.createElement('h2'); - nameElem.className = 'benchmark-group'; - nameElem.textContent = groupName + nameElem.className = 'benchmark-group-title'; + nameElem.textContent = groupHeader setElem.appendChild(nameElem); } + if (configGroup.description) { + const descriptElem = document.createElement('p'); + descriptElem.className = 'benchmark-group-description'; + descriptElem.textContent = configGroup.description; + setElem.appendChild(descriptElem); + } + + // add a container for the group graphs const graphsElem = document.createElement('div'); graphsElem.className = 'benchmark-graphs'; setElem.appendChild(graphsElem); - if ((oneChartGroups ? oneChartGroups : []).includes(groupName)) { + if (configGroup.single_chart) { const canvas = document.createElement('canvas'); canvas.className = 'benchmark-chart'; graphsElem.appendChild(canvas); @@ -67,8 +97,9 @@ function renderBenchSetGroups(name, benchSets, main, xAxis, oneChartGroups) { data: uniqueCommits.map(c => lookup.get(c[1])), }; }); - const labels = uniqueCommits.map((d) => (xAxis === 'date') ? moment(d[0]) : d[1].slice(0, 7)); - renderGraph(canvas, datasets, labels, xAxis, 10) + const labels = uniqueCommits.map((d) => (configGroup.xAxis === 'date') ? moment(d[0]) : d[1].slice(0, 7)); + const fill = configGroup.backgroundFill === undefined ? true : configGroup.backgroundFill + renderGraph(canvas, datasets, labels, configGroup.xAxis, 10, fill) } else { for (const [benchName, benches] of benchSet.entries()) { const canvas = document.createElement('canvas'); @@ -78,18 +109,24 @@ function renderBenchSetGroups(name, benchSets, main, xAxis, oneChartGroups) { name: benchName, data: benches }] - const labels = benches.map((d) => (xAxis === 'date') ? moment(d.commit.timestamp) : d.commit.id.slice(0, 7)); + const labels = benches.map((d) => (configGroup.xAxis === 'date') ? moment(d.commit.timestamp) : d.commit.id.slice(0, 7)); const labelString = benches.length > 0 ? benches[0].bench.unit : ''; - renderGraph(canvas, datasets, labels, xAxis, 60, labelString); + renderGraph(canvas, datasets, labels, configGroup.xAxis, 60, labelString); } } } } -function renderAllCharts(dataSets) { +/** + * Returns the sum of all numbers passed to the function. + * @param Object dataSets map of {group-key: {test-name: data, ...}, ...} + * @param Object config map of {suites: {key: data}, groups: {key: data}} + */ +function renderAllCharts(dataSets, config) { const main = document.getElementById('main'); - for (const { name, dataSet, xAxis, oneChartGroups } of dataSets) { - renderBenchSetGroups(name, dataSet, main, xAxis, oneChartGroups); + for (const { name, dataSet } of dataSets) { + const configEntry = config.suites[name] || {}; + renderBenchSetGroups(main, name, dataSet, configEntry, config.groups); } } diff --git a/src/config.ts b/src/config.ts index 1330264eb..6dbd7c342 100644 --- a/src/config.ts +++ b/src/config.ts @@ -20,9 +20,8 @@ export interface Config { failThreshold: number; alertCommentCcUsers: string[]; externalDataJsonPath: string | undefined; - maxItemsInChart: number | null; - chartXAxis: 'id' | 'date'; - oneChartGroups: string[]; + configDataJsonPath: string | undefined; + maxItemsInSuite: number | null; overwriteAssets: boolean; metadata: string; } @@ -65,6 +64,21 @@ async function validateOutputFilePath(filePath: string): Promise { } } +async function validateConfigFilePath(filePath: string | undefined): Promise { + if (!filePath) { + return Promise.resolve(undefined); + } + try { + const path = await resolveFilePath(filePath); + const content = await fs.readFile(path, 'utf8'); + JSON.parse(content); + // TODO validate against JSON schema + return path; + } catch (err) { + throw new Error(`Invalid value for 'render-json-path' input: ${err}`); + } +} + function validateGhPagesBranch(branch: string) { if (branch) { return; @@ -181,9 +195,9 @@ function getUintInput(name: string): number | null { return i; } -function validateMaxItemsInChart(max: number | null) { +function validateMaxItemsInSuite(max: number | null) { if (max !== null && max <= 0) { - throw new Error(`'max-items-in-chart' input value must be one or more but got ${max}`); + throw new Error(`'max-data-items' input value must be one or more but got ${max}`); } } @@ -198,15 +212,15 @@ function validateAlertThreshold(alertThreshold: number | null, failThreshold: nu } } -function validateChartXAxis(chartXAxis: string): 'id' | 'date' { - if (chartXAxis === 'id') { - return chartXAxis; - } - if (chartXAxis === 'date') { - return chartXAxis; - } - throw new Error(`'chart-xaxis' value must be 'id' or 'date' but got ${chartXAxis}`); -} +// function validateChartXAxis(chartXAxis: string): 'id' | 'date' { +// if (chartXAxis === 'id') { +// return chartXAxis; +// } +// if (chartXAxis === 'date') { +// return chartXAxis; +// } +// throw new Error(`'chart-xaxis' value must be 'id' or 'date' but got ${chartXAxis}`); +// } export async function configFromJobInput(): Promise { let outputFilePath: string = core.getInput('output-file-path'); @@ -223,10 +237,9 @@ export async function configFromJobInput(): Promise { const alertThreshold = getPercentageInput('alert-threshold'); const failOnAlert = getBoolInput('fail-on-alert'); const alertCommentCcUsers = getCommaSeparatedInput('alert-comment-cc-users'); + const maxItemsInSuite = getUintInput('max-data-items'); let externalDataJsonPath: undefined | string = core.getInput('external-data-json-path'); - const maxItemsInChart = getUintInput('max-items-in-chart'); - const chartXAxisValue = core.getInput('chart-xaxis'); - const oneChartGroups = core.getInput('one-chart-groups').split(','); + let configDataJsonPath: undefined | string = core.getInput('render-json-path'); let failThreshold = getPercentageInput('fail-threshold'); const overwriteAssets = getBoolInput('overwrite-assets'); const metadata = core.getInput('metadata'); @@ -247,11 +260,11 @@ export async function configFromJobInput(): Promise { validateAlertThreshold(alertThreshold, failThreshold); validateAlertCommentCcUsers(alertCommentCcUsers); externalDataJsonPath = await validateExternalDataJsonPath(externalDataJsonPath, autoPush); - validateMaxItemsInChart(maxItemsInChart); + configDataJsonPath = await validateConfigFilePath(configDataJsonPath); + validateMaxItemsInSuite(maxItemsInSuite); if (failThreshold === null) { failThreshold = alertThreshold; } - const chartXAxis = validateChartXAxis(chartXAxisValue); return { name, @@ -268,11 +281,10 @@ export async function configFromJobInput(): Promise { alertThreshold, failOnAlert, alertCommentCcUsers, + maxItemsInSuite, externalDataJsonPath, - maxItemsInChart, + configDataJsonPath, failThreshold, - chartXAxis, - oneChartGroups, overwriteAssets, metadata, }; diff --git a/src/extract.ts b/src/extract.ts index c0cfe79c9..54b31b7ca 100644 --- a/src/extract.ts +++ b/src/extract.ts @@ -183,7 +183,7 @@ export async function extractResult(config: Config): Promise { const benches = extractPytestResult(output); if (config.metadata !== '') { - benches.extra['gh-metadata'] = config.metadata; + benches.extra['metadata'] = config.metadata; } if (benches.results.length === 0) { diff --git a/src/git.ts b/src/git.ts index 6613bfa0c..c3f987eaf 100644 --- a/src/git.ts +++ b/src/git.ts @@ -1,3 +1,4 @@ +// Function for interacting with the git repository import { exec } from '@actions/exec'; import * as core from '@actions/core'; import * as github from '@actions/github'; diff --git a/src/write.ts b/src/write.ts index 15fac9ac5..89b462bd1 100644 --- a/src/write.ts +++ b/src/write.ts @@ -16,10 +16,7 @@ export interface DataJson { entries: BenchmarkSuites; } interface Assets { - index: string; - js: string; - funcs: string; - css: string; + [filename: string]: string; } export const SCRIPT_PREFIX = 'window.BENCHMARK_DATA = '; @@ -302,8 +299,6 @@ function addBenchmarkToDataJson(benchName: string, bench: Benchmark, data: DataJ let prevBench: Benchmark | null = null; data.lastUpdate = Date.now(); data.repoUrl = htmlUrl; - data.xAxis = config.chartXAxis; - data.oneChartGroups = config.oneChartGroups; // Add benchmark result if (data.entries[benchName] === undefined) { @@ -321,11 +316,12 @@ function addBenchmarkToDataJson(benchName: string, bench: Benchmark, data: DataJ suites.push(bench); - const { maxItemsInChart } = config; - if (maxItemsInChart !== null && suites.length > maxItemsInChart) { - suites.splice(0, suites.length - maxItemsInChart); + const { maxItemsInSuite } = config; + if (maxItemsInSuite !== null && suites.length > maxItemsInSuite) { + // TODO is this truncating from the wrong end? + suites.splice(0, suites.length - maxItemsInSuite); core.debug( - `Number of data items for '${benchName}' was truncated to ${maxItemsInChart} due to max-items-in-charts`, + `Number of data items for '${benchName}' was truncated to ${maxItemsInSuite} due to max-data-items`, ); } } @@ -377,10 +373,9 @@ async function writeBenchmarkToGitHubPagesWithRetry( await git.cmd('add', dataPath); - await addFileToGHPages(benchmarkDataDirPath, 'index.html', assets.index, overwriteAssets); - await addFileToGHPages(benchmarkDataDirPath, 'benchmark.css', assets.css, overwriteAssets); - await addFileToGHPages(benchmarkDataDirPath, 'main.js', assets.js, overwriteAssets); - await addFileToGHPages(benchmarkDataDirPath, 'funcs.js', assets.funcs, overwriteAssets); + for (const [filename, content] of Object.entries(assets)) { + await addFileToGHPages(benchmarkDataDirPath, filename, content, overwriteAssets); + } await git.cmd( 'commit', @@ -429,12 +424,18 @@ async function writeBenchmarkToGitHubPagesWithRetry( async function writeBenchmarkToGitHubPages(bench: Benchmark, config: Config): Promise { const { ghPagesBranch, skipFetchGhPages } = config; // note: assets need to be read before switching branch + const assets: Assets = { - index: await fs.readFile(path.join(__dirname, 'assets/index.html'), 'utf8'), - css: await fs.readFile(path.join(__dirname, 'assets/benchmark.css'), 'utf8'), - js: await fs.readFile(path.join(__dirname, 'assets/main.js'), 'utf8'), - funcs: await fs.readFile(path.join(__dirname, 'assets/funcs.js'), 'utf8'), + 'index.html': await fs.readFile(path.join(__dirname, 'assets/index.html'), 'utf8'), + 'benchmark.css': await fs.readFile(path.join(__dirname, 'assets/benchmark.css'), 'utf8'), + 'main.js': await fs.readFile(path.join(__dirname, 'assets/main.js'), 'utf8'), + 'funcs.js': await fs.readFile(path.join(__dirname, 'assets/funcs.js'), 'utf8'), }; + if (config.configDataJsonPath) { + assets['config.js'] = 'window.CONFIGURATION_DATA = ' + (await fs.readFile(config.configDataJsonPath, 'utf8')); + } else { + assets['config.js'] = 'window.CONFIGURATION_DATA = {}'; + } if (!skipFetchGhPages) { await git.cmd('fetch', 'origin', `${ghPagesBranch}:${ghPagesBranch}`); } diff --git a/test/config.ts b/test/config.ts index 2c4d731f8..b1bde2bc8 100644 --- a/test/config.ts +++ b/test/config.ts @@ -45,9 +45,7 @@ describe('configFromJobInput()', function() { 'fail-on-alert': 'false', 'alert-comment-cc-users': '', 'external-data-json-path': '', - 'max-items-in-chart': '', - 'one-chart-groups': '', - 'chart-xaxis': 'id', + 'max-data-items': '', }; const validation_tests = [ @@ -119,20 +117,20 @@ describe('configFromJobInput()', function() { expected: /auto-push must be false when external-data-json-path is set/, }, { - what: 'invalid integer value for max-items-in-chart', + what: 'invalid integer value for max-data-items', inputs: { ...defaultInputs, - 'max-items-in-chart': '3.14', + 'max-data-items': '3.14', }, - expected: /'max-items-in-chart' input must be unsigned integer but got '3.14'/, + expected: /'max-data-items' input must be unsigned integer but got '3.14'/, }, { - what: 'max-items-in-chart must not be zero', + what: 'max-data-items must not be zero', inputs: { ...defaultInputs, - 'max-items-in-chart': '0', + 'max-data-items': '0', }, - expected: /'max-items-in-chart' input value must be one or more/, + expected: /'max-data-items' input value must be one or more/, }, { what: 'alert-threshold must not be empty', @@ -157,6 +155,16 @@ describe('configFromJobInput()', function() { inputs: { ...defaultInputs, 'alert-threshold': '150%', 'fail-threshold': '120%' }, expected: /'alert-threshold' value must be smaller than 'fail-threshold' value but got 1.5 > 1.2/, }, + { + what: 'fail on non-existent config path', + inputs: { ...defaultInputs, 'render-json-path': 'none' }, + expected: /Invalid value for 'render-json-path' input/, + }, + { + what: 'fail on config file that is not JSON', + inputs: { ...defaultInputs, 'render-json-path': 'out.txt' }, + expected: /Invalid value for 'render-json-path' input/, + }, ] as Array<{ what: string; inputs: Inputs; @@ -181,7 +189,8 @@ describe('configFromJobInput()', function() { failOnAlert: boolean; alertCommentCcUsers: string[]; hasExternalDataJsonPath: boolean; - maxItemsInChart: null | number; + hasConfigJsonPath: boolean; + maxItemsInSuite: null | number; failThreshold: number | null; } @@ -196,7 +205,8 @@ describe('configFromJobInput()', function() { failOnAlert: false, alertCommentCcUsers: [], hasExternalDataJsonPath: false, - maxItemsInChart: null, + hasConfigJsonPath: false, + maxItemsInSuite: null, failThreshold: null, }; @@ -253,10 +263,15 @@ describe('configFromJobInput()', function() { inputs: { ...defaultInputs, 'external-data-json-path': 'external.json' }, expected: { ...defaultExpected, hasExternalDataJsonPath: true }, }, + { + what: 'config JSON file', + inputs: { ...defaultInputs, 'render-json-path': 'external.json' }, + expected: { ...defaultExpected, hasConfigJsonPath: true }, + }, { what: 'max items in chart', - inputs: { ...defaultInputs, 'max-items-in-chart': '50' }, - expected: { ...defaultExpected, maxItemsInChart: 50 }, + inputs: { ...defaultInputs, 'max-data-items': '50' }, + expected: { ...defaultExpected, maxItemsInSuite: 50 }, }, { what: 'different failure threshold from alert threshold', @@ -293,7 +308,7 @@ describe('configFromJobInput()', function() { A.deepEqual(actual.alertCommentCcUsers, test.expected.alertCommentCcUsers); A.ok(path.isAbsolute(actual.outputFilePath), actual.outputFilePath); A.ok(path.isAbsolute(actual.benchmarkDataDirPath), actual.benchmarkDataDirPath); - A.equal(actual.maxItemsInChart, test.expected.maxItemsInChart); + A.equal(actual.maxItemsInSuite, test.expected.maxItemsInSuite); if (test.expected.failThreshold === null) { A.equal(actual.failThreshold, test.expected.alertThreshold); } else { @@ -306,6 +321,13 @@ describe('configFromJobInput()', function() { } else { A.equal(actual.externalDataJsonPath, undefined); } + + if (test.expected.hasConfigJsonPath) { + A.equal(typeof actual.configDataJsonPath, 'string'); + A.ok(path.isAbsolute(actual.configDataJsonPath), actual.configDataJsonPath); + } else { + A.equal(actual.configDataJsonPath, undefined); + } }); } diff --git a/test/data/write/with-index-html/config.js b/test/data/write/with-index-html/config.js new file mode 100644 index 000000000..b88e07b6a --- /dev/null +++ b/test/data/write/with-index-html/config.js @@ -0,0 +1 @@ +window.CONFIGURATION_DATA = {} diff --git a/test/write.ts b/test/write.ts index b64f4afbf..e2e9e9d3b 100644 --- a/test/write.ts +++ b/test/write.ts @@ -187,10 +187,9 @@ describe('writeBenchmark()', function() { failOnAlert: true, alertCommentCcUsers: ['@user'], externalDataJsonPath: dataJson, - maxItemsInChart: null, + configDataJsonPath: undefined, + maxItemsInSuite: null, failThreshold: 2.0, - chartXAxis: 'id', - oneChartGroups: [], overwriteAssets: false, metadata: '', }; @@ -527,8 +526,8 @@ describe('writeBenchmark()', function() { error: ['Repository information is not available in payload: {', ' "repository": null', '}'], }, { - it: 'truncates data items if it exceeds max-items-in-chart', - config: { ...defaultCfg, maxItemsInChart: 1 }, + it: 'truncates data items if it exceeds max-data-items', + config: { ...defaultCfg, maxItemsInSuite: 1 }, data: { lastUpdate, repoUrl, @@ -547,7 +546,7 @@ describe('writeBenchmark()', function() { date: lastUpdate, benches: [bench('bench_fib_10', 100), bench('bench_fib_20', 10000)], // Exceeds 2.0 threshold }, - // Though first item is truncated due to maxItemsInChart, alert still can be raised since previous data + // Though first item is truncated due to maxItemsInSuite, alert still can be raised since previous data // is obtained before truncating an array of data items. error: [ '# :warning: **Performance Alert** :warning:', @@ -697,12 +696,12 @@ describe('writeBenchmark()', function() { for (const name of Object.keys(t.data.entries)) { const entries = t.data.entries[name]; if (name === t.config.name) { - if (t.config.maxItemsInChart === null || len < t.config.maxItemsInChart) { + if (t.config.maxItemsInSuite === null || len < t.config.maxItemsInSuite) { eq(len, entries.length + 1, name); // Check benchmark data except for the last appended one are not modified eq(json.entries[name].slice(0, -1), entries, name); } else { - // When data items was truncated due to max-items-in-chart + // When data items was truncated due to max-data-items eq(len, entries.length, name); // Number of items did not change because first item was shifted eq(json.entries[name].slice(0, -1), entries.slice(1), name); } @@ -779,6 +778,7 @@ describe('writeBenchmark()', function() { path.join('data-dir', 'benchmark.css'), path.join('data-dir', 'main.js'), path.join('data-dir', 'funcs.js'), + path.join('data-dir', 'config.js'), 'new-data-dir', path.join('with-index-html', 'data.js'), ]) { @@ -830,10 +830,9 @@ describe('writeBenchmark()', function() { failOnAlert: true, alertCommentCcUsers: [], externalDataJsonPath: undefined, - maxItemsInChart: null, + configDataJsonPath: undefined, + maxItemsInSuite: null, failThreshold: 2.0, - chartXAxis: 'id', - oneChartGroups: [], overwriteAssets: false, metadata: '', }; @@ -863,6 +862,7 @@ describe('writeBenchmark()', function() { addIndexHtml ? ['cmd', ['add', path.join(dir, 'benchmark.css')]] : undefined, addIndexHtml ? ['cmd', ['add', path.join(dir, 'main.js')]] : undefined, addIndexHtml ? ['cmd', ['add', path.join(dir, 'funcs.js')]] : undefined, + addIndexHtml ? ['cmd', ['add', path.join(dir, 'config.js')]] : undefined, ['cmd', ['commit', '-m', 'add Test benchmark benchmark result for current commit id']], autoPush ? ['push', [token, 'gh-pages']] : undefined, ['cmd', ['checkout', '-']], // Return from gh-pages