Performance Tests: Separate page setup from test #53808
Merged
Conversation
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
|
Size Change: 0 B Total Size: 1.62 MB ℹ️ View Unchanged
|
...instead of manually creating a new one. This is good practice as it allows us to utilize the Playwright fixtures for each page that is created, which is not possible when creating contexts manually. It also improves consistency as it ensures each page is created with the same initial params.
WunderBart
force-pushed
the
refactor/perf-use-test-context
branch
from
4476208
to
d9be785
Compare
WunderBart
commented
Aug 18, 2023
|
Flaky tests detected in feb23d5. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6259366983
|
WunderBart
force-pushed
the
refactor/perf-use-test-context
branch
from
80c5a02
to
d9be785
Compare
It might still be covered by the loading overlay, which doesn't mean it's not available, which is what we're interested in.
WunderBart
requested review from
kevin940726,
Mamaduka,
youknowriad,
swissspidy and
dmsnell
WunderBart
changed the title
swissspidy
reviewed
Sep 15, 2023
|
Thanks for the ping! On vacation right now, so only glanced over the code on my phone, but makes sense to put the fixtures there. I hope with #52993 we can then move the more reusable utils to the shared package |
dmsnell
reviewed
Sep 15, 2023
There was a problem hiding this comment.
Seems like a fine change to me, but I didn't scrutinize it too deeply. cc: @sgomes since you were involved in this effort before.
Thanks as always @WunderBart!
|
FYI #52993 is now merged. It's a bit confusing that |
Thanks for the heads up!
Right, it does make more sense to keep them with edit: OTOH, wouldn't it make more sense to move |
WunderBart
force-pushed
the
refactor/perf-use-test-context
branch
from
cfce284
to
1b00741
Compare
|
@swissspidy I moved the event tracing to Metrics in 1b00741. Let me know if that looks OK to you! Also, cc @kevin940726 here for insight on extending these fixtures. 🙏 |
kevin940726
approved these changes
Sep 21, 2023
| * Returns the loading durations using the Navigation Timing API. All the | ||
| * durations exclude the server response time. | ||
| */ | ||
| async getLoadingDurations(): Promise< LoadingDurations > { |
There was a problem hiding this comment.
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.
Good call. Looks like there were more places where we cast unnecessarily. Addressed in 33a78eb
There was a problem hiding this comment.
Removed a little bit too much, reverted some lines in 71a1d58 😅
|
|
||
| const firstPaintStartTime = paintTimings.find( | ||
| ( { name } ) => name === 'first-paint' | ||
| )?.startTime as number; |
There was a problem hiding this comment.
Nit: Instead of casting as number, how about we just use !?
Suggested change
| )?.startTime as number; | |
| )!.startTime; |
There was a problem hiding this comment.
Nit: Let's refactor this file into typescript if the tooling allows it?
There was a problem hiding this comment.
Refactored in 5c67b2d
WunderBart
force-pushed
the
refactor/perf-use-test-context
branch
from
ce9d7bf
to
b0fd7cf
Compare
WunderBart
force-pushed
the
refactor/perf-use-test-context
branch
from
b0fd7cf
to
d12b20d
Compare
mikachan
pushed a commit
that referenced
this pull request
Sep 27, 2023
mikachan
pushed a commit
that referenced
this pull request
Sep 28, 2023
# Conflicts: # test/performance/specs/post-editor.spec.js
fluiddot
pushed a commit
that referenced
this pull request
Oct 2, 2023
* Fix the install of system fonts from a font collection (#54713) * Fix set upload dir test (#54762) * Site Editor: Prevent unintended actions on the classic theme (#54422) * Add action and selector to track access to Patterns page * Add a URL parameter to check if the Patterns page was accessed directly * Revert lib changes * Fix critical error in the Post Editor * Explicitly add `! isBlockBasedTheme` with `isTemplatePartsMode` * Fix critical error in the Post Editor * Patterns: Fix back navigation after pattern creation (#54852) * Patterns: Fix category control width in site editor (#54853) * Patterns: Allow non-user patterns under Standard filter (#54756) * Performance Tests: Separate page setup from test (#53808) # Conflicts: # test/performance/specs/post-editor.spec.js * Performance Tests: Support legacy selector for expanded elements (#54690) * Paragraph: Make 'aria-label' consistent with other blocks (#54687) * Paragraph: Make 'aria-label' consistent with other blocks * Update tests * Try using BC labels in performance tests * Move lightbox render function to filter (#54670) * syntax refactor repace strpos with str_contains (#54832) * Font Library: avoid deprected error in test (#54802) * fix deprecated call * removing unwanted line * Fix the ShortcutProvider usage (#54851) Co-authored-by: Marin Atanasov <[email protected]> * Image: Ensure `false` values are preserved in memory when defined in `theme.json` (#54639) * Modify conditional when parsing config * Only drop the value if it's undefined. --------- Co-authored-by: Michal Czaplinski <[email protected]> * Use "Not synced" in place of "Standard" nomenclature for patterns (#54839) * Standard -> Not synced * Fix broken test --------- Co-authored-by: Glen Davies <[email protected]> * Format Library: Try to fix highlight popover jumping (#54736) * Move mime-type collection generation to a function that can be tested… (#54844) * Move mime-type collection generation to a function that can be tested. Refactored to use that function. * linting changes * Add unit tests to mime type getter * Fixed linting errors * test the entire output array and replace assertTrue by assertEquals * fixing docs --------- Co-authored-by: Matias Benedetto <[email protected]> * Ensure lightbox toggle is shown if block-level setting exists (#54878) * Block Editor: Update strings in blocks 'RenameModal' component (#54887) * Footnotes: Add aria-label to return links (#54843) * Add aria-label to footnotes front-end links. * Add visual output. Fix PHPCS errors. * Remove visual changes, fix in follow-up. * Font Library: Changed the OTF mime type expected value to be what PHP returns (#54886) * Changed the OTF mime type expected value to be what PHP returns * add unit test for otf file installation --------- Co-authored-by: madhusudhand <[email protected]> * Image: Fix layout shift when lightbox is opened and closed (#53026) * Replace overflow: hidden with JavaScript callback to prevent scrolling * Disable scroll callback on mobile; add comments; fix scrim styles The page jumps around when trying to override the scroll behavior on mobile, so I disabled it altogether. I've also added comments to clarify this and other decisions made around the scroll handling. While testing, I realized that the scrim was completely opaque during the zoom animation, which does not match the design, so I added a new animation specifically for the scrim to fix that. * Add handling for horizontally oriented pages * Move close button so that it's further from scrollbar on desktop * Fix call to handleScroll() and add touch callback to new render method * Improve lightbox experience on mobile To ensure pinch to zoom works as expected when the lightbox is open on mobile, I added logic to disable the scroll override when touch is detected. Without this, the scroll override kicks in whenever one tries to pinch to zoom, and it breaks the experience. I also revised the styles for the scrim to make it opaque, as having content visible outside of the lightbox is distracting when pinching to zoom on a mobile device, and I think having a consistent lightbox across devices will make for the best user experience. * Fix spacing * Add touch directives to block supports * Fix spacing in block supports * Rename attribute for clarity * Update comment * Update comments * Fix spacing --------- Co-authored-by: Ricardo Artemio Morales <[email protected]> * Font Library: move font uploads to a new tab (#54655) * move font uploads to a new tab in the modal * fix invalid success message and revert the dropzone to modal * add success notice for font uploads * make supported file formats message dynamic based on allowed extensions * update hint text and clear successful message with a fresh upload * Block custom CSS: Fix incorrect CSS when multiple root selectors (#53602) * Block custom CSS: Fix incorrect CSS when multiple root selectors * Fix PHP lint error * Use `scope_selector` and `append_to_selector` method and update unit test * Use `scopeSelector` and `appendToSelector` function and update JS unit test * Update packages/block-editor/src/components/global-styles/test/use-global-styles-output.js Co-authored-by: Aaron Robertshaw <[email protected]> * Update packages/block-editor/src/components/global-styles/test/use-global-styles-output.js Co-authored-by: Aaron Robertshaw <[email protected]> * Update packages/block-editor/src/components/global-styles/test/use-global-styles-output.js Co-authored-by: Aaron Robertshaw <[email protected]> * Update packages/block-editor/src/components/global-styles/test/use-global-styles-output.js Co-authored-by: Aaron Robertshaw <[email protected]> * Update packages/block-editor/src/components/global-styles/utils.js Co-authored-by: Aaron Robertshaw <[email protected]> * re-trigger CI --------- Co-authored-by: Aaron Robertshaw <[email protected]> * Add new e2e test for creating a pattern (#54855) * Use list role instead of listbox for patterns (#54884) * Popover: Fix the styles for components that use emotion within popovers (#54912) # Conflicts: # packages/components/CHANGELOG.md * Footnotes: use core’s meta revisioning if available (#52988) # Conflicts: # packages/block-library/src/footnotes/index.php * Remove base url from link control search results (#54553) * Expose baseURL setting on Post and Site Editors via block settings * Strip baseURL from rendered search results * Only fetch baseURL once in top level component * Simplify implementation to utilise URL parse functions * Improve comment wording to avoid referencing undefined var * Remove superfluous conditional * Decode URL prior to operations * Refactor for readability * Fix where url is not defined * Revert change to filter util * Ensure that filterURLForDisplay always receives a string as an arg * Make e2e test locator less strict * Prefer pipe * Force remove trailing slash * [Site Editor]: Update copy of using the default template in a page (#54728) * Remove overflow: hidden from the entity title h1 in the site editor sidebar (#54769) * Site Editor: Avoid same key warnings in template parts area listings (#54863) * TemplateAreas use template part clientId as key * HomeTemplateDetails use template part clientId as key * Cleanup useSelect hook * Fix ToolSelector popover variant (#54840) * Font Library: refactor endpoint permissions (#54829) * break the checking of user permission and file write permissions * return error 500 if the request to the install fonts endpoint needs write permissions and wordpress doens't have write permission on the server * do not ask file write permission on uninstall endpoint * Standardize the output of install and uninstall fonts endpoints Co-authored-by: Jason Crist <[email protected]> Co-authored-by: Jeff Ong <[email protected]> * Handle standardized output from install and uninstall endpoints in the frontend Co-authored-by: Jason Crist <[email protected]> Co-authored-by: Jeff Ong <[email protected]> * Update the install and unintall fonts endpoints unit tests for the new standardized output format Co-authored-by: Jason Crist <[email protected]> Co-authored-by: Jeff Ong <[email protected]> * fix the refersh call for the library Co-authored-by: Jason Crist <[email protected]> Co-authored-by: Jeff Ong <[email protected]> --------- Co-authored-by: Jason Crist <[email protected]> Co-authored-by: Jeff Ong <[email protected]> * Don’t use TypeScript files in scripts package (#54856) * Fix Search Block not updating in Nav block (#54823) * Avoid setState in render * Attempt at test coverage * Improve tests and make them work * Remove word-wrap property (#54866) --------- Co-authored-by: Matias Benedetto <[email protected]> Co-authored-by: Aki Hamano <[email protected]> Co-authored-by: Aaron Robertshaw <[email protected]> Co-authored-by: Bart Kalisz <[email protected]> Co-authored-by: George Mamadashvili <[email protected]> Co-authored-by: Artemio Morales <[email protected]> Co-authored-by: Riad Benguella <[email protected]> Co-authored-by: Marin Atanasov <[email protected]> Co-authored-by: Michal Czaplinski <[email protected]> Co-authored-by: Rich Tabor <[email protected]> Co-authored-by: Glen Davies <[email protected]> Co-authored-by: Jason Crist <[email protected]> Co-authored-by: Alex Stine <[email protected]> Co-authored-by: madhusudhand <[email protected]> Co-authored-by: Carlos Bravo <[email protected]> Co-authored-by: Ricardo Artemio Morales <[email protected]> Co-authored-by: Kai Hao <[email protected]> Co-authored-by: Adam Silverstein <[email protected]> Co-authored-by: Dave Smith <[email protected]> Co-authored-by: Nik Tsekouras <[email protected]> Co-authored-by: Ramon <[email protected]> Co-authored-by: Jason Crist <[email protected]> Co-authored-by: Jeff Ong <[email protected]> Co-authored-by: Pascal Birchler <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What?
This PR introduces a distinct separation between browser contexts used for setting up the test page and those used for running the actual tests. By doing this, we aim to boost metric stability, ensuring that any potential side effects from the setup phase don't impact the test metrics. By having a distinct testing phase, we can also iterate over the entire phase rather than within it (e.g., loading tests). This guarantees that every new page starts with identical initial parameters and no background pages remain open.
Additionally, this modification enables us to make full use of Playwright fixtures. These fixtures enhance consistency and minimize code duplication.
Testing Instructions
Performance tests should pass.