-
Notifications
You must be signed in to change notification settings - Fork 4.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
Performance Tests: Separate page setup from test #53808
Merged
Merged
Changes from 13 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
d9be785
Utilize the new context created internally
WunderBart 80bd98d
Merge remote-tracking branch 'origin' into refactor/perf-use-test-con…
WunderBart 59be4bc
Remove unnecessary waiter
WunderBart 954270a
Add more consistency
WunderBart a5778c2
Merge remote-tracking branch 'origin' into refactor/perf-use-test-con…
WunderBart cb3505f
Merge remote-tracking branch 'origin' into refactor/perf-use-test-con…
WunderBart e49ce6b
Isolate page setup from test + abstract fixtures
WunderBart cc8158a
We're only waiting for the first block to be available
WunderBart 7000dec
Make the metrics fixture context-agnostic
WunderBart 26c93df
Merge remote-tracking branch 'origin' into refactor/perf-use-test-con…
WunderBart 55c071b
Make sample iteration consistent
WunderBart 1b00741
Merge with the Metrics e2e fixture
WunderBart 693caec
Define Trace interface
WunderBart 33a78eb
Avoid unnecessart type casting
WunderBart feb23d5
Assert non-null instead of casting
WunderBart 5c67b2d
Migrate PerfUtils to TS
WunderBart 71a1d58
Revert some unwanted removals
WunderBart d12b20d
Improve jsdoc consistency
WunderBart File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,11 +1,55 @@ | ||||||
/** | ||||||
* External dependencies | ||||||
*/ | ||||||
import type { Page } from '@playwright/test'; | ||||||
import type { Page, Browser } from '@playwright/test'; | ||||||
|
||||||
type EventType = | ||||||
| 'click' | ||||||
| 'focus' | ||||||
| 'focusin' | ||||||
| 'keydown' | ||||||
| 'keypress' | ||||||
| 'keyup' | ||||||
| 'mouseout' | ||||||
| 'mouseover'; | ||||||
|
||||||
interface TraceEvent { | ||||||
cat: string; | ||||||
name: string; | ||||||
dur?: number; | ||||||
args: { | ||||||
data?: { | ||||||
type: EventType; | ||||||
}; | ||||||
}; | ||||||
} | ||||||
|
||||||
interface Trace { | ||||||
traceEvents: TraceEvent[]; | ||||||
} | ||||||
|
||||||
interface LoadingDurations { | ||||||
serverResponse: number; | ||||||
firstPaint: number; | ||||||
domContentLoaded: number; | ||||||
loaded: number; | ||||||
firstContentfulPaint: number; | ||||||
timeSinceResponseEnd: number; | ||||||
} | ||||||
|
||||||
type MetricsConstructorProps = { | ||||||
page: Page; | ||||||
}; | ||||||
|
||||||
export class Metrics { | ||||||
constructor( public readonly page: Page ) { | ||||||
browser: Browser; | ||||||
page: Page; | ||||||
trace: Trace; | ||||||
|
||||||
constructor( { page }: MetricsConstructorProps ) { | ||||||
this.page = page; | ||||||
this.browser = page.context().browser()!; | ||||||
this.trace = { traceEvents: [] }; | ||||||
} | ||||||
|
||||||
/** | ||||||
|
@@ -37,11 +81,9 @@ export class Metrics { | |||||
* Returns time to first byte (TTFB) using the Navigation Timing API. | ||||||
* | ||||||
* @see https://web.dev/ttfb/#measure-ttfb-in-javascript | ||||||
* | ||||||
* @return {Promise<number>} TTFB value. | ||||||
*/ | ||||||
async getTimeToFirstByte() { | ||||||
return this.page.evaluate< number >( () => { | ||||||
async getTimeToFirstByte(): Promise< number > { | ||||||
return await this.page.evaluate< number >( () => { | ||||||
const { responseStart, startTime } = ( | ||||||
performance.getEntriesByType( | ||||||
'navigation' | ||||||
|
@@ -56,11 +98,9 @@ export class Metrics { | |||||
* | ||||||
* @see https://w3c.github.io/largest-contentful-paint/ | ||||||
* @see https://web.dev/lcp/#measure-lcp-in-javascript | ||||||
* | ||||||
* @return {Promise<number>} LCP value. | ||||||
*/ | ||||||
async getLargestContentfulPaint() { | ||||||
return this.page.evaluate< number >( | ||||||
async getLargestContentfulPaint(): Promise< number > { | ||||||
return await this.page.evaluate< number >( | ||||||
() => | ||||||
new Promise( ( resolve ) => { | ||||||
new PerformanceObserver( ( entryList ) => { | ||||||
|
@@ -82,11 +122,9 @@ export class Metrics { | |||||
* | ||||||
* @see https://github.com/WICG/layout-instability | ||||||
* @see https://web.dev/cls/#measure-layout-shifts-in-javascript | ||||||
* | ||||||
* @return {Promise<number>} CLS value. | ||||||
*/ | ||||||
async getCumulativeLayoutShift() { | ||||||
return this.page.evaluate< number >( | ||||||
async getCumulativeLayoutShift(): Promise< number > { | ||||||
return await this.page.evaluate< number >( | ||||||
() => | ||||||
new Promise( ( resolve ) => { | ||||||
let CLS = 0; | ||||||
|
@@ -108,4 +146,132 @@ export class Metrics { | |||||
} ) | ||||||
); | ||||||
} | ||||||
|
||||||
/** | ||||||
* Returns the loading durations using the Navigation Timing API. All the | ||||||
* durations exclude the server response time. | ||||||
*/ | ||||||
async getLoadingDurations(): Promise< LoadingDurations > { | ||||||
return await this.page.evaluate( () => { | ||||||
const [ | ||||||
{ | ||||||
requestStart, | ||||||
responseStart, | ||||||
responseEnd, | ||||||
domContentLoadedEventEnd, | ||||||
loadEventEnd, | ||||||
}, | ||||||
] = performance.getEntriesByType( | ||||||
'navigation' | ||||||
) as PerformanceNavigationTiming[]; | ||||||
const paintTimings = performance.getEntriesByType( | ||||||
'paint' | ||||||
) as PerformancePaintTiming[]; | ||||||
|
||||||
const firstPaintStartTime = paintTimings.find( | ||||||
( { name } ) => name === 'first-paint' | ||||||
)?.startTime as number; | ||||||
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. Nit: Instead of casting as number, how about we just use
Suggested change
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. Updated in feb23d5 |
||||||
|
||||||
const firstContentfulPaintStartTime = paintTimings.find( | ||||||
( { name } ) => name === 'first-contentful-paint' | ||||||
)?.startTime as number; | ||||||
|
||||||
return { | ||||||
// Server side metric. | ||||||
serverResponse: responseStart - requestStart, | ||||||
// For client side metrics, consider the end of the response (the | ||||||
// browser receives the HTML) as the start time (0). | ||||||
firstPaint: firstPaintStartTime - responseEnd, | ||||||
domContentLoaded: domContentLoadedEventEnd - responseEnd, | ||||||
loaded: loadEventEnd - responseEnd, | ||||||
firstContentfulPaint: | ||||||
firstContentfulPaintStartTime - responseEnd, | ||||||
timeSinceResponseEnd: performance.now() - responseEnd, | ||||||
}; | ||||||
} ); | ||||||
} | ||||||
|
||||||
/** | ||||||
* Starts Chromium tracing with predefined options for performance testing. | ||||||
* | ||||||
* @param options Options to pass to `browser.startTracing()`. | ||||||
*/ | ||||||
async startTracing( options = {} ): Promise< void > { | ||||||
return await this.browser.startTracing( this.page, { | ||||||
screenshots: false, | ||||||
categories: [ 'devtools.timeline' ], | ||||||
...options, | ||||||
} ); | ||||||
} | ||||||
|
||||||
/** | ||||||
* Stops Chromium tracing and saves the trace. | ||||||
*/ | ||||||
async stopTracing(): Promise< void > { | ||||||
const traceBuffer = await this.browser.stopTracing(); | ||||||
const traceJSON = JSON.parse( traceBuffer.toString() ); | ||||||
|
||||||
this.trace = traceJSON; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Returns the durations of all typing events. | ||||||
*/ | ||||||
getTypingEventDurations(): number[][] { | ||||||
return [ | ||||||
this.getEventDurations( 'keydown' ), | ||||||
this.getEventDurations( 'keypress' ), | ||||||
this.getEventDurations( 'keyup' ), | ||||||
]; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Returns the durations of all selection events. | ||||||
*/ | ||||||
getSelectionEventDurations(): number[][] { | ||||||
return [ | ||||||
this.getEventDurations( 'focus' ), | ||||||
this.getEventDurations( 'focusin' ), | ||||||
]; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Returns the durations of all click events. | ||||||
*/ | ||||||
getClickEventDurations(): number[][] { | ||||||
return [ this.getEventDurations( 'click' ) ]; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Returns the durations of all hover events. | ||||||
*/ | ||||||
getHoverEventDurations(): number[][] { | ||||||
return [ | ||||||
this.getEventDurations( 'mouseover' ), | ||||||
this.getEventDurations( 'mouseout' ), | ||||||
]; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Returns the durations of all events of a given type. | ||||||
* | ||||||
* @param eventType The type of event to filter. | ||||||
*/ | ||||||
getEventDurations( eventType: EventType ): number[] { | ||||||
if ( this.trace.traceEvents.length === 0 ) { | ||||||
throw new Error( | ||||||
'No trace events found. Did you forget to call stopTracing()?' | ||||||
); | ||||||
} | ||||||
|
||||||
return this.trace.traceEvents | ||||||
.filter( | ||||||
( item: TraceEvent ): boolean => | ||||||
item.cat === 'devtools.timeline' && | ||||||
item.name === 'EventDispatch' && | ||||||
item?.args?.data?.type === eventType && | ||||||
!! item.dur | ||||||
) | ||||||
.map( ( item ) => ( item.dur ? item.dur / 1000 : 0 ) ); | ||||||
} | ||||||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { PerfUtils } from './perf-utils'; |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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'm curious to know whether playwright automatically infers the return type of
evaluate
here so that we don't have to explicitly cast it as it might be out-of-sync. 🤔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.
Good call. Looks like there were more places where we cast unnecessarily. Addressed in 33a78eb
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.
Removed a little bit too much, reverted some lines in 71a1d58 😅