Debug performance tests issues #48208
Closed
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.33 MB ℹ️ View Unchanged
|
|
Currently, on
@youknowriad @kevin940726, any idea what might be causing it? |
|
If you don't mind, could you please test the following code once? Perhaps it is taking a long time to load the posted content. diff --git a/packages/e2e-tests/specs/performance/site-editor.test.js b/packages/e2e-tests/specs/performance/site-editor.test.js
index a14b40d72b..5be2fbdfc4 100644
--- a/packages/e2e-tests/specs/performance/site-editor.test.js
+++ b/packages/e2e-tests/specs/performance/site-editor.test.js
@@ -140,11 +140,12 @@ describe( 'Site Editor Performance', () => {
await page.waitForSelector( '.edit-site-visual-editor', {
timeout: 120000,
} );
- await canvas().waitForSelector( '.wp-block', { timeout: 120000 } );
+ await canvas().waitForSelector( '.wp-block', { timeout: 500000 } );
// Measuring typing performance inside the post content.
await canvas().waitForSelector(
- '[data-type="core/post-content"] [data-type="core/paragraph"]'
+ '[data-type="core/post-content"] [data-type="core/paragraph"]',
+ { timeout: 500000 }
);
await enterEditMode();
await canvas().click( |
|
Flaky tests detected in d96dbd0. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4216993939
|
👋 Hi @t-hamano! Just now, I've pushed a commit that should verify if the test post size is the issue (1e1ecde). Let's see if that works and then eventually try another approach 🤞 |
|
Looks like it's not the post size - it still loads with a WSOD canvas. The weirdest part, though, is that I cannot seem to reproduce locally no matter what I do. I've followed the exact same steps the CI does (I think), and I'm still getting pass on each run. I've also tried running the Going to try higher timeout values, as suggested by @t-hamano above. It would also be nice to have the console logs - I'll see if I can get them. edit: Just noticed that these timeouts are actually huge and I don't think they even last to the given value. The global timeout will most likely end the job before reaching edit again: |
|
Yes, the huge timeouts are just to find the cause 😄 This test failure logs indicate that this line has passed: Therefore, I suspect that it is not a complete WSOD canvas and that it is taking time to load the submitted content. |
|
Still the same thing. I'm heading out for the week, @t-hamano, so feel free to commandeer this branch. Good luck! 🙌 Also, cc @youknowriad @kevin940726 as I couldn't get to the bottom of the issue. Hopefully will be more lucky on Monday. |
|
Thanks for addressing this issue! I also put a call out to the |
kevin940726
force-pushed
the
try/archive-perf-artifacts
branch
from
29acbf4
to
749cd73
Compare
|
Based on the canvas HTML, it seems like the problem might be within the Query block?
|
kevin940726
force-pushed
the
try/archive-perf-artifacts
branch
from
7bb3e65
to
6fc3ac1
Compare
kevin940726
force-pushed
the
try/archive-perf-artifacts
branch
from
755b893
to
6d464e4
Compare
This is very strange. The test attempts to reference a template with a post type of page. If the emptytheme is used in the test, then the Singluar template should be referenced. And the text "No Results Found." is only present in
Are we not accessing the correct template? |
|
I have committed to find out what page the test indicates (i.e., the text of the |
|
Okay, according to artifacts, it looks like Puppeteer is looking at the Index template instead of the expected template 🤔 |
|
@t-hamano Yeah, you're right. It's not the expected outcome. At least we now know what went wrong 😅 . My guess would be that somehow The image in #48208 (comment) by @WunderBart also indicates that it's not loading the page template, as the sidebar will look different. |
|
#48063 is the first PR I've seen to have this error. Before that, the error was different: |
|
If you access a non-existent post in the site editor, the sidebar navigation itself does not appear. However, artifacts shows the Index template. So the editor canvas might not correctly parse the URL path?
|
|
Okay, 9bc7099 seems to fix the first issue. The page now loads and the selector successfully selects the paragraph block. As a matter of fact, the HEAD commit for the performance test was successfully completed. However, the trunk test now fails with a familiar error (
Looking at the screenshot, it seems like the page is focusing on a list item block. It's not allowed to insert a Paragraph block there hence the error. I have two guesses:
Let's try them one-by-one. |
|
Performance test passed 🤣 Thank you so much! |
|
So d96dbd0 works, but we still don't know the exact reason why it failed 😅 . Layout shifting seems like the most possible answer. (FWIW, if we ever migrate the performance test to Playwright then this might become trivial.) This branch is obviously not in a ready-to-merge state. I'll defer the optimal solution for others more familiar with the code (@WunderBart, @youknowriad, @dmsnell 🙇) . |
|
Any way we can isolate the problem and fix it in a separate PR? A lot of PRs are blocked by the failing performance test. |
|
yeah I don't understand why the particular change solves it but. happy to see this small change in its own PR to unblock PRs for folks |
|
Nice sleuthing @t-hamano @kevin940726, thank you! 🚀 I've isolated the fix to #48240. I think we should also merge the artifacts upload for the perf tests. I'll move it to another PR. |
|
Isolated artifacts upload to #48243 |
WunderBart
changed the title
ellatrix
pushed a commit
that referenced
this pull request
Feb 20, 2023
kevin940726
added a commit
that referenced
this pull request
Feb 21, 2023
* Focus on the paragraph instead clicking it See #48208 (comment) * Add missing fixes As per #48240 (review) and #48240 (comment) * Try without the extra path param As per #48240 (review) * Remove obsolete await As per #48240 (comment) * Revert "Try without the extra path param" This reverts commit 5fad8cd10c0765b8054ffdae7f6bd1840c7bba6c. * Revert "Revert "Try without the extra path param"" This reverts commit 87b1109c0517b27ffe8f5bd5fb417a37e7a822d4. * Revert "Revert "Revert "Try without the extra path param""" This reverts commit 2b931a8bccfbafa9665e638045c6d1847955d98e. * Remove unneeded changes * Describe discrepancy of inexplicably-required line in code via explanatory comment. * Use `canvas.focus()` instead of `canvas.click()` --------- Co-authored-by: Kai Hao <kai@kaihao.dev> Co-authored-by: Ella van Durpe <ella@vandurpe.com> Co-authored-by: Dennis Snell <dennis.snell@automattic.com> Co-authored-by: Tetsuaki Hamano <tetsuaki.hamano@gmail.com>
|
The performance test failure issue has been fixed in #48240. The artifacts upload issue has been submitted as #48243. So let's close this PR. Thank you for addressing this issue, @WunderBart! |
ntsekouras
pushed a commit
that referenced
this pull request
Feb 23, 2023
* Focus on the paragraph instead clicking it See #48208 (comment) * Add missing fixes As per #48240 (review) and #48240 (comment) * Try without the extra path param As per #48240 (review) * Remove obsolete await As per #48240 (comment) * Revert "Try without the extra path param" This reverts commit 5fad8cd10c0765b8054ffdae7f6bd1840c7bba6c. * Revert "Revert "Try without the extra path param"" This reverts commit 87b1109c0517b27ffe8f5bd5fb417a37e7a822d4. * Revert "Revert "Revert "Try without the extra path param""" This reverts commit 2b931a8bccfbafa9665e638045c6d1847955d98e. * Remove unneeded changes * Describe discrepancy of inexplicably-required line in code via explanatory comment. * Use `canvas.focus()` instead of `canvas.click()` --------- Co-authored-by: Kai Hao <kai@kaihao.dev> Co-authored-by: Ella van Durpe <ella@vandurpe.com> Co-authored-by: Dennis Snell <dennis.snell@automattic.com> Co-authored-by: Tetsuaki Hamano <tetsuaki.hamano@gmail.com>
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?
Let's keep this PR for debugging failing performance tests.