Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metrics: Add a new e2e/metrics folder to start writing some test to collect performance metrics #97780

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions packages/calypso-e2e/src/lib/pages/editor-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,30 @@ export class EditorPage {

//#region Basic Entry

/**
* Loads HTML into the editor.
* @param content Content to load.
*/
async loadHtmlContent( content: string ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish we could reuse loadBlocksFromHtml() from Gutenberg. Perhaps we can move to the utils package and reuse from there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably can't reuse the one from Gutenberg, Calypso's editor is a bit more complex (iframed within iframe sometimes) so we can't reuse Gutenberg's one until the Calypso's iframe is completely removed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add some TODOs here then - ideally we should be able to remove the iframe soon, and then it would be nice to reuse what we can.

const editorParent = await this.getEditorParent();
await editorParent.evaluate( ( _, html ) => {
// @ts-expect-error Untyped global variable.
const { parse } = window.wp.blocks;
// @ts-expect-error Untyped global variable.
const { dispatch } = window.wp.data;
const blocks = parse( html );

blocks.forEach( ( block: any ) => {
if ( block.name === 'core/image' ) {
delete block.attributes.id;
delete block.attributes.url;
}
} );

dispatch( 'core/block-editor' ).resetBlocks( blocks );
}, content );
}

/**
* Selects blank template from the template modal.
*/
Expand Down
10 changes: 10 additions & 0 deletions test/e2e/jest.metrics.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const baseConfig = require( '@automattic/calypso-e2e/src/jest-playwright-config' );

module.exports = {
...baseConfig,
cacheDirectory: '<rootDir>/../../.cache/jest',
testMatch: [ '<rootDir>/metrics/**/*.[jt]s' ],
transform: {
'\\.[jt]sx?$': [ 'babel-jest', { configFile: '../../babel.config.js' } ],
},
};
84 changes: 84 additions & 0 deletions test/e2e/metrics/editor.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { readFileSync } from 'fs';
import path from 'path';
import {
DataHelper,
envVariables,
EditorPage,
TestAccount,
getTestAccountByFeature,
envToFeatureKey,
PostsPage,
} from '@automattic/calypso-e2e';
import { Metrics } from '@wordpress/e2e-test-utils-playwright';
import { Browser, Page } from 'playwright';

declare const browser: Browser;
const results: Record< string, number[] > = {};

describe( DataHelper.createSuiteTitle( 'Metrics: Editor' ), function () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We keep reference those as just "metrics" but I think it might make sense to change them to "performance metrics" everywhere, since metrics is too generic.

const features = envToFeatureKey( envVariables );
const accountName = getTestAccountByFeature(
features,
// The default accounts for gutenberg+simple are `gutenbergSimpleSiteEdgeUser` for GB edge
// and `gutenbergSimpleSiteUser` for stable. The criteria below conflicts with the default
// one that would return the `gutenbergSimpleSiteUser`. We also can't define it as part of
// the default criteria, and should pass it here, as an override. For this specific function
// call, `simpleSitePersonalPlanUser` will be retured when gutenberg is stable, and siteType
// is simple.
[ { gutenberg: 'stable', siteType: 'simple', accountName: 'simpleSitePersonalPlanUser' } ]
);

let page: Page;
let editorPage: EditorPage;
let postsPage: PostsPage;

beforeAll( async () => {
page = await browser.newPage();

const testAccount = new TestAccount( accountName );
await testAccount.authenticate( page );
} );

it( 'Start and fill a test post', async function () {
postsPage = new PostsPage( page );
const metrics = await new Metrics( { page } );
await postsPage.visit();
await postsPage.newPost();
editorPage = new EditorPage( page );
await editorPage.waitUntilLoaded();
const filePath = path.join( __dirname, './fixtures/large-post.html' );
await editorPage.loadHtmlContent( readFileSync( filePath, 'utf8' ).trim() );
const canvas = await editorPage.getEditorCanvas();
await canvas.locator( '.wp-block' ).first().waitFor();
await editorPage.enterTitle( 'Test Post' );
await editorPage.publish();

const samples = 2;
const throwaway = 1;
const iterations = samples + throwaway;

for ( let i = 1; i <= iterations; i++ ) {
await page.reload();
editorPage = new EditorPage( page );
editorPage.waitUntilLoaded();
const canvas = await editorPage.getEditorCanvas();
// Wait for the first block.
await canvas.locator( '.wp-block' ).first().waitFor();
// Get the durations.
const loadingDurations = await metrics.getLoadingDurations();

// Save the results.
if ( i > throwaway ) {
Object.entries( loadingDurations ).forEach( ( [ metric, duration ] ) => {
const metricKey = metric === 'timeSinceResponseEnd' ? 'firstBlock' : metric;
if ( ! results[ metricKey ] ) {
results[ metricKey ] = [];
}
results[ metricKey ].push( duration );
} );
}
}

console.log( results );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are we planning to do with the results? Is that good enough for the first pass / first step of what you're planning? Let me know if you need any help with the CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a question in the PR description about this. I would love to use the "performance-reporter" from either Gutenberg or Woo (check the Woo PR) instead of console logging. It's just a way to dump the results for now.

I've spent some time trying to wire the performance reporter but failed, It needs to be added to the playwright config file but in calypso e2e tests, there's not playwright config file really, it's kind of buried in a series of scripts and configs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, the entire setup is a bit rudimentary, and while it was migrated to Playwright, it still bears the mark of old technology used before it 😅

This is the config file: https://github.com/Automattic/wp-calypso/blob/2b4ed1bcc9a77c61f4456a70b051200957a84fd7/packages/calypso-e2e/src/jest-playwright-config/index.js - as you can see it uses Jest Playwright as a test runner and was never migrated to @playwright/test.

Now, I haven't tried using a custom reporter there, but you should be able to hook it the same way. You might need to work with a separate config to be able to inherit the original config and specify the custom reporter. Try it out and let me know how it goes.

Alternatively, migrating to @playwright/test could make sense, although I'm not sure what challenges you might bump into and how long it would take. I bet it's outside of your initial scope 😉

} );
} );
Loading
Loading