-
Notifications
You must be signed in to change notification settings - Fork 2k
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
base: trunk
Are you sure you want to change the base?
Metrics: Add a new e2e/metrics folder to start writing some test to collect performance metrics #97780
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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' } ], | ||
}, | ||
}; |
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 () { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
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 ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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 |
||
} ); | ||
} ); |
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.