From 184bf0d5f091b1ffb650afa3b5a6d3ea34e25daf Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Thu, 16 Feb 2023 18:42:06 +0100 Subject: [PATCH 01/21] Fix site editor performance tests --- .../specs/performance/site-editor.test.js | 154 +++++++++--------- 1 file changed, 77 insertions(+), 77 deletions(-) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index 897126111564c..e2b8fc1353bf3 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -59,6 +59,8 @@ describe( 'Site Editor Performance', () => { ); await createNewPost( { postType: 'page' } ); + await page.keyboard.type( 'Site Editor Performance Tests' ); + await page.evaluate( ( _html ) => { const { parse } = window.wp.blocks; const { dispatch } = window.wp.data; @@ -81,6 +83,12 @@ describe( 'Site Editor Performance', () => { } ); afterAll( async () => { + const resultsFilename = basename( __filename, '.js' ) + '.results.json'; + writeFileSync( + join( __dirname, resultsFilename ), + JSON.stringify( results, null, 2 ) + ); + await deleteAllTemplates( 'wp_template' ); await deleteAllTemplates( 'wp_template_part' ); await activateTheme( 'twentytwentyone' ); @@ -91,96 +99,88 @@ describe( 'Site Editor Performance', () => { postId: id, postType: 'page', } ); - } ); - it( 'Loading', async () => { - const editorURL = await page.url(); + // Wait for the canvas to become actionable. + await canvas().waitForSelector( + '[data-rich-text-placeholder="Write site tagline…"]' + ); + } ); - // Number of sample measurements to take. + describe( 'Loading', () => { const samples = 3; - // Number of throwaway measurements to perform before recording samples. - // Having at least one helps ensure that caching quirks don't manifest in - // the results. const throwaway = 1; + const iterations = Array.from( + { length: samples + throwaway }, + ( _, i ) => i + 1 + ); - let i = throwaway + samples; - - // Measuring loading time. - while ( i-- ) { - await page.close(); - page = await browser.newPage(); + it.each( iterations )( + `iteration %i of ${ iterations.length }`, + async ( i ) => { + if ( i > 1 ) { + const { + serverResponse, + firstPaint, + domContentLoaded, + loaded, + firstContentfulPaint, + firstBlock, + } = await getLoadingDurations(); + + results.serverResponse.push( serverResponse ); + results.firstPaint.push( firstPaint ); + results.domContentLoaded.push( domContentLoaded ); + results.loaded.push( loaded ); + results.firstContentfulPaint.push( firstContentfulPaint ); + results.firstBlock.push( firstBlock ); + } - await page.goto( editorURL ); - await page.waitForSelector( '.edit-site-visual-editor', { - timeout: 120000, - } ); - await canvas().waitForSelector( '.wp-block', { timeout: 120000 } ); - - if ( i < samples ) { - const { - serverResponse, - firstPaint, - domContentLoaded, - loaded, - firstContentfulPaint, - firstBlock, - } = await getLoadingDurations(); - - results.serverResponse.push( serverResponse ); - results.firstPaint.push( firstPaint ); - results.domContentLoaded.push( domContentLoaded ); - results.loaded.push( loaded ); - results.firstContentfulPaint.push( firstContentfulPaint ); - results.firstBlock.push( firstBlock ); + expect( true ).toBe( true ); } - } + ); } ); - it( 'Typing', async () => { - await page.waitForSelector( '.edit-site-visual-editor', { - timeout: 120000, - } ); - await canvas().waitForSelector( '.wp-block', { timeout: 120000 } ); + describe( 'Typing', () => { + it( 'trace 200 characters typing sequence', async () => { + // Measure typing performance inside the post content. + await enterEditMode(); - // Measuring typing performance inside the post content. - await canvas().waitForSelector( - '[data-type="core/post-content"] [data-type="core/paragraph"]' - ); - await enterEditMode(); - await canvas().focus( - '[data-type="core/post-content"] [data-type="core/paragraph"]' - ); - await insertBlock( 'Paragraph' ); - let i = 200; - const traceFile = __dirname + '/trace.json'; - await page.tracing.start( { - path: traceFile, - screenshots: false, - categories: [ 'devtools.timeline' ], - } ); - while ( i-- ) { - await page.keyboard.type( 'x' ); - } - await page.tracing.stop(); - const traceResults = JSON.parse( readFile( traceFile ) ); - const [ keyDownEvents, keyPressEvents, keyUpEvents ] = - getTypingEventDurations( traceResults ); - - for ( let j = 0; j < keyDownEvents.length; j++ ) { - results.type.push( - keyDownEvents[ j ] + keyPressEvents[ j ] + keyUpEvents[ j ] + // Insert a new paragraph right after the first one. + const targetParagraph = await canvas().waitForXPath( + '//p[contains(text(), "Lorem ipsum dolor sit amet")]' ); - } - - const resultsFilename = basename( __filename, '.js' ) + '.results.json'; + // The second click is needed to get the cursor inside the target + // paragraph. + await targetParagraph.click(); + await targetParagraph.click(); + await insertBlock( 'Paragraph' ); + + // Start tracing. + const traceFile = __dirname + '/trace.json'; + await page.tracing.start( { + path: traceFile, + screenshots: false, + categories: [ 'devtools.timeline' ], + } ); - writeFileSync( - join( __dirname, resultsFilename ), - JSON.stringify( results, null, 2 ) - ); + // Type "x" 200 times. + await page.keyboard.type( new Array( 200 ).fill( 'x' ).join( '' ) ); + + // Stop tracing and save results. + await page.tracing.stop(); + const traceResults = JSON.parse( readFile( traceFile ) ); + const [ keyDownEvents, keyPressEvents, keyUpEvents ] = + getTypingEventDurations( traceResults ); + for ( let i = 0; i < keyDownEvents.length; i++ ) { + results.type.push( + keyDownEvents[ i ] + keyPressEvents[ i ] + keyUpEvents[ i ] + ); + } - deleteFile( traceFile ); + // Delete the original trace file. + deleteFile( traceFile ); - expect( true ).toBe( true ); + expect( true ).toBe( true ); + } ); } ); } ); From 01bc9e299152751ce40f73ad9828bc28ca170996 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Thu, 16 Feb 2023 18:45:14 +0100 Subject: [PATCH 02/21] Restore some comments --- packages/e2e-tests/specs/performance/site-editor.test.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index e2b8fc1353bf3..90b5eb5ed9c3b 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -107,7 +107,11 @@ describe( 'Site Editor Performance', () => { } ); describe( 'Loading', () => { + // Number of measurements to take. const samples = 3; + // Number of throwaway measurements to perform before recording samples. + // Having at least one helps ensure that caching quirks don't manifest + // in the results. const throwaway = 1; const iterations = Array.from( { length: samples + throwaway }, From 3dbc1feecc5798d6e87ae4de37bce6d4d286310a Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Fri, 17 Feb 2023 11:15:19 +0100 Subject: [PATCH 03/21] Try uploading artifacts to see why CI is failing --- .github/workflows/performance.yml | 8 +++ bin/plugin/commands/performance.js | 52 ++++++++++++------- .../specs/performance/site-editor.test.js | 2 +- 3 files changed, 43 insertions(+), 19 deletions(-) diff --git a/.github/workflows/performance.yml b/.github/workflows/performance.yml index 33dc19aabce83..1da1eeba819a7 100644 --- a/.github/workflows/performance.yml +++ b/.github/workflows/performance.yml @@ -86,6 +86,14 @@ jobs: WP_MAJOR="${WP_VERSION_ARRAY[0]}.${WP_VERSION_ARRAY[1]}" ./bin/plugin/cli.js perf $GITHUB_SHA debd225d007f4e441ceec80fbd6fa96653f94737 --tests-branch $GITHUB_SHA --wp-version "$WP_MAJOR" + - name: Archive debug artifacts (screenshots, traces) + uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2 + if: always() + with: + name: perf-failures-artifacts + path: ./__test-results/artifacts + if-no-files-found: ignore + - uses: actions/github-script@98814c53be79b1d30f795b907e553d8679345975 # v6.4.0 if: github.event_name == 'push' id: commit-timestamp diff --git a/bin/plugin/commands/performance.js b/bin/plugin/commands/performance.js index 869f36de4c87b..6f2c2d82ef671 100644 --- a/bin/plugin/commands/performance.js +++ b/bin/plugin/commands/performance.js @@ -182,23 +182,39 @@ function curateResults( testSuite, results ) { * @return {Promise} Performance results for the branch. */ async function runTestSuite( testSuite, performanceTestDirectory, runKey ) { - await runShellScript( - `npm run test:performance -- packages/e2e-tests/specs/performance/${ testSuite }.test.js`, - performanceTestDirectory - ); - const resultsFile = path.join( - performanceTestDirectory, - `packages/e2e-tests/specs/performance/${ testSuite }.test.results.json` - ); - fs.mkdirSync( './__test-results', { recursive: true } ); - fs.copyFileSync( resultsFile, `./__test-results/${ runKey }.results.json` ); - const rawResults = await readJSONFile( - path.join( + try { + await runShellScript( + `npm run test:performance -- packages/e2e-tests/specs/performance/${ testSuite }.test.js`, + performanceTestDirectory + ); + const resultsFile = path.join( performanceTestDirectory, `packages/e2e-tests/specs/performance/${ testSuite }.test.results.json` - ) - ); - return curateResults( testSuite, rawResults ); + ); + fs.mkdirSync( './__test-results', { recursive: true } ); + fs.copyFileSync( + resultsFile, + `./__test-results/${ runKey }.results.json` + ); + const rawResults = await readJSONFile( + path.join( + performanceTestDirectory, + `packages/e2e-tests/specs/performance/${ testSuite }.test.results.json` + ) + ); + return curateResults( testSuite, rawResults ); + } catch ( error ) { + fs.mkdirSync( './__test-results/artifacts', { recursive: true } ); + const artifactsFolder = path.join( + performanceTestDirectory, + 'artifacts' + ); + await runShellScript( + 'cp -Rv ' + artifactsFolder + ' ' + './__test-results/artifacts' + ); + + throw error; + } } /** @@ -370,10 +386,10 @@ async function runPerformanceTests( branches, options ) { log( '\n>> Running the tests' ); const testSuites = [ - 'post-editor', + // 'post-editor', 'site-editor', - 'front-end-classic-theme', - 'front-end-block-theme', + // 'front-end-classic-theme', + // 'front-end-block-theme', ]; /** @type {Record>} */ diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index 90b5eb5ed9c3b..46bd9cd43aa65 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -106,7 +106,7 @@ describe( 'Site Editor Performance', () => { ); } ); - describe( 'Loading', () => { + describe.skip( 'Loading', () => { // Number of measurements to take. const samples = 3; // Number of throwaway measurements to perform before recording samples. From 1a1298cb22f5150395430f915a314e1501e348c4 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Fri, 17 Feb 2023 13:14:51 +0100 Subject: [PATCH 04/21] try setting failure breakpoints to get screenshots --- packages/e2e-tests/specs/performance/site-editor.test.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index 46bd9cd43aa65..b158c69e3144a 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -106,7 +106,7 @@ describe( 'Site Editor Performance', () => { ); } ); - describe.skip( 'Loading', () => { + describe( 'Loading', () => { // Number of measurements to take. const samples = 3; // Number of throwaway measurements to perform before recording samples. @@ -121,6 +121,8 @@ describe( 'Site Editor Performance', () => { it.each( iterations )( `iteration %i of ${ iterations.length }`, async ( i ) => { + expect( true ).toBe( false ); + if ( i > 1 ) { const { serverResponse, @@ -146,6 +148,8 @@ describe( 'Site Editor Performance', () => { describe( 'Typing', () => { it( 'trace 200 characters typing sequence', async () => { + expect( true ).toBe( false ); + // Measure typing performance inside the post content. await enterEditMode(); From 835f3c61a5efd5eb7f33e76bb07e83838da92ac9 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Fri, 17 Feb 2023 13:57:52 +0100 Subject: [PATCH 05/21] Try checking if the test post is being saved correctly --- .../specs/performance/site-editor.test.js | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index b158c69e3144a..1832248fe4a71 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -59,7 +59,7 @@ describe( 'Site Editor Performance', () => { ); await createNewPost( { postType: 'page' } ); - await page.keyboard.type( 'Site Editor Performance Tests' ); + // await page.keyboard.type( 'Site Editor Performance Tests' ); await page.evaluate( ( _html ) => { const { parse } = window.wp.blocks; @@ -94,18 +94,6 @@ describe( 'Site Editor Performance', () => { await activateTheme( 'twentytwentyone' ); } ); - beforeEach( async () => { - await visitSiteEditor( { - postId: id, - postType: 'page', - } ); - - // Wait for the canvas to become actionable. - await canvas().waitForSelector( - '[data-rich-text-placeholder="Write site tagline…"]' - ); - } ); - describe( 'Loading', () => { // Number of measurements to take. const samples = 3; @@ -123,6 +111,17 @@ describe( 'Site Editor Performance', () => { async ( i ) => { expect( true ).toBe( false ); + // Open the test page in Site Editor. + await visitSiteEditor( { + postId: id, + postType: 'page', + } ); + + // Wait for the canvas to become actionable. + await canvas().waitForSelector( + '[data-rich-text-placeholder="Write site tagline…"]' + ); + if ( i > 1 ) { const { serverResponse, @@ -150,6 +149,17 @@ describe( 'Site Editor Performance', () => { it( 'trace 200 characters typing sequence', async () => { expect( true ).toBe( false ); + // Open the test page in Site Editor. + await visitSiteEditor( { + postId: id, + postType: 'page', + } ); + + // Wait for the canvas to become actionable. + await canvas().waitForSelector( + '[data-rich-text-placeholder="Write site tagline…"]' + ); + // Measure typing performance inside the post content. await enterEditMode(); From f4a1a9fcbe801f3a756a22335e8171fb90a8ccc4 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Fri, 17 Feb 2023 14:29:36 +0100 Subject: [PATCH 06/21] Clear local storage in pageSetup Clearing local storage on a page that we're about to close can throw error because the current context could be an iframe, not the top level document. Attempting to clear the LS in an iframe would end up throwing the `Failed to read the 'localStorage' property from 'Window': Access is denied for this document` error. --- packages/e2e-tests/config/setup-performance-test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/e2e-tests/config/setup-performance-test.js b/packages/e2e-tests/config/setup-performance-test.js index 384a88b8a3f2d..ea89d725a2e27 100644 --- a/packages/e2e-tests/config/setup-performance-test.js +++ b/packages/e2e-tests/config/setup-performance-test.js @@ -20,6 +20,7 @@ const PUPPETEER_TIMEOUT = process.env.PUPPETEER_TIMEOUT; jest.setTimeout( PUPPETEER_TIMEOUT || 100000 ); async function setupPage() { + await clearLocalStorage(); await setBrowserViewport( 'large' ); await page.emulateMediaFeatures( [ { name: 'prefers-reduced-motion', value: 'reduce' }, @@ -34,14 +35,11 @@ beforeAll( async () => { await trashAllPosts(); await trashAllPosts( 'wp_block' ); - await clearLocalStorage(); await setupPage(); await activatePlugin( 'gutenberg-test-plugin-disables-the-css-animations' ); } ); afterEach( async () => { - // Clear localStorage between tests so that the next test starts clean. - await clearLocalStorage(); // Close the previous page entirely and create a new page, so that the next test // isn't affected by page unload work. await page.close(); From f2f52a1e297033b4f5a18e075190fd9cecf8a4e9 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Fri, 17 Feb 2023 14:33:10 +0100 Subject: [PATCH 07/21] Try wait longer for the SE --- packages/e2e-tests/specs/performance/site-editor.test.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index 1832248fe4a71..ada79a7db1ae1 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -94,7 +94,7 @@ describe( 'Site Editor Performance', () => { await activateTheme( 'twentytwentyone' ); } ); - describe( 'Loading', () => { + describe.skip( 'Loading', () => { // Number of measurements to take. const samples = 3; // Number of throwaway measurements to perform before recording samples. @@ -147,14 +147,13 @@ describe( 'Site Editor Performance', () => { describe( 'Typing', () => { it( 'trace 200 characters typing sequence', async () => { - expect( true ).toBe( false ); - // Open the test page in Site Editor. await visitSiteEditor( { postId: id, postType: 'page', } ); - + // eslint-disable-next-line no-restricted-syntax + await page.waitForTimeout( 29000 ); // Wait for the canvas to become actionable. await canvas().waitForSelector( '[data-rich-text-placeholder="Write site tagline…"]' From 0ab0c95c08cd06f34413fac056439ab632fd7463 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Sat, 18 Feb 2023 14:46:22 +0100 Subject: [PATCH 08/21] Clean up a few bits --- packages/e2e-tests/specs/performance/site-editor.test.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index ada79a7db1ae1..68a2d1c9c9782 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -94,7 +94,7 @@ describe( 'Site Editor Performance', () => { await activateTheme( 'twentytwentyone' ); } ); - describe.skip( 'Loading', () => { + describe( 'Loading', () => { // Number of measurements to take. const samples = 3; // Number of throwaway measurements to perform before recording samples. @@ -122,7 +122,7 @@ describe( 'Site Editor Performance', () => { '[data-rich-text-placeholder="Write site tagline…"]' ); - if ( i > 1 ) { + if ( i > throwaway ) { const { serverResponse, firstPaint, @@ -152,8 +152,7 @@ describe( 'Site Editor Performance', () => { postId: id, postType: 'page', } ); - // eslint-disable-next-line no-restricted-syntax - await page.waitForTimeout( 29000 ); + // Wait for the canvas to become actionable. await canvas().waitForSelector( '[data-rich-text-placeholder="Write site tagline…"]' From dee6e7152f704665b2d84e7b65a783cd8cf0bfe4 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Mon, 20 Feb 2023 20:19:17 +0100 Subject: [PATCH 09/21] Add missing path param --- .../e2e-tests/specs/performance/site-editor.test.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index 68a2d1c9c9782..32ce4eda2fb7a 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -46,7 +46,7 @@ const results = { listViewOpen: [], }; -let id; +let postId; describe( 'Site Editor Performance', () => { beforeAll( async () => { @@ -77,7 +77,7 @@ describe( 'Site Editor Performance', () => { }, html ); await saveDraft(); - id = await page.evaluate( () => + postId = await page.evaluate( () => new URL( document.location ).searchParams.get( 'post' ) ); } ); @@ -113,8 +113,9 @@ describe( 'Site Editor Performance', () => { // Open the test page in Site Editor. await visitSiteEditor( { - postId: id, + postId, postType: 'page', + path: '/navigation/single', } ); // Wait for the canvas to become actionable. @@ -149,8 +150,9 @@ describe( 'Site Editor Performance', () => { it( 'trace 200 characters typing sequence', async () => { // Open the test page in Site Editor. await visitSiteEditor( { - postId: id, + postId, postType: 'page', + path: '/navigation/single', } ); // Wait for the canvas to become actionable. From fbe4439521be8c39d4c776689c624f3b2a432fad Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Mon, 20 Feb 2023 20:35:53 +0100 Subject: [PATCH 10/21] Remove temp code --- packages/e2e-tests/specs/performance/site-editor.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index 32ce4eda2fb7a..876f8bfd7fbd2 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -109,8 +109,6 @@ describe( 'Site Editor Performance', () => { it.each( iterations )( `iteration %i of ${ iterations.length }`, async ( i ) => { - expect( true ).toBe( false ); - // Open the test page in Site Editor. await visitSiteEditor( { postId, From f84146cff501848e44128d5475680e497a6aa390 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Tue, 21 Feb 2023 12:07:44 +0100 Subject: [PATCH 11/21] Fix LS clearing An `about:blank` page cannot have its LS cleared. It will throw `Evaluation failed: DOMException: Failed to read the 'localStorage' property from 'Window': Access is denied for this document.` --- .../e2e-tests/config/setup-performance-test.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/e2e-tests/config/setup-performance-test.js b/packages/e2e-tests/config/setup-performance-test.js index ea89d725a2e27..655a945143c2b 100644 --- a/packages/e2e-tests/config/setup-performance-test.js +++ b/packages/e2e-tests/config/setup-performance-test.js @@ -20,28 +20,30 @@ const PUPPETEER_TIMEOUT = process.env.PUPPETEER_TIMEOUT; jest.setTimeout( PUPPETEER_TIMEOUT || 100000 ); async function setupPage() { - await clearLocalStorage(); await setBrowserViewport( 'large' ); await page.emulateMediaFeatures( [ { name: 'prefers-reduced-motion', value: 'reduce' }, ] ); } -// Before every test suite run, delete all content created by the test. This ensures -// other posts/comments/etc. aren't dirtying tests and tests don't depend on -// each other's side-effects. +// Before every test suite run, delete all content created by the test. This +// ensures other posts/comments/etc. aren't dirtying tests and tests don't +// depend on each other's side-effects. beforeAll( async () => { enablePageDialogAccept(); await trashAllPosts(); await trashAllPosts( 'wp_block' ); - await setupPage(); await activatePlugin( 'gutenberg-test-plugin-disables-the-css-animations' ); + await clearLocalStorage(); + await setupPage(); } ); afterEach( async () => { - // Close the previous page entirely and create a new page, so that the next test - // isn't affected by page unload work. + // Clear localStorage between tests so that the next test starts clean. + await clearLocalStorage(); + // Close the previous page entirely and create a new page, so that the next + // test isn't affected by page unload work. await page.close(); page = await browser.newPage(); // Set up testing config on new page. From edd93b6295058e8640a01d5c3a026c766640aaaf Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Tue, 21 Feb 2023 12:34:42 +0100 Subject: [PATCH 12/21] Add more tweaks --- .../specs/performance/site-editor.test.js | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index 876f8bfd7fbd2..408660d0e7137 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -107,7 +107,7 @@ describe( 'Site Editor Performance', () => { ); it.each( iterations )( - `iteration %i of ${ iterations.length }`, + `trace large post loading durations (%i of ${ iterations.length })`, async ( i ) => { // Open the test page in Site Editor. await visitSiteEditor( { @@ -145,7 +145,7 @@ describe( 'Site Editor Performance', () => { } ); describe( 'Typing', () => { - it( 'trace 200 characters typing sequence', async () => { + it( 'trace 200 characters typing sequence inside a large post', async () => { // Open the test page in Site Editor. await visitSiteEditor( { postId, @@ -153,22 +153,19 @@ describe( 'Site Editor Performance', () => { path: '/navigation/single', } ); - // Wait for the canvas to become actionable. + // Wait for the canvas to become editable. await canvas().waitForSelector( '[data-rich-text-placeholder="Write site tagline…"]' ); - // Measure typing performance inside the post content. + // Get inside the post content. await enterEditMode(); - // Insert a new paragraph right after the first one. - const targetParagraph = await canvas().waitForXPath( + // Insert a new paragraph right under the first one. + const firstParagraph = await canvas().waitForXPath( '//p[contains(text(), "Lorem ipsum dolor sit amet")]' ); - // The second click is needed to get the cursor inside the target - // paragraph. - await targetParagraph.click(); - await targetParagraph.click(); + await firstParagraph.focus(); await insertBlock( 'Paragraph' ); // Start tracing. From 4a549fccbcff43cb82cecbed89fadd7546bd7a36 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Tue, 21 Feb 2023 13:39:04 +0100 Subject: [PATCH 13/21] Revert "Try uploading artifacts to see why CI is failing" This reverts commit f86fa0806f2bc6896ad6bb1258f005af0a39abaf. --- .github/workflows/performance.yml | 8 ----- bin/plugin/commands/performance.js | 52 +++++++++++------------------- 2 files changed, 18 insertions(+), 42 deletions(-) diff --git a/.github/workflows/performance.yml b/.github/workflows/performance.yml index 1da1eeba819a7..33dc19aabce83 100644 --- a/.github/workflows/performance.yml +++ b/.github/workflows/performance.yml @@ -86,14 +86,6 @@ jobs: WP_MAJOR="${WP_VERSION_ARRAY[0]}.${WP_VERSION_ARRAY[1]}" ./bin/plugin/cli.js perf $GITHUB_SHA debd225d007f4e441ceec80fbd6fa96653f94737 --tests-branch $GITHUB_SHA --wp-version "$WP_MAJOR" - - name: Archive debug artifacts (screenshots, traces) - uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2 - if: always() - with: - name: perf-failures-artifacts - path: ./__test-results/artifacts - if-no-files-found: ignore - - uses: actions/github-script@98814c53be79b1d30f795b907e553d8679345975 # v6.4.0 if: github.event_name == 'push' id: commit-timestamp diff --git a/bin/plugin/commands/performance.js b/bin/plugin/commands/performance.js index 6f2c2d82ef671..869f36de4c87b 100644 --- a/bin/plugin/commands/performance.js +++ b/bin/plugin/commands/performance.js @@ -182,39 +182,23 @@ function curateResults( testSuite, results ) { * @return {Promise} Performance results for the branch. */ async function runTestSuite( testSuite, performanceTestDirectory, runKey ) { - try { - await runShellScript( - `npm run test:performance -- packages/e2e-tests/specs/performance/${ testSuite }.test.js`, - performanceTestDirectory - ); - const resultsFile = path.join( + await runShellScript( + `npm run test:performance -- packages/e2e-tests/specs/performance/${ testSuite }.test.js`, + performanceTestDirectory + ); + const resultsFile = path.join( + performanceTestDirectory, + `packages/e2e-tests/specs/performance/${ testSuite }.test.results.json` + ); + fs.mkdirSync( './__test-results', { recursive: true } ); + fs.copyFileSync( resultsFile, `./__test-results/${ runKey }.results.json` ); + const rawResults = await readJSONFile( + path.join( performanceTestDirectory, `packages/e2e-tests/specs/performance/${ testSuite }.test.results.json` - ); - fs.mkdirSync( './__test-results', { recursive: true } ); - fs.copyFileSync( - resultsFile, - `./__test-results/${ runKey }.results.json` - ); - const rawResults = await readJSONFile( - path.join( - performanceTestDirectory, - `packages/e2e-tests/specs/performance/${ testSuite }.test.results.json` - ) - ); - return curateResults( testSuite, rawResults ); - } catch ( error ) { - fs.mkdirSync( './__test-results/artifacts', { recursive: true } ); - const artifactsFolder = path.join( - performanceTestDirectory, - 'artifacts' - ); - await runShellScript( - 'cp -Rv ' + artifactsFolder + ' ' + './__test-results/artifacts' - ); - - throw error; - } + ) + ); + return curateResults( testSuite, rawResults ); } /** @@ -386,10 +370,10 @@ async function runPerformanceTests( branches, options ) { log( '\n>> Running the tests' ); const testSuites = [ - // 'post-editor', + 'post-editor', 'site-editor', - // 'front-end-classic-theme', - // 'front-end-block-theme', + 'front-end-classic-theme', + 'front-end-block-theme', ]; /** @type {Record>} */ From 91bdc33e68641455d7796e578915d068ee10c751 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Tue, 21 Feb 2023 13:56:20 +0100 Subject: [PATCH 14/21] Improve wording --- packages/e2e-tests/specs/performance/site-editor.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index 408660d0e7137..d98210515d3ce 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -116,7 +116,7 @@ describe( 'Site Editor Performance', () => { path: '/navigation/single', } ); - // Wait for the canvas to become actionable. + // Wait for the canvas to become editable. await canvas().waitForSelector( '[data-rich-text-placeholder="Write site tagline…"]' ); From 3781e859bda5c40827f4211735d70708585c3f67 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Tue, 21 Feb 2023 16:06:28 +0100 Subject: [PATCH 15/21] Remove obsolete path param As in https://github.com/WordPress/gutenberg/pull/48094 --- packages/e2e-tests/specs/performance/site-editor.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index d98210515d3ce..7706c471a7f53 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -113,7 +113,6 @@ describe( 'Site Editor Performance', () => { await visitSiteEditor( { postId, postType: 'page', - path: '/navigation/single', } ); // Wait for the canvas to become editable. @@ -150,7 +149,6 @@ describe( 'Site Editor Performance', () => { await visitSiteEditor( { postId, postType: 'page', - path: '/navigation/single', } ); // Wait for the canvas to become editable. From 10d7c04ac281a3ba334269b1d3fd0ee43be2ed22 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Tue, 28 Feb 2023 12:08:18 +0100 Subject: [PATCH 16/21] Add `sequence` util --- packages/e2e-tests/specs/performance/site-editor.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index 7706c471a7f53..50e2042cfe4b9 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -30,6 +30,9 @@ import { jest.setTimeout( 1000000 ); +const sequence = ( start, length ) => + Array.from( { length }, ( _, i ) => i + start ); + const results = { serverResponse: [], firstPaint: [], @@ -101,10 +104,7 @@ describe( 'Site Editor Performance', () => { // Having at least one helps ensure that caching quirks don't manifest // in the results. const throwaway = 1; - const iterations = Array.from( - { length: samples + throwaway }, - ( _, i ) => i + 1 - ); + const iterations = sequence( 1, samples + throwaway ); it.each( iterations )( `trace large post loading durations (%i of ${ iterations.length })`, From 8486b48fc4994a0624368445cb2861bcc188a77e Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Wed, 1 Mar 2023 13:28:58 +0100 Subject: [PATCH 17/21] Hold the PostContent block render until userCanEdit status loaded --- packages/block-library/src/post-content/edit.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/post-content/edit.js b/packages/block-library/src/post-content/edit.js index 7b41baf0fbae2..8385447581969 100644 --- a/packages/block-library/src/post-content/edit.js +++ b/packages/block-library/src/post-content/edit.js @@ -58,8 +58,12 @@ function EditableContent( { context = {} } ) { function Content( props ) { const { context: { queryId, postType, postId } = {} } = props; - const isDescendentOfQueryLoop = Number.isFinite( queryId ); const userCanEdit = useCanEditEntity( 'postType', postType, postId ); + if ( userCanEdit === undefined ) { + return null; + } + + const isDescendentOfQueryLoop = Number.isFinite( queryId ); const isEditable = userCanEdit && ! isDescendentOfQueryLoop; return isEditable ? ( From 6dcc9338871666f3c799630659e5f7db575e2545 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Wed, 1 Mar 2023 14:56:47 +0100 Subject: [PATCH 18/21] Don't use hacky selectors --- .../specs/performance/site-editor.test.js | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index 50e2042cfe4b9..c90ad3ce376e6 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -62,7 +62,6 @@ describe( 'Site Editor Performance', () => { ); await createNewPost( { postType: 'page' } ); - // await page.keyboard.type( 'Site Editor Performance Tests' ); await page.evaluate( ( _html ) => { const { parse } = window.wp.blocks; @@ -115,9 +114,9 @@ describe( 'Site Editor Performance', () => { postType: 'page', } ); - // Wait for the canvas to become editable. + // Wait for the first content block to render. await canvas().waitForSelector( - '[data-rich-text-placeholder="Write site tagline…"]' + '[data-type="core/post-content"] .wp-block' ); if ( i > throwaway ) { @@ -151,18 +150,15 @@ describe( 'Site Editor Performance', () => { postType: 'page', } ); - // Wait for the canvas to become editable. - await canvas().waitForSelector( - '[data-rich-text-placeholder="Write site tagline…"]' + // Wait for the first paragraph to be ready. + const firstParagraph = await canvas().waitForXPath( + '//p[contains(text(), "Lorem ipsum dolor sit amet")]' ); // Get inside the post content. await enterEditMode(); // Insert a new paragraph right under the first one. - const firstParagraph = await canvas().waitForXPath( - '//p[contains(text(), "Lorem ipsum dolor sit amet")]' - ); await firstParagraph.focus(); await insertBlock( 'Paragraph' ); From cc09c6fde15f1734340ebbf76b561e2dae83651c Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Thu, 2 Mar 2023 13:57:27 +0100 Subject: [PATCH 19/21] Move `sequence()` to `utils.js` --- packages/e2e-tests/specs/performance/site-editor.test.js | 4 +--- packages/e2e-tests/specs/performance/utils.js | 4 ++++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index c90ad3ce376e6..0fccc34b37eba 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -26,13 +26,11 @@ import { deleteFile, getTypingEventDurations, getLoadingDurations, + sequence, } from './utils'; jest.setTimeout( 1000000 ); -const sequence = ( start, length ) => - Array.from( { length }, ( _, i ) => i + start ); - const results = { serverResponse: [], firstPaint: [], diff --git a/packages/e2e-tests/specs/performance/utils.js b/packages/e2e-tests/specs/performance/utils.js index a5829bbc8feda..772070cea7082 100644 --- a/packages/e2e-tests/specs/performance/utils.js +++ b/packages/e2e-tests/specs/performance/utils.js @@ -124,3 +124,7 @@ export async function getLoadingDurations() { export function sum( arr ) { return arr.reduce( ( a, b ) => a + b, 0 ); } + +export function sequence( start, length ) { + return Array.from( { length }, ( _, i ) => i + start ); +} From 5623f43bebc772ff9a8d3fdb630daf0b5e3d7942 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Fri, 3 Mar 2023 14:21:19 +0100 Subject: [PATCH 20/21] Wait for any block for the `firstBlock` load time --- packages/e2e-tests/specs/performance/site-editor.test.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index 0fccc34b37eba..a89d6e4bdedc1 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -112,11 +112,10 @@ describe( 'Site Editor Performance', () => { postType: 'page', } ); - // Wait for the first content block to render. - await canvas().waitForSelector( - '[data-type="core/post-content"] .wp-block' - ); + // Wait for the first block. + await canvas().waitForSelector( '.wp-block' ); + // Save results. if ( i > throwaway ) { const { serverResponse, From 3da31c01875d1b2a5625c25307613bf3d420dba8 Mon Sep 17 00:00:00 2001 From: Bart Kalisz Date: Mon, 6 Mar 2023 14:34:37 +0100 Subject: [PATCH 21/21] Use single parent `describe` for consistency --- .../specs/performance/site-editor.test.js | 158 +++++++++--------- 1 file changed, 78 insertions(+), 80 deletions(-) diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js index a89d6e4bdedc1..69f5c336c7414 100644 --- a/packages/e2e-tests/specs/performance/site-editor.test.js +++ b/packages/e2e-tests/specs/performance/site-editor.test.js @@ -94,97 +94,95 @@ describe( 'Site Editor Performance', () => { await activateTheme( 'twentytwentyone' ); } ); - describe( 'Loading', () => { - // Number of measurements to take. - const samples = 3; - // Number of throwaway measurements to perform before recording samples. - // Having at least one helps ensure that caching quirks don't manifest - // in the results. - const throwaway = 1; - const iterations = sequence( 1, samples + throwaway ); - - it.each( iterations )( - `trace large post loading durations (%i of ${ iterations.length })`, - async ( i ) => { - // Open the test page in Site Editor. - await visitSiteEditor( { - postId, - postType: 'page', - } ); - - // Wait for the first block. - await canvas().waitForSelector( '.wp-block' ); - - // Save results. - if ( i > throwaway ) { - const { - serverResponse, - firstPaint, - domContentLoaded, - loaded, - firstContentfulPaint, - firstBlock, - } = await getLoadingDurations(); - - results.serverResponse.push( serverResponse ); - results.firstPaint.push( firstPaint ); - results.domContentLoaded.push( domContentLoaded ); - results.loaded.push( loaded ); - results.firstContentfulPaint.push( firstContentfulPaint ); - results.firstBlock.push( firstBlock ); - } - - expect( true ).toBe( true ); - } - ); - } ); - - describe( 'Typing', () => { - it( 'trace 200 characters typing sequence inside a large post', async () => { + // Number of loading measurements to take. + const loadingSamples = 3; + // Number of throwaway measurements to perform before recording samples. + // Having at least one helps ensure that caching quirks don't manifest + // in the results. + const loadingSamplesThrowaway = 1; + const loadingIterations = sequence( + 1, + loadingSamples + loadingSamplesThrowaway + ); + it.each( loadingIterations )( + `Loading (%i of ${ loadingIterations.length })`, + async ( i ) => { // Open the test page in Site Editor. await visitSiteEditor( { postId, postType: 'page', } ); - // Wait for the first paragraph to be ready. - const firstParagraph = await canvas().waitForXPath( - '//p[contains(text(), "Lorem ipsum dolor sit amet")]' - ); - - // Get inside the post content. - await enterEditMode(); + // Wait for the first block. + await canvas().waitForSelector( '.wp-block' ); + + // Save results. + if ( i > loadingSamplesThrowaway ) { + const { + serverResponse, + firstPaint, + domContentLoaded, + loaded, + firstContentfulPaint, + firstBlock, + } = await getLoadingDurations(); + + results.serverResponse.push( serverResponse ); + results.firstPaint.push( firstPaint ); + results.domContentLoaded.push( domContentLoaded ); + results.loaded.push( loaded ); + results.firstContentfulPaint.push( firstContentfulPaint ); + results.firstBlock.push( firstBlock ); + } - // Insert a new paragraph right under the first one. - await firstParagraph.focus(); - await insertBlock( 'Paragraph' ); + expect( true ).toBe( true ); + } + ); + + it( 'Typing', async () => { + // Open the test page in Site Editor. + await visitSiteEditor( { + postId, + postType: 'page', + } ); - // Start tracing. - const traceFile = __dirname + '/trace.json'; - await page.tracing.start( { - path: traceFile, - screenshots: false, - categories: [ 'devtools.timeline' ], - } ); + // Wait for the first paragraph to be ready. + const firstParagraph = await canvas().waitForXPath( + '//p[contains(text(), "Lorem ipsum dolor sit amet")]' + ); - // Type "x" 200 times. - await page.keyboard.type( new Array( 200 ).fill( 'x' ).join( '' ) ); - - // Stop tracing and save results. - await page.tracing.stop(); - const traceResults = JSON.parse( readFile( traceFile ) ); - const [ keyDownEvents, keyPressEvents, keyUpEvents ] = - getTypingEventDurations( traceResults ); - for ( let i = 0; i < keyDownEvents.length; i++ ) { - results.type.push( - keyDownEvents[ i ] + keyPressEvents[ i ] + keyUpEvents[ i ] - ); - } + // Get inside the post content. + await enterEditMode(); - // Delete the original trace file. - deleteFile( traceFile ); + // Insert a new paragraph right under the first one. + await firstParagraph.focus(); + await insertBlock( 'Paragraph' ); - expect( true ).toBe( true ); + // Start tracing. + const traceFile = __dirname + '/trace.json'; + await page.tracing.start( { + path: traceFile, + screenshots: false, + categories: [ 'devtools.timeline' ], } ); + + // Type "x" 200 times. + await page.keyboard.type( new Array( 200 ).fill( 'x' ).join( '' ) ); + + // Stop tracing and save results. + await page.tracing.stop(); + const traceResults = JSON.parse( readFile( traceFile ) ); + const [ keyDownEvents, keyPressEvents, keyUpEvents ] = + getTypingEventDurations( traceResults ); + for ( let i = 0; i < keyDownEvents.length; i++ ) { + results.type.push( + keyDownEvents[ i ] + keyPressEvents[ i ] + keyUpEvents[ i ] + ); + } + + // Delete the original trace file. + deleteFile( traceFile ); + + expect( true ).toBe( true ); } ); } );