diff --git a/.github/workflows/perfs.yml b/.github/workflows/perfs.yml
index 326f916813..e91da8d7c3 100644
--- a/.github/workflows/perfs.yml
+++ b/.github/workflows/perfs.yml
@@ -1,14 +1,23 @@
name: Performance tests
+
on:
pull_request:
- types: [labeled]
+ types: [opened, synchronize, reopened]
+
+# Abort if new commit since then
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+ cancel-in-progress: true
jobs:
perf-tests:
- if: ${{ github.event.label.name == 'Performance checks' }}
+ if:
+ ${{ !contains(github.event.pull_request.labels.*.name, 'skip-performance-checks') }}
runs-on: [ubuntu-latest]
+ permissions:
+ pull-requests: write
steps:
- - uses: actions/checkout@v2
+ - uses: actions/checkout@v3
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v2
with:
@@ -21,4 +30,27 @@ jobs:
- run: npm ci
- run: export DISPLAY=:99
- run: sudo Xvfb -ac :99 -screen 0 1280x1024x24 > /dev/null 2>&1 & # optional
- - run: node tests/performance/run.mjs
+ - run:
+ node tests/performance/run.mjs --branch $GITHUB_BASE_REF --remote-git-url
+ https://github.com/canalplus/rx-player.git --report perf-report.md
+ - name: Post comment
+ if: always()
+ uses: actions/github-script@v7
+ with:
+ script: |
+ const { readFileSync, existsSync } = require('fs');
+ if (!existsSync("./perf-report.md")) {
+ return;
+ }
+ const fileContent = readFileSync("./perf-report.md").toString();
+ github.rest.issues.createComment({
+ issue_number: context.issue.number,
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ // TODO: generate comment header inside the report file instead of here for better portability?
+ // We should already have access to the sha1 through `git` and the destination branch through the command line arguments.
+ body: "Automated performance checks have been performed on commit " +
+ "\`${{github.event.pull_request.head.sha}}\` with the base branch \`${{github.base_ref}}\`.\n\n" +
+ fileContent +
+ "\n\n If you want to skip performance checks for latter commits, add the `skip-performance-checks` label to this Pull Request.",
+ })
diff --git a/.gitignore b/.gitignore
index fb71aaae22..4bc3b90f08 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,8 +7,8 @@
/demo/worker.js
/tests/performance/node_modules
-/tests/performance/bundle1.js
-/tests/performance/bundle2.js
+/tests/performance/previous.js
+/tests/performance/current.js
/tests/performance/package.json
/tests/performance/package-lock.json
diff --git a/tests/performance/index1.html b/tests/performance/current.html
similarity index 80%
rename from tests/performance/index1.html
rename to tests/performance/current.html
index f0e0916f01..fa3eed8cca 100644
--- a/tests/performance/index1.html
+++ b/tests/performance/current.html
@@ -3,7 +3,7 @@
-
+
RxPlayer - Performance tests
diff --git a/tests/performance/index2.html b/tests/performance/previous.html
similarity index 75%
rename from tests/performance/index2.html
rename to tests/performance/previous.html
index 0938519466..8c930565c3 100644
--- a/tests/performance/index2.html
+++ b/tests/performance/previous.html
@@ -3,7 +3,7 @@
-
+
RxPlayer - Performance tests
diff --git a/tests/performance/run.mjs b/tests/performance/run.mjs
index 830a300c88..3c68f189f0 100644
--- a/tests/performance/run.mjs
+++ b/tests/performance/run.mjs
@@ -6,11 +6,11 @@ import esbuild from "esbuild";
import * as fs from "fs/promises";
import { createServer } from "http";
import * as path from "path";
-import { fileURLToPath } from "url";
+import { fileURLToPath, pathToFileURL } from "url";
import launchStaticServer from "../../scripts/launch_static_server.mjs";
-import getHumanReadableHours from "../../scripts/utils/get_human_readable_hours.mjs";
import removeDir from "../../scripts/utils/remove_dir.mjs";
import createContentServer from "../contents/server.mjs";
+import { appendFileSync, rmSync, writeFileSync } from "fs";
const currentDirectory = path.dirname(fileURLToPath(import.meta.url));
@@ -20,12 +20,17 @@ const CONTENT_SERVER_PORT = 3000;
/** Port of the HTTP server which will serve the performance test files */
const PERF_TESTS_PORT = 8080;
+/** Port of the HTTP server which will be used to exchange about test results. */
+const RESULT_SERVER_PORT = 6789;
+
/**
* Number of times test are runs on each browser/RxPlayer configuration.
* More iterations means (much) more time to perform tests, but also produce
* better estimates.
+ *
+ * TODO: GitHub actions fails when running the 128th browser. Find out why.
*/
-const TEST_ITERATIONS = 10;
+const TEST_ITERATIONS = 30;
/**
* After initialization is done, contains the path allowing to run the Chrome
@@ -34,12 +39,12 @@ const TEST_ITERATIONS = 10;
*/
let CHROME_CMD;
-/**
- * After initialization is done, contains the path allowing to run the Firefox
- * browser.
- * @type {string|undefined|null}
- */
-let FIREFOX_CMD;
+// /**
+// * After initialization is done, contains the path allowing to run the Firefox
+// * browser.
+// * @type {string|undefined|null}
+// */
+// let FIREFOX_CMD;
/** Options used when starting the Chrome browser. */
const CHROME_OPTIONS = [
@@ -56,19 +61,15 @@ const CHROME_OPTIONS = [
"--headless",
"--disable-gpu",
"--disable-dev-shm-usage",
-
- // We don't even care about that one but Chrome may not launch without this
- // for some unknown reason
- "--remote-debugging-port=9222",
+ "--disk-cache-dir=/dev/null",
];
-/** Options used when starting the Firefox browser. */
-const FIREFOX_OPTIONS = [
- "-no-remote",
- "-wait-for-browser",
- "-headless",
- // "--start-debugger-server 6000",
-];
+// /** Options used when starting the Firefox browser. */
+// const FIREFOX_OPTIONS = [
+// "-no-remote",
+// "-wait-for-browser",
+// "-headless",
+// ];
/**
* `ChildProcess` instance of the current browser being run.
@@ -84,110 +85,393 @@ let currentBrowser;
*/
const tasks = [];
-/**
- * Index of the currently ran task in the `tasks` array.
- * @see tasks
- */
-let nextTaskIndex = 0;
-
/**
* Store results of the performance tests in two arrays:
- * - the first one contains the test results of the current RxPlayer version
- * - the second one contains the test results of the last RxPlayer version
+ * - "current" contains the test results of the current RxPlayer version
+ * - "previous" contains the test results of the last RxPlayer version
*/
-const allSamples = [[], []];
+const allSamples = {
+ current: [],
+ previous: [],
+};
+
+// If true, this script is called directly
+if (import.meta.url === pathToFileURL(process.argv[1]).href) {
+ const args = process.argv.slice(2);
+ if (args.includes("-h") || args.includes("--help")) {
+ displayHelp();
+ process.exit(0);
+ }
-/**
- * Current results for the tests being run in `currentBrowser`.
- * Will be added to `allSamples` once those tests are finished.
- */
-let currentTestSample = [];
+ let branchName;
+ {
+ let branchNameIndex = args.indexOf("-b");
+ if (branchNameIndex < 0) {
+ branchNameIndex = args.indexOf("--branch");
+ }
+ if (branchNameIndex >= 0) {
+ branchName = args[branchNameIndex + 1];
+ if (branchName === undefined) {
+ // eslint-disable-next-line no-console
+ console.error("ERROR: no branch name provided\n");
+ displayHelp();
+ process.exit(1);
+ }
+ }
+ }
+
+ let remote;
+ {
+ let branchNameIndex = args.indexOf("-u");
+ if (branchNameIndex < 0) {
+ branchNameIndex = args.indexOf("--remote-git-url");
+ }
+ if (branchNameIndex >= 0) {
+ remote = args[branchNameIndex + 1];
+ if (remote === undefined) {
+ // eslint-disable-next-line no-console
+ console.error("ERROR: no remote URL provided\n");
+ displayHelp();
+ process.exit(1);
+ }
+ }
+ }
+
+ let reportFile;
+ {
+ let reportFileIndex = args.indexOf("-r");
+ if (reportFileIndex < 0) {
+ reportFileIndex = args.indexOf("--report");
+ }
+ if (reportFileIndex >= 0) {
+ reportFile = args[reportFileIndex + 1];
+ if (reportFile === undefined) {
+ // eslint-disable-next-line no-console
+ console.error("ERROR: no file path provided\n");
+ displayHelp();
+ process.exit(1);
+ }
+ }
+ }
+
+ /* eslint-disable no-console */
+ if (reportFile !== undefined) {
+ try {
+ console.log(`Removing previous report file if it exists ("${reportFile}")`);
+ rmSync(reportFile);
+ } catch (_) {
+ // We don't really care here
+ }
+ }
+
+ initializePerformanceTestsPages({
+ branchName: branchName ?? "dev",
+ remoteGitUrl: remote,
+ })
+ .then(() => runPerformanceTests())
+ .then(async (results) => {
+ if (reportFile !== undefined) {
+ try {
+ writeFileSync(reportFile, "Tests results\n" + "-------------\n\n");
+ if (results.worse.length === 0) {
+ appendToReportFile("✅ Tests have passed.\n");
+ } else {
+ appendToReportFile("❌ Tests have failed.\n");
+ }
+ appendToReportFile(
+ "Performance tests 1st run output\n" + "--------------------------------",
+ );
+ } catch (err) {
+ console.error(
+ `Cannot write file output: Invalid file path given: ${reportFile}`,
+ );
+ }
+ }
+
+ if (results.worse.length > 0) {
+ const failureTxt =
+ "\nWorse performance for tests:\n\n" +
+ formatResultInHumanReadableWay(results.worse);
+ console.warn(failureTxt);
+ appendToReportFile(failureTxt);
+ }
+
+ if (results.better.length > 0) {
+ const betterTxt =
+ "\nBetter performance for tests:\n\n" +
+ formatResultInHumanReadableWay(results.better);
+ console.log(betterTxt);
+ appendToReportFile(betterTxt);
+ }
+
+ if (results.notSignificative.length > 0) {
+ const notSignificativeTxt =
+ "\nNo significative change in performance for tests:\n\n" +
+ formatResultInHumanReadableWay(results.notSignificative);
+ console.log(notSignificativeTxt);
+ appendToReportFile(notSignificativeTxt);
+ }
+
+ if (results.worse.length === 0) {
+ process.exit(0);
+ }
+
+ console.warn("\nRetrying one time just to check if unlucky...");
+
+ const results2 = await runPerformanceTests();
+ console.error("\nFinal result after 2 attempts\n-----------------------------\n");
+ appendToReportFile(
+ "\nPerformance tests 2nd run output\n" + "--------------------------------",
+ );
+
+ if (results.better.length > 0) {
+ console.error(
+ "\nBetter performance at first attempt for tests:\n\n" +
+ formatResultInHumanReadableWay(results.better),
+ );
+ }
+ if (results2.better.length > 0) {
+ const betterTxt =
+ "\nBetter performance for tests:\n\n" +
+ formatResultInHumanReadableWay(results.better);
+ appendToReportFile(betterTxt);
+ console.error(
+ "\nBetter performance at second attempt for tests:\n\n" +
+ formatResultInHumanReadableWay(results2.better),
+ );
+ }
+
+ if (results.worse.length > 0) {
+ console.error(
+ "\nWorse performance at first attempt for tests:\n\n" +
+ formatResultInHumanReadableWay(results.worse),
+ );
+ }
+ if (results2.worse.length > 0) {
+ const failureTxt =
+ "\nWorse performance at second attempt for tests:\n\n" +
+ formatResultInHumanReadableWay(results.worse);
+ console.warn(failureTxt);
+ appendToReportFile(failureTxt);
+ }
+
+ if (results2.notSignificative.length > 0) {
+ const notSignificativeTxt =
+ "\nNo significative change in performance for tests:\n\n" +
+ formatResultInHumanReadableWay(results.notSignificative);
+ appendToReportFile(notSignificativeTxt);
+ }
+
+ for (const failure1 of results.worse) {
+ if (results2.worse.some((r) => r.testName === failure1.testName)) {
+ process.exit(1);
+ }
+ }
+ process.exit(0);
+
+ function appendToReportFile(text) {
+ if (reportFile === undefined) {
+ return;
+ }
+ try {
+ appendFileSync(reportFile, text + "\n");
+ } catch (err) {
+ /* eslint-disable-next-line no-console */
+ console.error(
+ `Cannot write file output: Invalid file path given: ${reportFile}`,
+ );
+ }
+ }
+ })
+ .catch((err) => {
+ console.error("Error:", err);
+ return process.exit(1);
+ });
+ /* eslint-enable no-console */
+}
/**
- * Contains references to every launched servers, with a `close` method allowing
- * to close each one of them.
+ * Take test results as outputed by performance tests and output a markdown
+ * table listing them in hopefully a readable way.
+ * @param {Array.