From 52119dfcc9cdd98ec21fd6189b2d51f4d4957f62 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Tue, 6 Aug 2024 11:23:04 +0200 Subject: [PATCH] Fix CI Metrics job (#50214) --- .github/workflows/ci.yml | 9 ----- .github/workflows/scripts/run-metrics.sh | 21 ++++++++---- .../woocommerce/changelog/fix-ci-metrics-job | 4 +++ plugins/woocommerce/package.json | 8 +++-- .../tests/e2e-pw/utils/simple-products.js | 34 ++++++++++++------- .../tests/metrics/playwright.config.js | 9 ++--- .../tests/metrics/specs/editor.spec.js | 1 - .../metrics/specs/product-editor.spec.js | 8 ++--- tools/compare-perf/config.js | 18 +++++----- 9 files changed, 65 insertions(+), 47 deletions(-) create mode 100644 plugins/woocommerce/changelog/fix-ci-metrics-job diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f9d43842306..c4ca04202cc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -194,15 +194,6 @@ jobs: name: flaky-tests-${{ strategy.job-index }} path: ${{ env.ARTIFACTS_PATH }}/flaky-tests if-no-files-found: ignore - - - name: 'Archive metrics results' - if: ${{ success() && startsWith(matrix.name, 'Metrics') }} # this seems too fragile, we should update the reporting path and use the generic upload step above - uses: actions/upload-artifact@v4 - env: - WP_ARTIFACTS_PATH: ${{ github.workspace }}/artifacts - with: - name: metrics-results - path: ${{ env.WP_ARTIFACTS_PATH }}/*.performance-results*.json evaluate-project-jobs: # In order to add a required status check we need a consistent job that we can grab onto. diff --git a/.github/workflows/scripts/run-metrics.sh b/.github/workflows/scripts/run-metrics.sh index 4bbe85ee470..7086646a4ae 100755 --- a/.github/workflows/scripts/run-metrics.sh +++ b/.github/workflows/scripts/run-metrics.sh @@ -7,25 +7,34 @@ if [[ -z "$GITHUB_EVENT_NAME" ]]; then exit 1 fi +echo "Installing dependencies" +pnpm install --filter="compare-perf" + if [[ "$GITHUB_EVENT_NAME" == "pull_request" ]]; then echo "Comparing performance with trunk" pnpm --filter="compare-perf" run compare perf $GITHUB_SHA trunk --tests-branch $GITHUB_SHA elif [[ "$GITHUB_EVENT_NAME" == "push" ]]; then echo "Comparing performance with base branch" - # The base hash used here need to be a commit that is compatible with the current WP version - # The current one is 19f3d0884617d7ecdcf37664f648a51e2987cada - # it needs to be updated every time it becomes unsupported by the current wp-env (WP version). - # It is used as a base comparison point to avoid fluctuation in the performance metrics. WP_VERSION=$(awk -F ': ' '/^Tested up to/{print $2}' readme.txt) + # Updating the WP version used for performance jobs means there’s a high + # chance that the reference commit used for performance test stability + # becomes incompatible with the WP version. So, every time the "Tested up + # to" flag is updated in the readme.txt, we also have to update the + # reference commit below (BASE_SHA). The new reference needs to meet the + # following requirements: + # - Be compatible with the new WP version used in the “Tested up to” flag. + # - Be tracked on https://www.codevitals.run/project/woo for all existing + # metrics. + BASE_SHA=3d7d7f02017383937f1a4158d433d0e5d44b3dc9 echo "WP_VERSION: $WP_VERSION" IFS=. read -ra WP_VERSION_ARRAY <<< "$WP_VERSION" WP_MAJOR="${WP_VERSION_ARRAY[0]}.${WP_VERSION_ARRAY[1]}" - pnpm --filter="compare-perf" run compare perf $GITHUB_SHA 19f3d0884617d7ecdcf37664f648a51e2987cada --tests-branch $GITHUB_SHA --wp-version "$WP_MAJOR" + pnpm --filter="compare-perf" run compare perf $GITHUB_SHA $BASE_SHA --tests-branch $GITHUB_SHA --wp-version "$WP_MAJOR" echo "Publish results to CodeVitals" COMMITTED_AT=$(git show -s $GITHUB_SHA --format="%cI") - pnpm --filter="compare-perf" run log $CODEVITALS_PROJECT_TOKEN trunk $GITHUB_SHA 19f3d0884617d7ecdcf37664f648a51e2987cada $COMMITTED_AT + pnpm --filter="compare-perf" run log $CODEVITALS_PROJECT_TOKEN trunk $GITHUB_SHA $BASE_SHA $COMMITTED_AT else echo "Unsupported event: $GITHUB_EVENT_NAME" fi diff --git a/plugins/woocommerce/changelog/fix-ci-metrics-job b/plugins/woocommerce/changelog/fix-ci-metrics-job new file mode 100644 index 00000000000..d743f02743a --- /dev/null +++ b/plugins/woocommerce/changelog/fix-ci-metrics-job @@ -0,0 +1,4 @@ +Significance: patch +Type: dev + +Fix Metrics CI job diff --git a/plugins/woocommerce/package.json b/plugins/woocommerce/package.json index 0506a8a3dbe..6c41b1cead1 100644 --- a/plugins/woocommerce/package.json +++ b/plugins/woocommerce/package.json @@ -529,8 +529,12 @@ ".wp-env.json" ], "events": [ - "disabled" - ] + "push" + ], + "report": { + "resultsBlobName": "core-metrics-report", + "resultsPath": "../../tools/compare-perf/artifacts/" + } }, { "name": "Blocks e2e tests", diff --git a/plugins/woocommerce/tests/e2e-pw/utils/simple-products.js b/plugins/woocommerce/tests/e2e-pw/utils/simple-products.js index 1e9af15844b..a37c31ce486 100644 --- a/plugins/woocommerce/tests/e2e-pw/utils/simple-products.js +++ b/plugins/woocommerce/tests/e2e-pw/utils/simple-products.js @@ -22,22 +22,32 @@ async function isBlockProductEditorEnabled( page ) { /** * This function is typically used for enabling/disabling the block product editor in settings page. * - * @param {string} action The action that will be performed. - * @param {import('@playwright/test').Page} page + * @param {string} action The action that will be performed. + * @param {import('@playwright/test').Page} page */ async function toggleBlockProductEditor( action = 'enable', page ) { await page.goto( SETTINGS_URL ); - if ( action === 'disable' ) { - await page - .locator( '#woocommerce_feature_product_block_editor_enabled' ) - .uncheck(); - } else { - await page - .locator( '#woocommerce_feature_product_block_editor_enabled' ) - .check(); + + const enableProductEditor = page.locator( + '#woocommerce_feature_product_block_editor_enabled' + ); + const isEnabled = await enableProductEditor.isChecked(); + + if ( + ( action === 'enable' && isEnabled ) || + ( action === 'disable' && ! isEnabled ) + ) { + // No need to toggle the setting. + return; } + + if ( action === 'enable' ) { + await enableProductEditor.check(); + } else if ( action === 'disable' ) { + await enableProductEditor.uncheck(); + } + await page - .locator( '.submit' ) .getByRole( 'button', { name: 'Save changes', } ) @@ -81,7 +91,7 @@ async function expectBlockProductEditor( page ) { /** * Click on a block editor tab. * - * @param {string} tabName + * @param {string} tabName * @param {import('@playwright/test').Page} page */ async function clickOnTab( tabName, page ) { diff --git a/plugins/woocommerce/tests/metrics/playwright.config.js b/plugins/woocommerce/tests/metrics/playwright.config.js index 20043d2088c..053ea9231d2 100644 --- a/plugins/woocommerce/tests/metrics/playwright.config.js +++ b/plugins/woocommerce/tests/metrics/playwright.config.js @@ -1,3 +1,6 @@ +/** + * External dependencies + */ import path from 'path'; import { fileURLToPath } from 'url'; import { defineConfig, devices } from '@playwright/test'; @@ -11,9 +14,7 @@ process.env.STORAGE_STATE_PATH ??= path.join( process.env.WP_BASE_URL ??= 'http://localhost:8086'; const config = defineConfig( { - reporter: process.env.CI - ? './config/performance-reporter.ts' - : [ [ 'list' ], [ './config/performance-reporter.ts' ] ], + reporter: [ [ 'list' ], [ './config/performance-reporter.ts' ] ], forbidOnly: !! process.env.CI, fullyParallel: false, workers: 1, @@ -24,7 +25,7 @@ const config = defineConfig( { testDir: './specs', outputDir: path.join( process.env.WP_ARTIFACTS_PATH, 'test-results' ), snapshotPathTemplate: - '{testDir}/{testFileDir}/__snapshots__/{arg}-{projectName}{ext}', + '{testDir}/{testFileDir}/__snapshots__/{arg}-{projectName}{ext}', globalSetup: fileURLToPath( new URL( './config/global-setup.ts', 'file:' + __filename ).href ), diff --git a/plugins/woocommerce/tests/metrics/specs/editor.spec.js b/plugins/woocommerce/tests/metrics/specs/editor.spec.js index 62d0432e78a..30307a4d022 100644 --- a/plugins/woocommerce/tests/metrics/specs/editor.spec.js +++ b/plugins/woocommerce/tests/metrics/specs/editor.spec.js @@ -146,7 +146,6 @@ test.describe( 'Editor Performance', () => { await perfUtils.loadBlocksForLargePost(); await editor.insertBlock( { name: 'core/paragraph' } ); draftId = await perfUtils.saveDraft(); - console.log( draftId ); } ); test( 'Run the test', async ( { admin, page, perfUtils, metrics } ) => { diff --git a/plugins/woocommerce/tests/metrics/specs/product-editor.spec.js b/plugins/woocommerce/tests/metrics/specs/product-editor.spec.js index ce5eebb090f..b23eab5dda9 100644 --- a/plugins/woocommerce/tests/metrics/specs/product-editor.spec.js +++ b/plugins/woocommerce/tests/metrics/specs/product-editor.spec.js @@ -30,10 +30,6 @@ test.describe( 'Product editor performance', () => { }, } ); - test.beforeEach( async ( { page } ) => { - await toggleBlockProductEditor( 'enable', page ); - } ); - test.afterAll( async ( {}, testInfo ) => { const medians = {}; Object.keys( results ).forEach( ( metric ) => { @@ -45,6 +41,10 @@ test.describe( 'Product editor performance', () => { } ); } ); + test( 'Enable Product Editor', async ( { page } ) => { + await toggleBlockProductEditor( 'enable', page ); + } ); + test.describe( 'Loading', () => { const samples = 2; const throwaway = 1; diff --git a/tools/compare-perf/config.js b/tools/compare-perf/config.js index e8f1e5b2e0d..a84d788b392 100644 --- a/tools/compare-perf/config.js +++ b/tools/compare-perf/config.js @@ -2,13 +2,13 @@ const path = require( 'path' ); const getPnpmPackage = ( sourceDir ) => { const packageJson = require( path.join( sourceDir, 'package.json' ) ); - let pnpm_package = 'pnpm'; + let pnpmPackage = 'pnpm'; if ( packageJson.engines.pnpm ) { - pnpm_package = `pnpm@${ packageJson.engines.pnpm }`; + pnpmPackage = `pnpm@${ packageJson.engines.pnpm }`; } - return pnpm_package; + return pnpmPackage; }; const config = { @@ -16,18 +16,18 @@ const config = { pluginPath: '/plugins/woocommerce', testsPath: '/plugins/woocommerce/tests/metrics/specs', getSetupTestRunner: ( sourceDir ) => { - const pnpm_package = getPnpmPackage( sourceDir ); + const pnpmPackage = getPnpmPackage( sourceDir ); - return `npm install -g ${ pnpm_package } && pnpm install --filter="@woocommerce/plugin-woocommerce" &> /dev/null && cd plugins/woocommerce && pnpm exec playwright install chromium`; + return `npm install -g ${ pnpmPackage } && pnpm install --frozen-lockfile --filter="@woocommerce/plugin-woocommerce" &> /dev/null && cd plugins/woocommerce && pnpm exec playwright install chromium`; }, getSetupCommand: ( sourceDir ) => { - const pnpm_package = getPnpmPackage( sourceDir ); + const pnpmPackage = getPnpmPackage( sourceDir ); - return `npm install -g ${ pnpm_package } && pnpm install &> /dev/null && pnpm build &> /dev/null`; + return `npm install -g ${ pnpmPackage } && pnpm install --frozen-lockfile &> /dev/null && pnpm build &> /dev/null`; }, getTestCommand: ( sourceDir ) => { - const pnpm_package = getPnpmPackage( sourceDir ); - return `npm install -g ${ pnpm_package } && cd plugins/woocommerce && pnpm test:metrics`; + const pnpmPackage = getPnpmPackage( sourceDir ); + return `npm install -g ${ pnpmPackage } && cd plugins/woocommerce && pnpm test:metrics`; }, };