From 85f8e94058514b4fb1e60340098ba574e474013c Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Wed, 13 Mar 2024 15:57:37 +0000 Subject: [PATCH 01/16] Implement feature flags performance tests This commit introduces performance testing for the feature flags functionality. New tests have been added in "loadtime.spec.ts" to evaluate load time metrics. A GitHub workflow for performance tests is also introduced in this update for continuous integration. Minor adjustments to the "SdkModal" test and package configuration were made to accommodate the new tests. --- .github/workflows/perf-tests.yml | 98 +++++++ package.json | 135 +++++----- .../modals/__tests__/SdkModal.test.js | 2 +- tests/performance/cli/results.js | 252 ++++++++++++++++++ .../config/performance-reporter.ts | 112 ++++++++ tests/performance/playwright.config.ts | 40 +++ tests/performance/specs/loadtime.spec.ts | 48 ++++ tests/performance/utils/index.ts | 27 ++ 8 files changed, 646 insertions(+), 68 deletions(-) create mode 100644 .github/workflows/perf-tests.yml create mode 100644 tests/performance/cli/results.js create mode 100644 tests/performance/config/performance-reporter.ts create mode 100644 tests/performance/playwright.config.ts create mode 100644 tests/performance/specs/loadtime.spec.ts create mode 100644 tests/performance/utils/index.ts diff --git a/.github/workflows/perf-tests.yml b/.github/workflows/perf-tests.yml new file mode 100644 index 0000000..aea4e96 --- /dev/null +++ b/.github/workflows/perf-tests.yml @@ -0,0 +1,98 @@ +name: Performance Tests +on: + push: + branches-ignore: + - 'main-built' + +jobs: + performance-tests: + name: 'Performance Tests' + runs-on: ubuntu-latest + env: + WP_BASE_URL: 'http://localhost:8888' + WP_USERNAME: 'admin' + WP_PASSWORD: 'password' + WP_AUTH_STORAGE: '.auth/wordpress.json' + WP_ARTIFACTS_PATH: ${{ github.workspace }}/artifacts + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version-file: '.nvmrc' + cache: yarn + + - name: Cache Composer dependencies + uses: actions/cache@v4 + with: + path: /tmp/composer-cache + key: ${{ runner.os }}-${{ hashFiles('**/composer.lock') }} + + - name: Setup composer + uses: php-actions/composer@v6 + with: + php_version: '8.3' + dev: no + + - name: Install dependencies + run: yarn install --immutable + + - name: Build packages + run: yarn build + + - name: Playwright install + run: yarn playwright install chromium + + - name: Start wp-env + run: yarn wp-env + + - name: Run tests + run: | + yarn test:performance + mv ${{ env.WP_ARTIFACTS_PATH }}/performance-results.json ${{ runner.temp }}/results_after.json + + - name: Check out base commit + run: | + if [[ -z "$BASE_REF" ]]; then + git fetch -n origin $BASE_SHA + git reset --hard $BASE_SHA + else + git fetch -n origin $BASE_REF + git reset --hard $BASE_SHA + fi + env: + BASE_REF: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.ref || '' }} + BASE_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }} + + # Run tests without causing job to fail if they don't pass (e.g. because of env issues). + - name: Run tests for base + run: | + npm run test:performance || true + if [ -f "{{ env.WP_ARTIFACTS_PATH }}/performance-results.json" ]; then + mv ${{ env.WP_ARTIFACTS_PATH }}/performance-results.json ${{ runner.temp }}/results_before.json + fi; + + - name: Reset to original commit + run: | + git reset --hard $GITHUB_SHA + + - name: Compare results with base + run: | + if [ -f "${{ runner.temp }}/results_before.json" ]; then + npm run test:performance:results ${{ runner.temp }}/results_after.json ${{ runner.temp }}/results_before.json + else + npm run test:performance:results ${{ runner.temp }}/results_after.json + fi; + + - name: Add workflow summary + run: | + cat ${{ env.WP_ARTIFACTS_PATH }}/performance-results.md >> $GITHUB_STEP_SUMMARY + + - name: Upload performance results + if: success() + uses: actions/upload-artifact@v4 + with: + name: performance-results + path: ${{ env.WP_ARTIFACTS_PATH }}/performance-results.json diff --git a/package.json b/package.json index 977ee0f..f0e6314 100644 --- a/package.json +++ b/package.json @@ -1,69 +1,70 @@ { - "name": "codeb-feature-flags", - "version": "0.3.2", - "description": "Allows developers to enable / disable features based on flags.", - "license": "ISC", - "author": "Mohan Raj ", - "scripts": { - "build": "wp-scripts build", - "lint:css": "wp-scripts lint-style", - "lint:css:fix": "npm run lint:css -- --fix", - "lint:js": "wp-scripts lint-js", - "lint:js:fix": "wp-scripts lint-js --fix", - "prepare": "husky", - "start": "wp-scripts start", - "test:e2e": "wp-scripts test-playwright", - "test:js": "wp-scripts test-unit-js", - "test:watch": "wp-scripts test-unit-js --watch", - "version:major": "node ./scripts/version major", - "version:minor": "node ./scripts/version minor", - "version:patch": "node ./scripts/version patch", - "wp-env": "wp-env start", - "wp-env:coverage": "wp-env start --xdebug=coverage", - "php:unit": "wp-env run --env-cwd='wp-content/plugins/feature-flags' tests-wordpress composer test:unit", - "php:integration": "wp-env run tests-wordpress --env-cwd=wp-content/plugins/feature-flags composer test:integration", - "php:multisite": "wp-env run tests-wordpress --env-cwd=wp-content/plugins/feature-flags composer test:multisite" - }, - "dependencies": { - "@testing-library/user-event": "^14.5.2", - "@wordpress/api-fetch": "^6.48.0", - "@wordpress/components": "^27.1.0", - "@wordpress/data": "^9.23.0", - "@wordpress/dom-ready": "^3.53.0", - "@wordpress/hooks": "^3.53.0", - "@wordpress/i18n": "^4.53.0", - "@wordpress/notices": "^4.21.0", - "dotenv": "^16.4.5", - "react": "18.2.0", - "react-dom": "18.2.0", - "react-syntax-highlighter": "^15.5.0", - "react-test-renderer": "^18.2.0", - "ts-loader": "^9.5.1", - "typescript": "^5.4.2" - }, - "devDependencies": { - "@playwright/test": "^1.42.1", - "@testing-library/jest-dom": "^6.4.2", - "@testing-library/react": "14.2.1", - "@types/jest": "^29.5.12", - "@types/node": "^20.11.25", - "@types/react-syntax-highlighter": "^15.5.11", - "@types/wordpress__components": "^23.0.11", - "@wordpress/e2e-test-utils-playwright": "^0.21.0", - "@wordpress/env": "^9.5.0", - "@wordpress/eslint-plugin": "^17.10.0", - "@wordpress/scripts": "^27.4.0", - "eslint": "^8.57.0", - "eslint-import-resolver-alias": "^1.1.2", - "eslint-plugin-cypress": "^2.15.1", - "eslint-plugin-import": "^2.29.1", - "husky": "^9.0.11", - "jest-environment-jsdom": "^29.7.0", - "prettier": "^3.2.5" - }, - "keywords": [ - "feature flags", - "wordpress", - "plugin" - ] + "name": "codeb-feature-flags", + "version": "0.3.2", + "description": "Allows developers to enable / disable features based on flags.", + "license": "ISC", + "author": "Mohan Raj ", + "scripts": { + "build": "wp-scripts build", + "lint:css": "wp-scripts lint-style", + "lint:css:fix": "npm run lint:css -- --fix", + "lint:js": "wp-scripts lint-js", + "lint:js:fix": "wp-scripts lint-js --fix", + "prepare": "husky", + "start": "wp-scripts start", + "test:e2e": "wp-scripts test-playwright", + "test:js": "wp-scripts test-unit-js", + "test:performance": "wp-scripts test-playwright --config tests/performance/playwright.config.ts", + "test:watch": "wp-scripts test-unit-js --watch", + "version:major": "node ./scripts/version major", + "version:minor": "node ./scripts/version minor", + "version:patch": "node ./scripts/version patch", + "wp-env": "wp-env start", + "wp-env:coverage": "wp-env start --xdebug=coverage", + "php:unit": "wp-env run --env-cwd='wp-content/plugins/feature-flags' tests-wordpress composer test:unit", + "php:integration": "wp-env run tests-wordpress --env-cwd=wp-content/plugins/feature-flags composer test:integration", + "php:multisite": "wp-env run tests-wordpress --env-cwd=wp-content/plugins/feature-flags composer test:multisite" + }, + "dependencies": { + "@testing-library/user-event": "^14.5.2", + "@wordpress/api-fetch": "^6.48.0", + "@wordpress/components": "^27.1.0", + "@wordpress/data": "^9.23.0", + "@wordpress/dom-ready": "^3.53.0", + "@wordpress/hooks": "^3.53.0", + "@wordpress/i18n": "^4.53.0", + "@wordpress/notices": "^4.21.0", + "dotenv": "^16.4.5", + "react": "18.2.0", + "react-dom": "18.2.0", + "react-syntax-highlighter": "^15.5.0", + "react-test-renderer": "^18.2.0", + "ts-loader": "^9.5.1", + "typescript": "^5.4.2" + }, + "devDependencies": { + "@playwright/test": "^1.42.1", + "@testing-library/jest-dom": "^6.4.2", + "@testing-library/react": "14.2.1", + "@types/jest": "^29.5.12", + "@types/node": "^20.11.25", + "@types/react-syntax-highlighter": "^15.5.11", + "@types/wordpress__components": "^23.0.11", + "@wordpress/e2e-test-utils-playwright": "^0.21.0", + "@wordpress/env": "^9.5.0", + "@wordpress/eslint-plugin": "^17.10.0", + "@wordpress/scripts": "^27.4.0", + "eslint": "^8.57.0", + "eslint-import-resolver-alias": "^1.1.2", + "eslint-plugin-cypress": "^2.15.1", + "eslint-plugin-import": "^2.29.1", + "husky": "^9.0.11", + "jest-environment-jsdom": "^29.7.0", + "prettier": "^3.2.5" + }, + "keywords": [ + "feature flags", + "wordpress", + "plugin" + ] } diff --git a/src/components/modals/__tests__/SdkModal.test.js b/src/components/modals/__tests__/SdkModal.test.js index ed66599..42dd786 100644 --- a/src/components/modals/__tests__/SdkModal.test.js +++ b/src/components/modals/__tests__/SdkModal.test.js @@ -4,7 +4,7 @@ import SdkModal from '../SdkModal'; describe('SdkModal component', () => { test('should render modal correctly', async () => { - const item = { name: 'Test Flag' }; + const item = { id: 1, name: 'Test Flag', enabled: false }; const closeSdkModal = jest.fn(); render(); diff --git a/tests/performance/cli/results.js b/tests/performance/cli/results.js new file mode 100644 index 0000000..efbaa2f --- /dev/null +++ b/tests/performance/cli/results.js @@ -0,0 +1,252 @@ +#!/usr/bin/env node + +/** + * External dependencies + */ +const { existsSync, readFileSync, writeFileSync } = require( 'node:fs' ); +const { join, resolve } = require( 'node:path' ); + +process.env.WP_ARTIFACTS_PATH ??= join( process.cwd(), 'artifacts' ); + +const args = process.argv.slice( 2 ); + +const beforeFile = args[ 1 ]; +const afterFile = + args[ 0 ] || resolve( './artifacts/performance-results.json' ); + +if ( ! existsSync( afterFile ) ) { + console.error( `File not found: ${ afterFile }` ); + process.exit( 1 ); +} + +if ( beforeFile && ! existsSync( beforeFile ) ) { + console.error( `File not found: ${ beforeFile }` ); + process.exit( 1 ); +} + +/** + * @param {unknown} v + * @return {string} Formatted value. + */ +function formatTableValue( v ) { + if ( v === true || v === 'true' ) return '✅'; + if ( ! v || v === 'false' ) return ''; + return v?.toString() || String( v ); +} + +/** + * Simple way to format an array of objects as a Markdown table. + * + * For example, this array: + * + * [ + * { + * foo: 123, + * bar: 456, + * baz: 'Yes', + * }, + * { + * foo: 777, + * bar: 999, + * baz: 'No', + * } + * ] + * + * Will result in the following table: + * + * | foo | bar | baz | + * |-----|-----|-----| + * | 123 | 456 | Yes | + * | 777 | 999 | No | + * + * @param {Array} rows Table rows. + * @return {string} Markdown table content. + */ +function formatAsMarkdownTable( rows ) { + let result = ''; + const headers = Object.keys( rows[ 0 ] ); + for ( const header of headers ) { + result += `| ${ header } `; + } + result += '|\n'; + for ( const header of headers ) { + const dashes = '-'.repeat( header.length ); + result += `| ${ dashes } `; + } + result += '|\n'; + + for ( const row of rows ) { + for ( const [ key, value ] of Object.entries( row ) ) { + result += `| ${ formatTableValue( value ).padStart( + key.length, + ' ' + ) } `; + } + result += '|\n'; + } + + return result; +} + +/** + * Computes the median number from an array numbers. + * + * @param {number[]} array List of numbers. + * @return {number} Median. + */ +function median( array ) { + const mid = Math.floor( array.length / 2 ); + const numbers = [ ...array ].sort( ( a, b ) => a - b ); + const result = + array.length % 2 !== 0 + ? numbers[ mid ] + : ( numbers[ mid - 1 ] + numbers[ mid ] ) / 2; + + return Number( result.toFixed( 2 ) ); +} + +/** + * @type {Array<{file: string, title: string, results: Record[]}>} + */ +let beforeStats = []; + +/** + * @type {Array<{file: string, title: string, results: Record[]}>} + */ +let afterStats; + +if ( beforeFile ) { + try { + beforeStats = JSON.parse( + readFileSync( beforeFile, { encoding: 'utf-8' } ) + ); + } catch {} +} + +try { + afterStats = JSON.parse( readFileSync( afterFile, { encoding: 'utf-8' } ) ); +} catch { + console.error( `Could not read file: ${ afterFile }` ); + process.exit( 1 ); +} + +let summaryMarkdown = `**Performance Test Results**\n\n`; + +if ( process.env.GITHUB_SHA ) { + summaryMarkdown += `Performance test results for ${ process.env.GITHUB_SHA } are in 🛎️!\n\n`; +} else { + summaryMarkdown += `Performance test results are in 🛎️!\n\n`; +} + +if ( beforeFile ) { + summaryMarkdown += `Note: the numbers in parentheses show the difference to the previous (baseline) test run.\n\n`; +} + +console.log( 'Performance Test Results\n' ); + +if ( beforeFile ) { + console.log( + 'Note: the numbers in parentheses show the difference to the previous (baseline) test run.\n' + ); +} + +const DELTA_VARIANCE = 0.5; +const PERCENTAGE_VARIANCE = 2; + +/** + * Format value and add unit. + * + * Turns bytes into MB (base 10). + * + * @todo Dynamic formatting based on definition in result.json. + * + * @param {number} value Value. + * @param {string} key Key. + * @return {string} Formatted value. + */ +function formatValue( value, key ) { + if ( key === 'CLS' ) { + return value.toFixed( 2 ); + } + + if ( key === 'wpDbQueries' ) { + return value.toFixed( 0 ); + } + + if ( key === 'wpMemoryUsage' ) { + return `${ ( value / Math.pow( 10, 6 ) ).toFixed( 2 ) } MB`; + } + + return `${ value.toFixed( 2 ) } ms`; +} + +for ( const { file, title, results } of afterStats ) { + const prevStat = beforeStats.find( ( s ) => s.file === file ); + + /** + * @type {Array>} + */ + const diffResults = []; + + for ( const i in results ) { + const newResult = results[ i ]; + + /** + * @type {Record} + */ + const diffResult = { + Run: i, + }; + + for ( const [ key, values ] of Object.entries( newResult ) ) { + // Only do comparison if the number of results is the same. + const prevValues = + prevStat?.results.length === results.length + ? prevStat?.results[ i ].key + : null; + + const value = median( values ); + const prevValue = prevValues ? median( prevValues ) : 0; + const delta = value - prevValue; + const percentage = ( delta / value ) * 100; + + // Skip if there is not a significant delta or none at all. + if ( + ! prevValues || + ! percentage || + Math.abs( percentage ) <= PERCENTAGE_VARIANCE || + ! delta || + Math.abs( delta ) <= DELTA_VARIANCE + ) { + diffResult[ key ] = formatValue( + /** @type {number} */ ( value ), + key + ); + continue; + } + + const prefix = delta > 0 ? '+' : ''; + + diffResult[ key ] = `${ formatValue( + value, + key + ) } (${ prefix }${ formatValue( + delta, + key + ) } / ${ prefix }${ percentage }%)`; + } + + diffResults.push( diffResult ); + } + + console.log( title ); + console.table( diffResults ); + + summaryMarkdown += `**${ title }**\n\n`; + summaryMarkdown += `${ formatAsMarkdownTable( diffResults ) }\n`; +} + +writeFileSync( + join( process.env.WP_ARTIFACTS_PATH, '/performance-results.md' ), + summaryMarkdown +); diff --git a/tests/performance/config/performance-reporter.ts b/tests/performance/config/performance-reporter.ts new file mode 100644 index 0000000..7c00a80 --- /dev/null +++ b/tests/performance/config/performance-reporter.ts @@ -0,0 +1,112 @@ +import { join } from 'node:path'; +import { writeFileSync } from 'node:fs'; +import type { + FullConfig, + FullResult, + Reporter, + TestCase, + TestResult, +} from '@playwright/test/reporter'; +import { median } from '../utils'; + +process.env.WP_ARTIFACTS_PATH ??= join( process.cwd(), 'artifacts' ); + +class PerformanceReporter implements Reporter { + private shard?: FullConfig[ 'shard' ]; + + allResults: Record< + string, + { + title: string; + results: Record< string, number[] >[]; + } + > = {}; + + onBegin( config: FullConfig ) { + if ( config.shard ) { + this.shard = config.shard; + } + } + + /** + * Called after a test has been finished in the worker process. + * + * Used to add test results to the final summary of all tests. + * + * @param test + * @param result + */ + onTestEnd( test: TestCase, result: TestResult ) { + const performanceResults = result.attachments.find( + ( attachment ) => attachment.name === 'results' + ); + + if ( performanceResults?.body ) { + this.allResults[ test.location.file ] ??= { + // 0 = empty, 1 = browser, 2 = file name, 3 = test suite name. + title: test.titlePath()[ 3 ], + results: [], + }; + this.allResults[ test.location.file ].results.push( + JSON.parse( performanceResults.body.toString( 'utf-8' ) ) + ); + } + } + + /** + * Called after all tests have been run, or testing has been interrupted. + * + * Provides a quick summary and writes all raw numbers to a file + * for further processing, for example to compare with a previous run. + * + * @param result + */ + onEnd( result: FullResult ) { + const summary = []; + + if ( Object.keys( this.allResults ).length > 0 ) { + if ( this.shard ) { + console.log( + `\nPerformance Test Results ${ this.shard.current }/${ this.shard.total }` + ); + } else { + console.log( `\nPerformance Test Results` ); + } + console.log( `Status: ${ result.status }` ); + } + + for ( const [ file, { title, results } ] of Object.entries( + this.allResults + ) ) { + console.log( `\n${ title }\n` ); + console.table( + results.map( ( r ) => + Object.fromEntries( + Object.entries( r ).map( ( [ key, value ] ) => [ + key, + median( value ), + ] ) + ) + ) + ); + + summary.push( { + file, + title, + results, + } ); + } + + if ( ! this.shard ) { + writeFileSync( + join( + process.env.WP_ARTIFACTS_PATH as string, + 'performance-results.json' + ), + JSON.stringify( summary, null, 2 ) + ); + } + } +} + +export default PerformanceReporter; diff --git a/tests/performance/playwright.config.ts b/tests/performance/playwright.config.ts new file mode 100644 index 0000000..ae56a4a --- /dev/null +++ b/tests/performance/playwright.config.ts @@ -0,0 +1,40 @@ +/** + * External dependencies + */ +import { join } from 'node:path'; +import { defineConfig, devices } from '@playwright/test'; +import dotenv from 'dotenv'; + +dotenv.config(); + +process.env.WP_ARTIFACTS_PATH ??= join(process.cwd(), 'artifacts'); +const authStoragePath = process.env.WP_AUTH_STORAGE; +process.env.STORAGE_STATE_PATH ??= join( + process.env.WP_ARTIFACTS_PATH, + authStoragePath +); +process.env.TEST_ITERATIONS ??= '4'; + +const config = defineConfig({ + globalSetup: require.resolve('../e2e/global-setup.ts'), + reporter: process.env.CI + ? [['blob'], ['./config/performance-reporter.ts']] + : [['list'], ['./config/performance-reporter.ts']], + forbidOnly: !!process.env.CI, + workers: 1, + retries: 0, + repeatEach: 1, + timeout: parseInt(process.env.TIMEOUT || '', 10) || 600_000, // Defaults to 10 minutes. + reportSlowTests: null, + use: { + baseURL: process.env.WP_BASE_URL, + }, + projects: [ + { + name: 'chromium', + use: { ...devices['Desktop Chrome'] }, + }, + ], +}); + +export default config; diff --git a/tests/performance/specs/loadtime.spec.ts b/tests/performance/specs/loadtime.spec.ts new file mode 100644 index 0000000..38cf219 --- /dev/null +++ b/tests/performance/specs/loadtime.spec.ts @@ -0,0 +1,48 @@ +import { expect, test } from '@wordpress/e2e-test-utils-playwright'; +import { camelCaseDashes } from '../utils'; + +const results: Record = {}; + +test.use({ storageState: process.env.WP_AUTH_STORAGE }); +test.describe('Feature flags Perf Tests', () => { + // After all results are processed, attach results for further processing. + // For easier handling, only one attachment per file. + test.afterAll(async ({}, testInfo) => { + await testInfo.attach('results', { + body: JSON.stringify(results, null, 2), + contentType: 'application/json', + }); + }); + + const iterations = Number(process.env.TEST_ITERATIONS); + for (let i = 1; i <= iterations; i++) { + test(`Measure load time metrics (${i} of ${iterations})`, async ({ + page, + admin, + metrics, + }) => { + await admin.visitAdminPage('/'); + await page.getByRole('link', { name: 'Feature Flags' }).click(); + await expect( + page.getByRole('heading', { name: 'Feature Flags' }) + ).toBeVisible(); + + const serverTiming = await metrics.getServerTiming(); + + for (const [key, value] of Object.entries(serverTiming)) { + results[camelCaseDashes(key)] ??= []; + results[camelCaseDashes(key)]?.push(value); + } + + const ttfb = await metrics.getTimeToFirstByte(); + const lcp = await metrics.getLargestContentfulPaint(); + + results.largestContentfulPaint ??= []; + results.largestContentfulPaint.push(lcp); + results.timeToFirstByte ??= []; + results.timeToFirstByte.push(ttfb); + results.lcpMinusTtfb ??= []; + results.lcpMinusTtfb.push(lcp - ttfb); + }); + } +}); diff --git a/tests/performance/utils/index.ts b/tests/performance/utils/index.ts new file mode 100644 index 0000000..bcf2707 --- /dev/null +++ b/tests/performance/utils/index.ts @@ -0,0 +1,27 @@ +/** + * Helper function to camel case the letter after dashes, removing the dashes. + * + * @param str + */ +export function camelCaseDashes( str: string ) { + return str.replace( /-([a-z])/g, function ( g ) { + return g[ 1 ].toUpperCase(); + } ); +} + +/** + * Computes the median number from an array numbers. + * + * @param array List of numbers. + * @return Median. + */ +export function median( array: number[] ) { + const mid = Math.floor( array.length / 2 ); + const numbers = [ ...array ].sort( ( a, b ) => a - b ); + const result = + array.length % 2 !== 0 + ? numbers[ mid ] + : ( numbers[ mid - 1 ] + numbers[ mid ] ) / 2; + + return Number( result.toFixed( 2 ) ); +} From 51c787e1fcd1121f33778aa89ccb69a07df3ad45 Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Wed, 13 Mar 2024 16:28:33 +0000 Subject: [PATCH 02/16] add pull request step --- .github/workflows/perf-tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/perf-tests.yml b/.github/workflows/perf-tests.yml index aea4e96..266aa82 100644 --- a/.github/workflows/perf-tests.yml +++ b/.github/workflows/perf-tests.yml @@ -3,6 +3,7 @@ on: push: branches-ignore: - 'main-built' + pull_request: jobs: performance-tests: From 31bb2c154f1c4a7cc640cb382e89881cd2abf581 Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Wed, 13 Mar 2024 16:45:10 +0000 Subject: [PATCH 03/16] add performance results steo --- .github/workflows/perf-tests.yml | 4 ++-- package.json | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/perf-tests.yml b/.github/workflows/perf-tests.yml index 266aa82..b9d0dc9 100644 --- a/.github/workflows/perf-tests.yml +++ b/.github/workflows/perf-tests.yml @@ -82,9 +82,9 @@ jobs: - name: Compare results with base run: | if [ -f "${{ runner.temp }}/results_before.json" ]; then - npm run test:performance:results ${{ runner.temp }}/results_after.json ${{ runner.temp }}/results_before.json + yarn test:performance:results ${{ runner.temp }}/results_after.json ${{ runner.temp }}/results_before.json else - npm run test:performance:results ${{ runner.temp }}/results_after.json + yarn test:performance:results ${{ runner.temp }}/results_after.json fi; - name: Add workflow summary diff --git a/package.json b/package.json index f0e6314..784ec25 100644 --- a/package.json +++ b/package.json @@ -15,6 +15,8 @@ "test:e2e": "wp-scripts test-playwright", "test:js": "wp-scripts test-unit-js", "test:performance": "wp-scripts test-playwright --config tests/performance/playwright.config.ts", + "test:performance:merge-reports": "playwright merge-reports --reporter tests/performance/config/performance-reporter.ts ./blob-report", + "test:performance:results": "node tests/performance/cli/results.js", "test:watch": "wp-scripts test-unit-js --watch", "version:major": "node ./scripts/version major", "version:minor": "node ./scripts/version minor", From 8cc2fcdf76019adec8656f04a5c451da81aa8e62 Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Wed, 13 Mar 2024 16:58:00 +0000 Subject: [PATCH 04/16] exclude performance test in jests --- jest.config.js | 5 +- tests/performance/cli/results.js | 165 +++++++++--------- .../config/performance-reporter.ts | 59 ++++--- tests/performance/utils/index.ts | 20 +-- 4 files changed, 127 insertions(+), 122 deletions(-) diff --git a/jest.config.js b/jest.config.js index 7ebd5f5..4b2af1a 100644 --- a/jest.config.js +++ b/jest.config.js @@ -5,7 +5,10 @@ module.exports = { }, modulePathIgnorePatterns: ['/vendor/'], testEnvironment: 'jsdom', - testPathIgnorePatterns: ['/tests/e2e/'], + testPathIgnorePatterns: [ + '/tests/e2e/', + '/tests/performance/', + ], collectCoverage: true, coverageReporters: ['text', 'cobertura'], }; diff --git a/tests/performance/cli/results.js b/tests/performance/cli/results.js index efbaa2f..b1bcd80 100644 --- a/tests/performance/cli/results.js +++ b/tests/performance/cli/results.js @@ -1,37 +1,39 @@ #!/usr/bin/env node - /** * External dependencies */ -const { existsSync, readFileSync, writeFileSync } = require( 'node:fs' ); -const { join, resolve } = require( 'node:path' ); +// eslint-disable-next-line @typescript-eslint/no-var-requires +const { existsSync, readFileSync, writeFileSync } = require('node:fs'); +// eslint-disable-next-line @typescript-eslint/no-var-requires +const { join, resolve } = require('node:path'); -process.env.WP_ARTIFACTS_PATH ??= join( process.cwd(), 'artifacts' ); +process.env.WP_ARTIFACTS_PATH ??= join(process.cwd(), 'artifacts'); -const args = process.argv.slice( 2 ); +const args = process.argv.slice(2); -const beforeFile = args[ 1 ]; -const afterFile = - args[ 0 ] || resolve( './artifacts/performance-results.json' ); +const beforeFile = args[1]; +const afterFile = args[0] || resolve('./artifacts/performance-results.json'); -if ( ! existsSync( afterFile ) ) { - console.error( `File not found: ${ afterFile }` ); - process.exit( 1 ); +if (!existsSync(afterFile)) { + // eslint-disable-next-line no-console + console.error(`File not found: ${afterFile}`); + process.exit(1); } -if ( beforeFile && ! existsSync( beforeFile ) ) { - console.error( `File not found: ${ beforeFile }` ); - process.exit( 1 ); +if (beforeFile && !existsSync(beforeFile)) { + // eslint-disable-next-line no-console + console.error(`File not found: ${beforeFile}`); + process.exit(1); } /** * @param {unknown} v * @return {string} Formatted value. */ -function formatTableValue( v ) { - if ( v === true || v === 'true' ) return '✅'; - if ( ! v || v === 'false' ) return ''; - return v?.toString() || String( v ); +function formatTableValue(v) { + if (v === true || v === 'true') return '✅'; + if (!v || v === 'false') return ''; + return v?.toString() || String(v); } /** @@ -62,25 +64,22 @@ function formatTableValue( v ) { * @param {Array} rows Table rows. * @return {string} Markdown table content. */ -function formatAsMarkdownTable( rows ) { +function formatAsMarkdownTable(rows) { let result = ''; - const headers = Object.keys( rows[ 0 ] ); - for ( const header of headers ) { - result += `| ${ header } `; + const headers = Object.keys(rows[0]); + for (const header of headers) { + result += `| ${header} `; } result += '|\n'; - for ( const header of headers ) { - const dashes = '-'.repeat( header.length ); - result += `| ${ dashes } `; + for (const header of headers) { + const dashes = '-'.repeat(header.length); + result += `| ${dashes} `; } result += '|\n'; - for ( const row of rows ) { - for ( const [ key, value ] of Object.entries( row ) ) { - result += `| ${ formatTableValue( value ).padStart( - key.length, - ' ' - ) } `; + for (const row of rows) { + for (const [key, value] of Object.entries(row)) { + result += `| ${formatTableValue(value).padStart(key.length, ' ')} `; } result += '|\n'; } @@ -94,15 +93,15 @@ function formatAsMarkdownTable( rows ) { * @param {number[]} array List of numbers. * @return {number} Median. */ -function median( array ) { - const mid = Math.floor( array.length / 2 ); - const numbers = [ ...array ].sort( ( a, b ) => a - b ); +function median(array) { + const mid = Math.floor(array.length / 2); + const numbers = [...array].sort((a, b) => a - b); const result = array.length % 2 !== 0 - ? numbers[ mid ] - : ( numbers[ mid - 1 ] + numbers[ mid ] ) / 2; + ? numbers[mid] + : (numbers[mid - 1] + numbers[mid]) / 2; - return Number( result.toFixed( 2 ) ); + return Number(result.toFixed(2)); } /** @@ -115,36 +114,39 @@ let beforeStats = []; */ let afterStats; -if ( beforeFile ) { +if (beforeFile) { try { beforeStats = JSON.parse( - readFileSync( beforeFile, { encoding: 'utf-8' } ) + readFileSync(beforeFile, { encoding: 'utf-8' }) ); } catch {} } try { - afterStats = JSON.parse( readFileSync( afterFile, { encoding: 'utf-8' } ) ); + afterStats = JSON.parse(readFileSync(afterFile, { encoding: 'utf-8' })); } catch { - console.error( `Could not read file: ${ afterFile }` ); - process.exit( 1 ); + // eslint-disable-next-line no-console + console.error(`Could not read file: ${afterFile}`); + process.exit(1); } let summaryMarkdown = `**Performance Test Results**\n\n`; -if ( process.env.GITHUB_SHA ) { - summaryMarkdown += `Performance test results for ${ process.env.GITHUB_SHA } are in 🛎️!\n\n`; +if (process.env.GITHUB_SHA) { + summaryMarkdown += `Performance test results for ${process.env.GITHUB_SHA} are in 🛎️!\n\n`; } else { summaryMarkdown += `Performance test results are in 🛎️!\n\n`; } -if ( beforeFile ) { +if (beforeFile) { summaryMarkdown += `Note: the numbers in parentheses show the difference to the previous (baseline) test run.\n\n`; } -console.log( 'Performance Test Results\n' ); +// eslint-disable-next-line no-console +console.log('Performance Test Results\n'); -if ( beforeFile ) { +if (beforeFile) { + // eslint-disable-next-line no-console console.log( 'Note: the numbers in parentheses show the difference to the previous (baseline) test run.\n' ); @@ -164,32 +166,32 @@ const PERCENTAGE_VARIANCE = 2; * @param {string} key Key. * @return {string} Formatted value. */ -function formatValue( value, key ) { - if ( key === 'CLS' ) { - return value.toFixed( 2 ); +function formatValue(value, key) { + if (key === 'CLS') { + return value.toFixed(2); } - if ( key === 'wpDbQueries' ) { - return value.toFixed( 0 ); + if (key === 'wpDbQueries') { + return value.toFixed(0); } - if ( key === 'wpMemoryUsage' ) { - return `${ ( value / Math.pow( 10, 6 ) ).toFixed( 2 ) } MB`; + if (key === 'wpMemoryUsage') { + return `${(value / Math.pow(10, 6)).toFixed(2)} MB`; } - return `${ value.toFixed( 2 ) } ms`; + return `${value.toFixed(2)} ms`; } -for ( const { file, title, results } of afterStats ) { - const prevStat = beforeStats.find( ( s ) => s.file === file ); +for (const { file, title, results } of afterStats) { + const prevStat = beforeStats.find((s) => s.file === file); /** * @type {Array>} */ const diffResults = []; - for ( const i in results ) { - const newResult = results[ i ]; + for (const i in results) { + const newResult = results[i]; /** * @type {Record} @@ -198,28 +200,28 @@ for ( const { file, title, results } of afterStats ) { Run: i, }; - for ( const [ key, values ] of Object.entries( newResult ) ) { + for (const [key, values] of Object.entries(newResult)) { // Only do comparison if the number of results is the same. const prevValues = prevStat?.results.length === results.length - ? prevStat?.results[ i ].key + ? prevStat?.results[i].key : null; - const value = median( values ); - const prevValue = prevValues ? median( prevValues ) : 0; + const value = median(values); + const prevValue = prevValues ? median(prevValues) : 0; const delta = value - prevValue; - const percentage = ( delta / value ) * 100; + const percentage = (delta / value) * 100; // Skip if there is not a significant delta or none at all. if ( - ! prevValues || - ! percentage || - Math.abs( percentage ) <= PERCENTAGE_VARIANCE || - ! delta || - Math.abs( delta ) <= DELTA_VARIANCE + !prevValues || + !percentage || + Math.abs(percentage) <= PERCENTAGE_VARIANCE || + !delta || + Math.abs(delta) <= DELTA_VARIANCE ) { - diffResult[ key ] = formatValue( - /** @type {number} */ ( value ), + diffResult[key] = formatValue( + /** @type {number} */ (value), key ); continue; @@ -227,26 +229,25 @@ for ( const { file, title, results } of afterStats ) { const prefix = delta > 0 ? '+' : ''; - diffResult[ key ] = `${ formatValue( + diffResult[key] = `${formatValue( value, key - ) } (${ prefix }${ formatValue( - delta, - key - ) } / ${ prefix }${ percentage }%)`; + )} (${prefix}${formatValue(delta, key)} / ${prefix}${percentage}%)`; } - diffResults.push( diffResult ); + diffResults.push(diffResult); } - console.log( title ); - console.table( diffResults ); + // eslint-disable-next-line no-console + console.log(title); + // eslint-disable-next-line no-console + console.table(diffResults); - summaryMarkdown += `**${ title }**\n\n`; - summaryMarkdown += `${ formatAsMarkdownTable( diffResults ) }\n`; + summaryMarkdown += `**${title}**\n\n`; + summaryMarkdown += `${formatAsMarkdownTable(diffResults)}\n`; } writeFileSync( - join( process.env.WP_ARTIFACTS_PATH, '/performance-results.md' ), + join(process.env.WP_ARTIFACTS_PATH, '/performance-results.md'), summaryMarkdown ); diff --git a/tests/performance/config/performance-reporter.ts b/tests/performance/config/performance-reporter.ts index 7c00a80..0ed03b4 100644 --- a/tests/performance/config/performance-reporter.ts +++ b/tests/performance/config/performance-reporter.ts @@ -1,3 +1,4 @@ +/* eslint-disable no-console */ import { join } from 'node:path'; import { writeFileSync } from 'node:fs'; import type { @@ -9,21 +10,21 @@ import type { } from '@playwright/test/reporter'; import { median } from '../utils'; -process.env.WP_ARTIFACTS_PATH ??= join( process.cwd(), 'artifacts' ); +process.env.WP_ARTIFACTS_PATH ??= join(process.cwd(), 'artifacts'); class PerformanceReporter implements Reporter { - private shard?: FullConfig[ 'shard' ]; + private shard?: FullConfig['shard']; allResults: Record< string, { title: string; - results: Record< string, number[] >[]; + results: Record[]; } > = {}; - onBegin( config: FullConfig ) { - if ( config.shard ) { + onBegin(config: FullConfig) { + if (config.shard) { this.shard = config.shard; } } @@ -36,19 +37,19 @@ class PerformanceReporter implements Reporter { * @param test * @param result */ - onTestEnd( test: TestCase, result: TestResult ) { + onTestEnd(test: TestCase, result: TestResult) { const performanceResults = result.attachments.find( - ( attachment ) => attachment.name === 'results' + (attachment) => attachment.name === 'results' ); - if ( performanceResults?.body ) { - this.allResults[ test.location.file ] ??= { + if (performanceResults?.body) { + this.allResults[test.location.file] ??= { // 0 = empty, 1 = browser, 2 = file name, 3 = test suite name. - title: test.titlePath()[ 3 ], + title: test.titlePath()[3], results: [], }; - this.allResults[ test.location.file ].results.push( - JSON.parse( performanceResults.body.toString( 'utf-8' ) ) + this.allResults[test.location.file].results.push( + JSON.parse(performanceResults.body.toString('utf-8')) ); } } @@ -61,49 +62,49 @@ class PerformanceReporter implements Reporter { * * @param result */ - onEnd( result: FullResult ) { + onEnd(result: FullResult) { const summary = []; - if ( Object.keys( this.allResults ).length > 0 ) { - if ( this.shard ) { + if (Object.keys(this.allResults).length > 0) { + if (this.shard) { console.log( - `\nPerformance Test Results ${ this.shard.current }/${ this.shard.total }` + `\nPerformance Test Results ${this.shard.current}/${this.shard.total}` ); } else { - console.log( `\nPerformance Test Results` ); + console.log(`\nPerformance Test Results`); } - console.log( `Status: ${ result.status }` ); + console.log(`Status: ${result.status}`); } - for ( const [ file, { title, results } ] of Object.entries( + for (const [file, { title, results }] of Object.entries( this.allResults - ) ) { - console.log( `\n${ title }\n` ); + )) { + console.log(`\n${title}\n`); console.table( - results.map( ( r ) => + results.map((r) => Object.fromEntries( - Object.entries( r ).map( ( [ key, value ] ) => [ + Object.entries(r).map(([key, value]) => [ key, - median( value ), - ] ) + median(value), + ]) ) ) ); - summary.push( { + summary.push({ file, title, results, - } ); + }); } - if ( ! this.shard ) { + if (!this.shard) { writeFileSync( join( process.env.WP_ARTIFACTS_PATH as string, 'performance-results.json' ), - JSON.stringify( summary, null, 2 ) + JSON.stringify(summary, null, 2) ); } } diff --git a/tests/performance/utils/index.ts b/tests/performance/utils/index.ts index bcf2707..8d67ae0 100644 --- a/tests/performance/utils/index.ts +++ b/tests/performance/utils/index.ts @@ -3,10 +3,10 @@ * * @param str */ -export function camelCaseDashes( str: string ) { - return str.replace( /-([a-z])/g, function ( g ) { - return g[ 1 ].toUpperCase(); - } ); +export function camelCaseDashes(str: string) { + return str.replace(/-([a-z])/g, function (g) { + return g[1].toUpperCase(); + }); } /** @@ -15,13 +15,13 @@ export function camelCaseDashes( str: string ) { * @param array List of numbers. * @return Median. */ -export function median( array: number[] ) { - const mid = Math.floor( array.length / 2 ); - const numbers = [ ...array ].sort( ( a, b ) => a - b ); +export function median(array: number[]) { + const mid = Math.floor(array.length / 2); + const numbers = [...array].sort((a, b) => a - b); const result = array.length % 2 !== 0 - ? numbers[ mid ] - : ( numbers[ mid - 1 ] + numbers[ mid ] ) / 2; + ? numbers[mid] + : (numbers[mid - 1] + numbers[mid]) / 2; - return Number( result.toFixed( 2 ) ); + return Number(result.toFixed(2)); } From f8f7b4590184fcff27bf538a9acc8122f42c64d3 Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Wed, 13 Mar 2024 20:56:40 +0000 Subject: [PATCH 05/16] uses shard to get the perf report --- .github/workflows/perf-tests.yml | 78 +++++++++++++------------- tests/performance/playwright.config.ts | 2 +- 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/.github/workflows/perf-tests.yml b/.github/workflows/perf-tests.yml index b9d0dc9..446efc5 100644 --- a/.github/workflows/perf-tests.yml +++ b/.github/workflows/perf-tests.yml @@ -9,6 +9,10 @@ jobs: performance-tests: name: 'Performance Tests' runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + shard: [ 1/4, 2/4, 3/4, 4/4 ] env: WP_BASE_URL: 'http://localhost:8888' WP_USERNAME: 'admin' @@ -51,49 +55,45 @@ jobs: - name: Run tests run: | - yarn test:performance - mv ${{ env.WP_ARTIFACTS_PATH }}/performance-results.json ${{ runner.temp }}/results_after.json + yarn test:performance --shard ${{ matrix.shard }} - - name: Check out base commit - run: | - if [[ -z "$BASE_REF" ]]; then - git fetch -n origin $BASE_SHA - git reset --hard $BASE_SHA - else - git fetch -n origin $BASE_REF - git reset --hard $BASE_SHA - fi - env: - BASE_REF: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.ref || '' }} - BASE_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }} - - # Run tests without causing job to fail if they don't pass (e.g. because of env issues). - - name: Run tests for base - run: | - npm run test:performance || true - if [ -f "{{ env.WP_ARTIFACTS_PATH }}/performance-results.json" ]; then - mv ${{ env.WP_ARTIFACTS_PATH }}/performance-results.json ${{ runner.temp }}/results_before.json - fi; + - name: Upload blob report to GitHub Actions Artifacts + if: always() + uses: actions/upload-artifact@v4 + with: + name: all-blob-reports + path: blob-report + retention-days: 1 - - name: Reset to original commit - run: | - git reset --hard $GITHUB_SHA + merge-reports: + # Merge reports after playwright-tests, even if some shards have failed + if: always() + needs: [ performance-tests ] + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 - - name: Compare results with base - run: | - if [ -f "${{ runner.temp }}/results_before.json" ]; then - yarn test:performance:results ${{ runner.temp }}/results_after.json ${{ runner.temp }}/results_before.json - else - yarn test:performance:results ${{ runner.temp }}/results_after.json - fi; + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version-file: '.nvmrc' + cache: yarn - - name: Add workflow summary - run: | - cat ${{ env.WP_ARTIFACTS_PATH }}/performance-results.md >> $GITHUB_STEP_SUMMARY + - name: Install dependencies + run: yarn install --immutable + + - name: Download blob reports from GitHub Actions Artifacts + uses: actions/download-artifact@v4 + with: + name: all-blob-reports + path: all-blob-reports + + - name: Merge into single performance report + run: yarn test:performance:merge-reports - - name: Upload performance results - if: success() + - name: Upload performance report uses: actions/upload-artifact@v4 with: - name: performance-results - path: ${{ env.WP_ARTIFACTS_PATH }}/performance-results.json + name: performance-report-${{ github.run_attempt }} + path: artifacts/performance-results.json + retention-days: 14 diff --git a/tests/performance/playwright.config.ts b/tests/performance/playwright.config.ts index ae56a4a..fb8d13e 100644 --- a/tests/performance/playwright.config.ts +++ b/tests/performance/playwright.config.ts @@ -13,7 +13,7 @@ process.env.STORAGE_STATE_PATH ??= join( process.env.WP_ARTIFACTS_PATH, authStoragePath ); -process.env.TEST_ITERATIONS ??= '4'; +process.env.TEST_ITERATIONS ??= '20'; const config = defineConfig({ globalSetup: require.resolve('../e2e/global-setup.ts'), From ef5dd105cf2697f3a22c6bbfbfa227297d1ca36a Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Thu, 14 Mar 2024 09:31:54 +0000 Subject: [PATCH 06/16] uses upload artifact v3 --- .github/workflows/perf-tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/perf-tests.yml b/.github/workflows/perf-tests.yml index 446efc5..a90a07b 100644 --- a/.github/workflows/perf-tests.yml +++ b/.github/workflows/perf-tests.yml @@ -59,7 +59,7 @@ jobs: - name: Upload blob report to GitHub Actions Artifacts if: always() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v3 with: name: all-blob-reports path: blob-report @@ -79,7 +79,7 @@ jobs: node-version-file: '.nvmrc' cache: yarn - - name: Install dependencies + - name: Install dependencies. run: yarn install --immutable - name: Download blob reports from GitHub Actions Artifacts From 91f3592c2dad024b76b09d78fb5d4e1ff26cb692 Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Thu, 14 Mar 2024 09:44:58 +0000 Subject: [PATCH 07/16] uses download artifact v3 --- .github/workflows/perf-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/perf-tests.yml b/.github/workflows/perf-tests.yml index a90a07b..53e3758 100644 --- a/.github/workflows/perf-tests.yml +++ b/.github/workflows/perf-tests.yml @@ -83,7 +83,7 @@ jobs: run: yarn install --immutable - name: Download blob reports from GitHub Actions Artifacts - uses: actions/download-artifact@v4 + uses: actions/download-artifact@v3 with: name: all-blob-reports path: all-blob-reports From 893f33e094077707cca7e04b8699b7ea23f29278 Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Thu, 14 Mar 2024 10:00:50 +0000 Subject: [PATCH 08/16] use different dir for reports --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 784ec25..07f5cc8 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "test:e2e": "wp-scripts test-playwright", "test:js": "wp-scripts test-unit-js", "test:performance": "wp-scripts test-playwright --config tests/performance/playwright.config.ts", - "test:performance:merge-reports": "playwright merge-reports --reporter tests/performance/config/performance-reporter.ts ./blob-report", + "test:performance:merge-reports": "playwright merge-reports --reporter tests/performance/config/performance-reporter.ts ./all-blob-reports", "test:performance:results": "node tests/performance/cli/results.js", "test:watch": "wp-scripts test-unit-js --watch", "version:major": "node ./scripts/version major", From 7530c847878717b6c2ff7db69e1c52242973d158 Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Thu, 14 Mar 2024 10:09:46 +0000 Subject: [PATCH 09/16] uses v3 upload --- .github/workflows/perf-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/perf-tests.yml b/.github/workflows/perf-tests.yml index 53e3758..1e2befa 100644 --- a/.github/workflows/perf-tests.yml +++ b/.github/workflows/perf-tests.yml @@ -92,7 +92,7 @@ jobs: run: yarn test:performance:merge-reports - name: Upload performance report - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v3 with: name: performance-report-${{ github.run_attempt }} path: artifacts/performance-results.json From 97fdc230cb4deb861f135e2d072d58c1d4ab04b5 Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Thu, 14 Mar 2024 10:20:06 +0000 Subject: [PATCH 10/16] Revert "exclude performance test in jests" This reverts commit 8cc2fcdf76019adec8656f04a5c451da81aa8e62. --- jest.config.js | 5 +- tests/performance/cli/results.js | 165 +++++++++--------- .../config/performance-reporter.ts | 59 +++---- tests/performance/utils/index.ts | 20 +-- 4 files changed, 122 insertions(+), 127 deletions(-) diff --git a/jest.config.js b/jest.config.js index 4b2af1a..7ebd5f5 100644 --- a/jest.config.js +++ b/jest.config.js @@ -5,10 +5,7 @@ module.exports = { }, modulePathIgnorePatterns: ['/vendor/'], testEnvironment: 'jsdom', - testPathIgnorePatterns: [ - '/tests/e2e/', - '/tests/performance/', - ], + testPathIgnorePatterns: ['/tests/e2e/'], collectCoverage: true, coverageReporters: ['text', 'cobertura'], }; diff --git a/tests/performance/cli/results.js b/tests/performance/cli/results.js index b1bcd80..efbaa2f 100644 --- a/tests/performance/cli/results.js +++ b/tests/performance/cli/results.js @@ -1,39 +1,37 @@ #!/usr/bin/env node + /** * External dependencies */ -// eslint-disable-next-line @typescript-eslint/no-var-requires -const { existsSync, readFileSync, writeFileSync } = require('node:fs'); -// eslint-disable-next-line @typescript-eslint/no-var-requires -const { join, resolve } = require('node:path'); +const { existsSync, readFileSync, writeFileSync } = require( 'node:fs' ); +const { join, resolve } = require( 'node:path' ); -process.env.WP_ARTIFACTS_PATH ??= join(process.cwd(), 'artifacts'); +process.env.WP_ARTIFACTS_PATH ??= join( process.cwd(), 'artifacts' ); -const args = process.argv.slice(2); +const args = process.argv.slice( 2 ); -const beforeFile = args[1]; -const afterFile = args[0] || resolve('./artifacts/performance-results.json'); +const beforeFile = args[ 1 ]; +const afterFile = + args[ 0 ] || resolve( './artifacts/performance-results.json' ); -if (!existsSync(afterFile)) { - // eslint-disable-next-line no-console - console.error(`File not found: ${afterFile}`); - process.exit(1); +if ( ! existsSync( afterFile ) ) { + console.error( `File not found: ${ afterFile }` ); + process.exit( 1 ); } -if (beforeFile && !existsSync(beforeFile)) { - // eslint-disable-next-line no-console - console.error(`File not found: ${beforeFile}`); - process.exit(1); +if ( beforeFile && ! existsSync( beforeFile ) ) { + console.error( `File not found: ${ beforeFile }` ); + process.exit( 1 ); } /** * @param {unknown} v * @return {string} Formatted value. */ -function formatTableValue(v) { - if (v === true || v === 'true') return '✅'; - if (!v || v === 'false') return ''; - return v?.toString() || String(v); +function formatTableValue( v ) { + if ( v === true || v === 'true' ) return '✅'; + if ( ! v || v === 'false' ) return ''; + return v?.toString() || String( v ); } /** @@ -64,22 +62,25 @@ function formatTableValue(v) { * @param {Array} rows Table rows. * @return {string} Markdown table content. */ -function formatAsMarkdownTable(rows) { +function formatAsMarkdownTable( rows ) { let result = ''; - const headers = Object.keys(rows[0]); - for (const header of headers) { - result += `| ${header} `; + const headers = Object.keys( rows[ 0 ] ); + for ( const header of headers ) { + result += `| ${ header } `; } result += '|\n'; - for (const header of headers) { - const dashes = '-'.repeat(header.length); - result += `| ${dashes} `; + for ( const header of headers ) { + const dashes = '-'.repeat( header.length ); + result += `| ${ dashes } `; } result += '|\n'; - for (const row of rows) { - for (const [key, value] of Object.entries(row)) { - result += `| ${formatTableValue(value).padStart(key.length, ' ')} `; + for ( const row of rows ) { + for ( const [ key, value ] of Object.entries( row ) ) { + result += `| ${ formatTableValue( value ).padStart( + key.length, + ' ' + ) } `; } result += '|\n'; } @@ -93,15 +94,15 @@ function formatAsMarkdownTable(rows) { * @param {number[]} array List of numbers. * @return {number} Median. */ -function median(array) { - const mid = Math.floor(array.length / 2); - const numbers = [...array].sort((a, b) => a - b); +function median( array ) { + const mid = Math.floor( array.length / 2 ); + const numbers = [ ...array ].sort( ( a, b ) => a - b ); const result = array.length % 2 !== 0 - ? numbers[mid] - : (numbers[mid - 1] + numbers[mid]) / 2; + ? numbers[ mid ] + : ( numbers[ mid - 1 ] + numbers[ mid ] ) / 2; - return Number(result.toFixed(2)); + return Number( result.toFixed( 2 ) ); } /** @@ -114,39 +115,36 @@ let beforeStats = []; */ let afterStats; -if (beforeFile) { +if ( beforeFile ) { try { beforeStats = JSON.parse( - readFileSync(beforeFile, { encoding: 'utf-8' }) + readFileSync( beforeFile, { encoding: 'utf-8' } ) ); } catch {} } try { - afterStats = JSON.parse(readFileSync(afterFile, { encoding: 'utf-8' })); + afterStats = JSON.parse( readFileSync( afterFile, { encoding: 'utf-8' } ) ); } catch { - // eslint-disable-next-line no-console - console.error(`Could not read file: ${afterFile}`); - process.exit(1); + console.error( `Could not read file: ${ afterFile }` ); + process.exit( 1 ); } let summaryMarkdown = `**Performance Test Results**\n\n`; -if (process.env.GITHUB_SHA) { - summaryMarkdown += `Performance test results for ${process.env.GITHUB_SHA} are in 🛎️!\n\n`; +if ( process.env.GITHUB_SHA ) { + summaryMarkdown += `Performance test results for ${ process.env.GITHUB_SHA } are in 🛎️!\n\n`; } else { summaryMarkdown += `Performance test results are in 🛎️!\n\n`; } -if (beforeFile) { +if ( beforeFile ) { summaryMarkdown += `Note: the numbers in parentheses show the difference to the previous (baseline) test run.\n\n`; } -// eslint-disable-next-line no-console -console.log('Performance Test Results\n'); +console.log( 'Performance Test Results\n' ); -if (beforeFile) { - // eslint-disable-next-line no-console +if ( beforeFile ) { console.log( 'Note: the numbers in parentheses show the difference to the previous (baseline) test run.\n' ); @@ -166,32 +164,32 @@ const PERCENTAGE_VARIANCE = 2; * @param {string} key Key. * @return {string} Formatted value. */ -function formatValue(value, key) { - if (key === 'CLS') { - return value.toFixed(2); +function formatValue( value, key ) { + if ( key === 'CLS' ) { + return value.toFixed( 2 ); } - if (key === 'wpDbQueries') { - return value.toFixed(0); + if ( key === 'wpDbQueries' ) { + return value.toFixed( 0 ); } - if (key === 'wpMemoryUsage') { - return `${(value / Math.pow(10, 6)).toFixed(2)} MB`; + if ( key === 'wpMemoryUsage' ) { + return `${ ( value / Math.pow( 10, 6 ) ).toFixed( 2 ) } MB`; } - return `${value.toFixed(2)} ms`; + return `${ value.toFixed( 2 ) } ms`; } -for (const { file, title, results } of afterStats) { - const prevStat = beforeStats.find((s) => s.file === file); +for ( const { file, title, results } of afterStats ) { + const prevStat = beforeStats.find( ( s ) => s.file === file ); /** * @type {Array>} */ const diffResults = []; - for (const i in results) { - const newResult = results[i]; + for ( const i in results ) { + const newResult = results[ i ]; /** * @type {Record} @@ -200,28 +198,28 @@ for (const { file, title, results } of afterStats) { Run: i, }; - for (const [key, values] of Object.entries(newResult)) { + for ( const [ key, values ] of Object.entries( newResult ) ) { // Only do comparison if the number of results is the same. const prevValues = prevStat?.results.length === results.length - ? prevStat?.results[i].key + ? prevStat?.results[ i ].key : null; - const value = median(values); - const prevValue = prevValues ? median(prevValues) : 0; + const value = median( values ); + const prevValue = prevValues ? median( prevValues ) : 0; const delta = value - prevValue; - const percentage = (delta / value) * 100; + const percentage = ( delta / value ) * 100; // Skip if there is not a significant delta or none at all. if ( - !prevValues || - !percentage || - Math.abs(percentage) <= PERCENTAGE_VARIANCE || - !delta || - Math.abs(delta) <= DELTA_VARIANCE + ! prevValues || + ! percentage || + Math.abs( percentage ) <= PERCENTAGE_VARIANCE || + ! delta || + Math.abs( delta ) <= DELTA_VARIANCE ) { - diffResult[key] = formatValue( - /** @type {number} */ (value), + diffResult[ key ] = formatValue( + /** @type {number} */ ( value ), key ); continue; @@ -229,25 +227,26 @@ for (const { file, title, results } of afterStats) { const prefix = delta > 0 ? '+' : ''; - diffResult[key] = `${formatValue( + diffResult[ key ] = `${ formatValue( value, key - )} (${prefix}${formatValue(delta, key)} / ${prefix}${percentage}%)`; + ) } (${ prefix }${ formatValue( + delta, + key + ) } / ${ prefix }${ percentage }%)`; } - diffResults.push(diffResult); + diffResults.push( diffResult ); } - // eslint-disable-next-line no-console - console.log(title); - // eslint-disable-next-line no-console - console.table(diffResults); + console.log( title ); + console.table( diffResults ); - summaryMarkdown += `**${title}**\n\n`; - summaryMarkdown += `${formatAsMarkdownTable(diffResults)}\n`; + summaryMarkdown += `**${ title }**\n\n`; + summaryMarkdown += `${ formatAsMarkdownTable( diffResults ) }\n`; } writeFileSync( - join(process.env.WP_ARTIFACTS_PATH, '/performance-results.md'), + join( process.env.WP_ARTIFACTS_PATH, '/performance-results.md' ), summaryMarkdown ); diff --git a/tests/performance/config/performance-reporter.ts b/tests/performance/config/performance-reporter.ts index 0ed03b4..7c00a80 100644 --- a/tests/performance/config/performance-reporter.ts +++ b/tests/performance/config/performance-reporter.ts @@ -1,4 +1,3 @@ -/* eslint-disable no-console */ import { join } from 'node:path'; import { writeFileSync } from 'node:fs'; import type { @@ -10,21 +9,21 @@ import type { } from '@playwright/test/reporter'; import { median } from '../utils'; -process.env.WP_ARTIFACTS_PATH ??= join(process.cwd(), 'artifacts'); +process.env.WP_ARTIFACTS_PATH ??= join( process.cwd(), 'artifacts' ); class PerformanceReporter implements Reporter { - private shard?: FullConfig['shard']; + private shard?: FullConfig[ 'shard' ]; allResults: Record< string, { title: string; - results: Record[]; + results: Record< string, number[] >[]; } > = {}; - onBegin(config: FullConfig) { - if (config.shard) { + onBegin( config: FullConfig ) { + if ( config.shard ) { this.shard = config.shard; } } @@ -37,19 +36,19 @@ class PerformanceReporter implements Reporter { * @param test * @param result */ - onTestEnd(test: TestCase, result: TestResult) { + onTestEnd( test: TestCase, result: TestResult ) { const performanceResults = result.attachments.find( - (attachment) => attachment.name === 'results' + ( attachment ) => attachment.name === 'results' ); - if (performanceResults?.body) { - this.allResults[test.location.file] ??= { + if ( performanceResults?.body ) { + this.allResults[ test.location.file ] ??= { // 0 = empty, 1 = browser, 2 = file name, 3 = test suite name. - title: test.titlePath()[3], + title: test.titlePath()[ 3 ], results: [], }; - this.allResults[test.location.file].results.push( - JSON.parse(performanceResults.body.toString('utf-8')) + this.allResults[ test.location.file ].results.push( + JSON.parse( performanceResults.body.toString( 'utf-8' ) ) ); } } @@ -62,49 +61,49 @@ class PerformanceReporter implements Reporter { * * @param result */ - onEnd(result: FullResult) { + onEnd( result: FullResult ) { const summary = []; - if (Object.keys(this.allResults).length > 0) { - if (this.shard) { + if ( Object.keys( this.allResults ).length > 0 ) { + if ( this.shard ) { console.log( - `\nPerformance Test Results ${this.shard.current}/${this.shard.total}` + `\nPerformance Test Results ${ this.shard.current }/${ this.shard.total }` ); } else { - console.log(`\nPerformance Test Results`); + console.log( `\nPerformance Test Results` ); } - console.log(`Status: ${result.status}`); + console.log( `Status: ${ result.status }` ); } - for (const [file, { title, results }] of Object.entries( + for ( const [ file, { title, results } ] of Object.entries( this.allResults - )) { - console.log(`\n${title}\n`); + ) ) { + console.log( `\n${ title }\n` ); console.table( - results.map((r) => + results.map( ( r ) => Object.fromEntries( - Object.entries(r).map(([key, value]) => [ + Object.entries( r ).map( ( [ key, value ] ) => [ key, - median(value), - ]) + median( value ), + ] ) ) ) ); - summary.push({ + summary.push( { file, title, results, - }); + } ); } - if (!this.shard) { + if ( ! this.shard ) { writeFileSync( join( process.env.WP_ARTIFACTS_PATH as string, 'performance-results.json' ), - JSON.stringify(summary, null, 2) + JSON.stringify( summary, null, 2 ) ); } } diff --git a/tests/performance/utils/index.ts b/tests/performance/utils/index.ts index 8d67ae0..bcf2707 100644 --- a/tests/performance/utils/index.ts +++ b/tests/performance/utils/index.ts @@ -3,10 +3,10 @@ * * @param str */ -export function camelCaseDashes(str: string) { - return str.replace(/-([a-z])/g, function (g) { - return g[1].toUpperCase(); - }); +export function camelCaseDashes( str: string ) { + return str.replace( /-([a-z])/g, function ( g ) { + return g[ 1 ].toUpperCase(); + } ); } /** @@ -15,13 +15,13 @@ export function camelCaseDashes(str: string) { * @param array List of numbers. * @return Median. */ -export function median(array: number[]) { - const mid = Math.floor(array.length / 2); - const numbers = [...array].sort((a, b) => a - b); +export function median( array: number[] ) { + const mid = Math.floor( array.length / 2 ); + const numbers = [ ...array ].sort( ( a, b ) => a - b ); const result = array.length % 2 !== 0 - ? numbers[mid] - : (numbers[mid - 1] + numbers[mid]) / 2; + ? numbers[ mid ] + : ( numbers[ mid - 1 ] + numbers[ mid ] ) / 2; - return Number(result.toFixed(2)); + return Number( result.toFixed( 2 ) ); } From 71b90a4be0269b7f44f65b3e1a12260f32a932c4 Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Thu, 14 Mar 2024 11:01:33 +0000 Subject: [PATCH 11/16] Revert "Revert "exclude performance test in jests"" This reverts commit 97fdc230cb4deb861f135e2d072d58c1d4ab04b5. --- jest.config.js | 5 +- tests/performance/cli/results.js | 165 +++++++++--------- .../config/performance-reporter.ts | 59 ++++--- tests/performance/utils/index.ts | 20 +-- 4 files changed, 127 insertions(+), 122 deletions(-) diff --git a/jest.config.js b/jest.config.js index 7ebd5f5..4b2af1a 100644 --- a/jest.config.js +++ b/jest.config.js @@ -5,7 +5,10 @@ module.exports = { }, modulePathIgnorePatterns: ['/vendor/'], testEnvironment: 'jsdom', - testPathIgnorePatterns: ['/tests/e2e/'], + testPathIgnorePatterns: [ + '/tests/e2e/', + '/tests/performance/', + ], collectCoverage: true, coverageReporters: ['text', 'cobertura'], }; diff --git a/tests/performance/cli/results.js b/tests/performance/cli/results.js index efbaa2f..b1bcd80 100644 --- a/tests/performance/cli/results.js +++ b/tests/performance/cli/results.js @@ -1,37 +1,39 @@ #!/usr/bin/env node - /** * External dependencies */ -const { existsSync, readFileSync, writeFileSync } = require( 'node:fs' ); -const { join, resolve } = require( 'node:path' ); +// eslint-disable-next-line @typescript-eslint/no-var-requires +const { existsSync, readFileSync, writeFileSync } = require('node:fs'); +// eslint-disable-next-line @typescript-eslint/no-var-requires +const { join, resolve } = require('node:path'); -process.env.WP_ARTIFACTS_PATH ??= join( process.cwd(), 'artifacts' ); +process.env.WP_ARTIFACTS_PATH ??= join(process.cwd(), 'artifacts'); -const args = process.argv.slice( 2 ); +const args = process.argv.slice(2); -const beforeFile = args[ 1 ]; -const afterFile = - args[ 0 ] || resolve( './artifacts/performance-results.json' ); +const beforeFile = args[1]; +const afterFile = args[0] || resolve('./artifacts/performance-results.json'); -if ( ! existsSync( afterFile ) ) { - console.error( `File not found: ${ afterFile }` ); - process.exit( 1 ); +if (!existsSync(afterFile)) { + // eslint-disable-next-line no-console + console.error(`File not found: ${afterFile}`); + process.exit(1); } -if ( beforeFile && ! existsSync( beforeFile ) ) { - console.error( `File not found: ${ beforeFile }` ); - process.exit( 1 ); +if (beforeFile && !existsSync(beforeFile)) { + // eslint-disable-next-line no-console + console.error(`File not found: ${beforeFile}`); + process.exit(1); } /** * @param {unknown} v * @return {string} Formatted value. */ -function formatTableValue( v ) { - if ( v === true || v === 'true' ) return '✅'; - if ( ! v || v === 'false' ) return ''; - return v?.toString() || String( v ); +function formatTableValue(v) { + if (v === true || v === 'true') return '✅'; + if (!v || v === 'false') return ''; + return v?.toString() || String(v); } /** @@ -62,25 +64,22 @@ function formatTableValue( v ) { * @param {Array} rows Table rows. * @return {string} Markdown table content. */ -function formatAsMarkdownTable( rows ) { +function formatAsMarkdownTable(rows) { let result = ''; - const headers = Object.keys( rows[ 0 ] ); - for ( const header of headers ) { - result += `| ${ header } `; + const headers = Object.keys(rows[0]); + for (const header of headers) { + result += `| ${header} `; } result += '|\n'; - for ( const header of headers ) { - const dashes = '-'.repeat( header.length ); - result += `| ${ dashes } `; + for (const header of headers) { + const dashes = '-'.repeat(header.length); + result += `| ${dashes} `; } result += '|\n'; - for ( const row of rows ) { - for ( const [ key, value ] of Object.entries( row ) ) { - result += `| ${ formatTableValue( value ).padStart( - key.length, - ' ' - ) } `; + for (const row of rows) { + for (const [key, value] of Object.entries(row)) { + result += `| ${formatTableValue(value).padStart(key.length, ' ')} `; } result += '|\n'; } @@ -94,15 +93,15 @@ function formatAsMarkdownTable( rows ) { * @param {number[]} array List of numbers. * @return {number} Median. */ -function median( array ) { - const mid = Math.floor( array.length / 2 ); - const numbers = [ ...array ].sort( ( a, b ) => a - b ); +function median(array) { + const mid = Math.floor(array.length / 2); + const numbers = [...array].sort((a, b) => a - b); const result = array.length % 2 !== 0 - ? numbers[ mid ] - : ( numbers[ mid - 1 ] + numbers[ mid ] ) / 2; + ? numbers[mid] + : (numbers[mid - 1] + numbers[mid]) / 2; - return Number( result.toFixed( 2 ) ); + return Number(result.toFixed(2)); } /** @@ -115,36 +114,39 @@ let beforeStats = []; */ let afterStats; -if ( beforeFile ) { +if (beforeFile) { try { beforeStats = JSON.parse( - readFileSync( beforeFile, { encoding: 'utf-8' } ) + readFileSync(beforeFile, { encoding: 'utf-8' }) ); } catch {} } try { - afterStats = JSON.parse( readFileSync( afterFile, { encoding: 'utf-8' } ) ); + afterStats = JSON.parse(readFileSync(afterFile, { encoding: 'utf-8' })); } catch { - console.error( `Could not read file: ${ afterFile }` ); - process.exit( 1 ); + // eslint-disable-next-line no-console + console.error(`Could not read file: ${afterFile}`); + process.exit(1); } let summaryMarkdown = `**Performance Test Results**\n\n`; -if ( process.env.GITHUB_SHA ) { - summaryMarkdown += `Performance test results for ${ process.env.GITHUB_SHA } are in 🛎️!\n\n`; +if (process.env.GITHUB_SHA) { + summaryMarkdown += `Performance test results for ${process.env.GITHUB_SHA} are in 🛎️!\n\n`; } else { summaryMarkdown += `Performance test results are in 🛎️!\n\n`; } -if ( beforeFile ) { +if (beforeFile) { summaryMarkdown += `Note: the numbers in parentheses show the difference to the previous (baseline) test run.\n\n`; } -console.log( 'Performance Test Results\n' ); +// eslint-disable-next-line no-console +console.log('Performance Test Results\n'); -if ( beforeFile ) { +if (beforeFile) { + // eslint-disable-next-line no-console console.log( 'Note: the numbers in parentheses show the difference to the previous (baseline) test run.\n' ); @@ -164,32 +166,32 @@ const PERCENTAGE_VARIANCE = 2; * @param {string} key Key. * @return {string} Formatted value. */ -function formatValue( value, key ) { - if ( key === 'CLS' ) { - return value.toFixed( 2 ); +function formatValue(value, key) { + if (key === 'CLS') { + return value.toFixed(2); } - if ( key === 'wpDbQueries' ) { - return value.toFixed( 0 ); + if (key === 'wpDbQueries') { + return value.toFixed(0); } - if ( key === 'wpMemoryUsage' ) { - return `${ ( value / Math.pow( 10, 6 ) ).toFixed( 2 ) } MB`; + if (key === 'wpMemoryUsage') { + return `${(value / Math.pow(10, 6)).toFixed(2)} MB`; } - return `${ value.toFixed( 2 ) } ms`; + return `${value.toFixed(2)} ms`; } -for ( const { file, title, results } of afterStats ) { - const prevStat = beforeStats.find( ( s ) => s.file === file ); +for (const { file, title, results } of afterStats) { + const prevStat = beforeStats.find((s) => s.file === file); /** * @type {Array>} */ const diffResults = []; - for ( const i in results ) { - const newResult = results[ i ]; + for (const i in results) { + const newResult = results[i]; /** * @type {Record} @@ -198,28 +200,28 @@ for ( const { file, title, results } of afterStats ) { Run: i, }; - for ( const [ key, values ] of Object.entries( newResult ) ) { + for (const [key, values] of Object.entries(newResult)) { // Only do comparison if the number of results is the same. const prevValues = prevStat?.results.length === results.length - ? prevStat?.results[ i ].key + ? prevStat?.results[i].key : null; - const value = median( values ); - const prevValue = prevValues ? median( prevValues ) : 0; + const value = median(values); + const prevValue = prevValues ? median(prevValues) : 0; const delta = value - prevValue; - const percentage = ( delta / value ) * 100; + const percentage = (delta / value) * 100; // Skip if there is not a significant delta or none at all. if ( - ! prevValues || - ! percentage || - Math.abs( percentage ) <= PERCENTAGE_VARIANCE || - ! delta || - Math.abs( delta ) <= DELTA_VARIANCE + !prevValues || + !percentage || + Math.abs(percentage) <= PERCENTAGE_VARIANCE || + !delta || + Math.abs(delta) <= DELTA_VARIANCE ) { - diffResult[ key ] = formatValue( - /** @type {number} */ ( value ), + diffResult[key] = formatValue( + /** @type {number} */ (value), key ); continue; @@ -227,26 +229,25 @@ for ( const { file, title, results } of afterStats ) { const prefix = delta > 0 ? '+' : ''; - diffResult[ key ] = `${ formatValue( + diffResult[key] = `${formatValue( value, key - ) } (${ prefix }${ formatValue( - delta, - key - ) } / ${ prefix }${ percentage }%)`; + )} (${prefix}${formatValue(delta, key)} / ${prefix}${percentage}%)`; } - diffResults.push( diffResult ); + diffResults.push(diffResult); } - console.log( title ); - console.table( diffResults ); + // eslint-disable-next-line no-console + console.log(title); + // eslint-disable-next-line no-console + console.table(diffResults); - summaryMarkdown += `**${ title }**\n\n`; - summaryMarkdown += `${ formatAsMarkdownTable( diffResults ) }\n`; + summaryMarkdown += `**${title}**\n\n`; + summaryMarkdown += `${formatAsMarkdownTable(diffResults)}\n`; } writeFileSync( - join( process.env.WP_ARTIFACTS_PATH, '/performance-results.md' ), + join(process.env.WP_ARTIFACTS_PATH, '/performance-results.md'), summaryMarkdown ); diff --git a/tests/performance/config/performance-reporter.ts b/tests/performance/config/performance-reporter.ts index 7c00a80..0ed03b4 100644 --- a/tests/performance/config/performance-reporter.ts +++ b/tests/performance/config/performance-reporter.ts @@ -1,3 +1,4 @@ +/* eslint-disable no-console */ import { join } from 'node:path'; import { writeFileSync } from 'node:fs'; import type { @@ -9,21 +10,21 @@ import type { } from '@playwright/test/reporter'; import { median } from '../utils'; -process.env.WP_ARTIFACTS_PATH ??= join( process.cwd(), 'artifacts' ); +process.env.WP_ARTIFACTS_PATH ??= join(process.cwd(), 'artifacts'); class PerformanceReporter implements Reporter { - private shard?: FullConfig[ 'shard' ]; + private shard?: FullConfig['shard']; allResults: Record< string, { title: string; - results: Record< string, number[] >[]; + results: Record[]; } > = {}; - onBegin( config: FullConfig ) { - if ( config.shard ) { + onBegin(config: FullConfig) { + if (config.shard) { this.shard = config.shard; } } @@ -36,19 +37,19 @@ class PerformanceReporter implements Reporter { * @param test * @param result */ - onTestEnd( test: TestCase, result: TestResult ) { + onTestEnd(test: TestCase, result: TestResult) { const performanceResults = result.attachments.find( - ( attachment ) => attachment.name === 'results' + (attachment) => attachment.name === 'results' ); - if ( performanceResults?.body ) { - this.allResults[ test.location.file ] ??= { + if (performanceResults?.body) { + this.allResults[test.location.file] ??= { // 0 = empty, 1 = browser, 2 = file name, 3 = test suite name. - title: test.titlePath()[ 3 ], + title: test.titlePath()[3], results: [], }; - this.allResults[ test.location.file ].results.push( - JSON.parse( performanceResults.body.toString( 'utf-8' ) ) + this.allResults[test.location.file].results.push( + JSON.parse(performanceResults.body.toString('utf-8')) ); } } @@ -61,49 +62,49 @@ class PerformanceReporter implements Reporter { * * @param result */ - onEnd( result: FullResult ) { + onEnd(result: FullResult) { const summary = []; - if ( Object.keys( this.allResults ).length > 0 ) { - if ( this.shard ) { + if (Object.keys(this.allResults).length > 0) { + if (this.shard) { console.log( - `\nPerformance Test Results ${ this.shard.current }/${ this.shard.total }` + `\nPerformance Test Results ${this.shard.current}/${this.shard.total}` ); } else { - console.log( `\nPerformance Test Results` ); + console.log(`\nPerformance Test Results`); } - console.log( `Status: ${ result.status }` ); + console.log(`Status: ${result.status}`); } - for ( const [ file, { title, results } ] of Object.entries( + for (const [file, { title, results }] of Object.entries( this.allResults - ) ) { - console.log( `\n${ title }\n` ); + )) { + console.log(`\n${title}\n`); console.table( - results.map( ( r ) => + results.map((r) => Object.fromEntries( - Object.entries( r ).map( ( [ key, value ] ) => [ + Object.entries(r).map(([key, value]) => [ key, - median( value ), - ] ) + median(value), + ]) ) ) ); - summary.push( { + summary.push({ file, title, results, - } ); + }); } - if ( ! this.shard ) { + if (!this.shard) { writeFileSync( join( process.env.WP_ARTIFACTS_PATH as string, 'performance-results.json' ), - JSON.stringify( summary, null, 2 ) + JSON.stringify(summary, null, 2) ); } } diff --git a/tests/performance/utils/index.ts b/tests/performance/utils/index.ts index bcf2707..8d67ae0 100644 --- a/tests/performance/utils/index.ts +++ b/tests/performance/utils/index.ts @@ -3,10 +3,10 @@ * * @param str */ -export function camelCaseDashes( str: string ) { - return str.replace( /-([a-z])/g, function ( g ) { - return g[ 1 ].toUpperCase(); - } ); +export function camelCaseDashes(str: string) { + return str.replace(/-([a-z])/g, function (g) { + return g[1].toUpperCase(); + }); } /** @@ -15,13 +15,13 @@ export function camelCaseDashes( str: string ) { * @param array List of numbers. * @return Median. */ -export function median( array: number[] ) { - const mid = Math.floor( array.length / 2 ); - const numbers = [ ...array ].sort( ( a, b ) => a - b ); +export function median(array: number[]) { + const mid = Math.floor(array.length / 2); + const numbers = [...array].sort((a, b) => a - b); const result = array.length % 2 !== 0 - ? numbers[ mid ] - : ( numbers[ mid - 1 ] + numbers[ mid ] ) / 2; + ? numbers[mid] + : (numbers[mid - 1] + numbers[mid]) / 2; - return Number( result.toFixed( 2 ) ); + return Number(result.toFixed(2)); } From 0a3539acb33ad48027dd9e5714197331c97b2d2c Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Thu, 14 Mar 2024 11:01:43 +0000 Subject: [PATCH 12/16] Revert "uses v3 upload" This reverts commit 7530c847878717b6c2ff7db69e1c52242973d158. --- .github/workflows/perf-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/perf-tests.yml b/.github/workflows/perf-tests.yml index 1e2befa..53e3758 100644 --- a/.github/workflows/perf-tests.yml +++ b/.github/workflows/perf-tests.yml @@ -92,7 +92,7 @@ jobs: run: yarn test:performance:merge-reports - name: Upload performance report - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: performance-report-${{ github.run_attempt }} path: artifacts/performance-results.json From bbaff1abbe427ab0aea905fe5a25696bff2b7785 Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Thu, 14 Mar 2024 11:01:47 +0000 Subject: [PATCH 13/16] Revert "use different dir for reports" This reverts commit 893f33e094077707cca7e04b8699b7ea23f29278. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 07f5cc8..784ec25 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "test:e2e": "wp-scripts test-playwright", "test:js": "wp-scripts test-unit-js", "test:performance": "wp-scripts test-playwright --config tests/performance/playwright.config.ts", - "test:performance:merge-reports": "playwright merge-reports --reporter tests/performance/config/performance-reporter.ts ./all-blob-reports", + "test:performance:merge-reports": "playwright merge-reports --reporter tests/performance/config/performance-reporter.ts ./blob-report", "test:performance:results": "node tests/performance/cli/results.js", "test:watch": "wp-scripts test-unit-js --watch", "version:major": "node ./scripts/version major", From f9173a819a00cbeaa53dd10df771ecff7f16a15a Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Thu, 14 Mar 2024 11:01:51 +0000 Subject: [PATCH 14/16] Revert "uses download artifact v3" This reverts commit 91f3592c2dad024b76b09d78fb5d4e1ff26cb692. --- .github/workflows/perf-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/perf-tests.yml b/.github/workflows/perf-tests.yml index 53e3758..a90a07b 100644 --- a/.github/workflows/perf-tests.yml +++ b/.github/workflows/perf-tests.yml @@ -83,7 +83,7 @@ jobs: run: yarn install --immutable - name: Download blob reports from GitHub Actions Artifacts - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: all-blob-reports path: all-blob-reports From 069e1d3b2ce7ef8dd296c836c37d5e300388731f Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Thu, 14 Mar 2024 11:01:55 +0000 Subject: [PATCH 15/16] Revert "uses upload artifact v3" This reverts commit ef5dd105cf2697f3a22c6bbfbfa227297d1ca36a. --- .github/workflows/perf-tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/perf-tests.yml b/.github/workflows/perf-tests.yml index a90a07b..446efc5 100644 --- a/.github/workflows/perf-tests.yml +++ b/.github/workflows/perf-tests.yml @@ -59,7 +59,7 @@ jobs: - name: Upload blob report to GitHub Actions Artifacts if: always() - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: all-blob-reports path: blob-report @@ -79,7 +79,7 @@ jobs: node-version-file: '.nvmrc' cache: yarn - - name: Install dependencies. + - name: Install dependencies run: yarn install --immutable - name: Download blob reports from GitHub Actions Artifacts From 9d354a93a4465fe6d55ebbd35fb5afbaaaeb8407 Mon Sep 17 00:00:00 2001 From: Mohan Raj Date: Thu, 14 Mar 2024 11:02:02 +0000 Subject: [PATCH 16/16] Revert "uses shard to get the perf report" This reverts commit f8f7b4590184fcff27bf538a9acc8122f42c64d3. --- .github/workflows/perf-tests.yml | 78 +++++++++++++------------- tests/performance/playwright.config.ts | 2 +- 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/.github/workflows/perf-tests.yml b/.github/workflows/perf-tests.yml index 446efc5..b9d0dc9 100644 --- a/.github/workflows/perf-tests.yml +++ b/.github/workflows/perf-tests.yml @@ -9,10 +9,6 @@ jobs: performance-tests: name: 'Performance Tests' runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - shard: [ 1/4, 2/4, 3/4, 4/4 ] env: WP_BASE_URL: 'http://localhost:8888' WP_USERNAME: 'admin' @@ -55,45 +51,49 @@ jobs: - name: Run tests run: | - yarn test:performance --shard ${{ matrix.shard }} + yarn test:performance + mv ${{ env.WP_ARTIFACTS_PATH }}/performance-results.json ${{ runner.temp }}/results_after.json - - name: Upload blob report to GitHub Actions Artifacts - if: always() - uses: actions/upload-artifact@v4 - with: - name: all-blob-reports - path: blob-report - retention-days: 1 - - merge-reports: - # Merge reports after playwright-tests, even if some shards have failed - if: always() - needs: [ performance-tests ] - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - - name: Setup Node - uses: actions/setup-node@v4 - with: - node-version-file: '.nvmrc' - cache: yarn + - name: Check out base commit + run: | + if [[ -z "$BASE_REF" ]]; then + git fetch -n origin $BASE_SHA + git reset --hard $BASE_SHA + else + git fetch -n origin $BASE_REF + git reset --hard $BASE_SHA + fi + env: + BASE_REF: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.ref || '' }} + BASE_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }} + + # Run tests without causing job to fail if they don't pass (e.g. because of env issues). + - name: Run tests for base + run: | + npm run test:performance || true + if [ -f "{{ env.WP_ARTIFACTS_PATH }}/performance-results.json" ]; then + mv ${{ env.WP_ARTIFACTS_PATH }}/performance-results.json ${{ runner.temp }}/results_before.json + fi; - - name: Install dependencies - run: yarn install --immutable + - name: Reset to original commit + run: | + git reset --hard $GITHUB_SHA - - name: Download blob reports from GitHub Actions Artifacts - uses: actions/download-artifact@v4 - with: - name: all-blob-reports - path: all-blob-reports + - name: Compare results with base + run: | + if [ -f "${{ runner.temp }}/results_before.json" ]; then + yarn test:performance:results ${{ runner.temp }}/results_after.json ${{ runner.temp }}/results_before.json + else + yarn test:performance:results ${{ runner.temp }}/results_after.json + fi; - - name: Merge into single performance report - run: yarn test:performance:merge-reports + - name: Add workflow summary + run: | + cat ${{ env.WP_ARTIFACTS_PATH }}/performance-results.md >> $GITHUB_STEP_SUMMARY - - name: Upload performance report + - name: Upload performance results + if: success() uses: actions/upload-artifact@v4 with: - name: performance-report-${{ github.run_attempt }} - path: artifacts/performance-results.json - retention-days: 14 + name: performance-results + path: ${{ env.WP_ARTIFACTS_PATH }}/performance-results.json diff --git a/tests/performance/playwright.config.ts b/tests/performance/playwright.config.ts index fb8d13e..ae56a4a 100644 --- a/tests/performance/playwright.config.ts +++ b/tests/performance/playwright.config.ts @@ -13,7 +13,7 @@ process.env.STORAGE_STATE_PATH ??= join( process.env.WP_ARTIFACTS_PATH, authStoragePath ); -process.env.TEST_ITERATIONS ??= '20'; +process.env.TEST_ITERATIONS ??= '4'; const config = defineConfig({ globalSetup: require.resolve('../e2e/global-setup.ts'),