This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Fix 404 and 500 errors in E2E test logs, editor and front-end. #5989
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
rubikuserbot
requested review from
a team and
frontdevde
and removed request for
a team
|
Size Change: -1.66 kB (0%) Total Size: 862 kB
ℹ️ View Unchanged
|
tomasztunik
changed the title
tomasztunik
changed the title
tomasztunik
changed the title
sunyatasattva
added
the
skip-changelog
This was referenced Mar 7, 2022
gigitux
reviewed
Mar 7, 2022
There was a problem hiding this comment.
Thanks for working on this! 💪
404 errors disappear, but for the tests, I have some errors in my local env, but on CI, everything looks great. I guess it is a problem with my env.
I added two comments about the package.json. My suggestion is to do another npm install to generate a new package-lock.json.
tomasztunik
commented
Mar 7, 2022
tomasztunik
force-pushed
the
fix/5945-404-errors
branch
from
c8c0f47 to
c63d4b4
Compare
we want to always load lazy loaded components from their default paths to avoid having to handle importing components in different ways for regular and atomic component packages
The Site Editor is expecting the footer parts to be there or it will try to load them anyways and throw a 404. It's not breaking, but it's polluting the console.
Previously, we used a util called `trashAllPosts` which navigated to the post UI and deleted all the posts to tear down any side-effects of template editing tests. However, with a [recent change](WordPress/wordpress-develop@14e20f7), WP Core removed the UI for those and that made our tests meet a 500 error. Using the REST API should also make everything faster.
was still present in attribute-filter.test.js
Before it was responsible for enabling translations for all the atomic blocks
tomasztunik
force-pushed
the
fix/5945-404-errors
branch
2 times, most recently
from
e441cc6 to
6d88635
Compare
gigitux
force-pushed
the
fix/5945-404-errors
branch
from
6d88635 to
fa27249
Compare
tomasztunik
force-pushed
the
fix/5945-404-errors
branch
from
f908feb to
84bfa7f
Compare
…rce-gutenberg-products-block into fix/5945-404-errors
|
With @gigitux we've wanted to understand more the possible impact of changing the translations and we've learned a little bit more how the process works. Changing the chunk names will cause the translations to stop working as the loading of translation files is based on the md5 checksum of the translated filename path. Once new translation files are generated when deployed to w.org existing translations should be reused and new translation json file will be generated with md5 part reflecting changed block entrypoint filename. This release will require downloading of new translation files if it doesn't happen automatically :) So 🤞 but looks like it should be good. |
gigitux
force-pushed
the
fix/5945-404-errors
branch
from
90da334 to
e1260af
Compare
|
We should verify after realease if we need to do any manual trigger to regenerate translation files for new version |
gigitux
approved these changes
Mar 10, 2022
There was a problem hiding this comment.
Great job! Thanks for working on this @tomasztunik @sunyatasattva
We should verify after realease if we need to do any manual trigger to regenerate translation files for new version
Good point, but from what we discovered, the process should be pretty smooth 🤞
This was referenced Dec 11, 2023
5 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Motivation
Running the E2E tests and debugging their failures was becoming unwieldy. Our logs were polluted by large amounts of errors, most of which wouldn't make the tests fail, but were in fact captures of the browser's console logs. This made it hard to identify the actual culprits of a failed test run and to parse the logs output.
This PR aims to clean up all those false negatives, and output cleaner logs.
Changes
Originally, we had treated atomic blocks as different, since they were only supposed to be registered in case a parent component was consuming them. With the advent of Global Styles, and the move towards FSE where the atomic blocks are going to be used as product elements, this distinction made less sense.
Specifically, we had outputted the built components as custom-named webpack chunks, which meant that classes inheriting from
AbstractBlockneeded to hardcode this custom path to be compatible.This meant that we previously had to enqueue scripts in the editor to make sure they were loaded correctly (see, e.g., #5816), but the class handling of the assets were still trying to fetch them by their default path, causing a slew of
404errors which weren't really impacting the functionality of the application, but were triggering unnecessary network requests and were caught by Jest during our E2E tests, polluting the logs.Additionally, on the front-end it also caused more
404errors, due to blocks being registered normally as a requirement for enabling Global Styles.A side-effect of this PR is simplifying this configuration, as now default paths work and don't have to be hard-coded.
This change moves build output for atomic blocks from
build/atomic-block-components/_name_tobuild/product-_name_reflecting the actual block name and always prefixed with context namespace (product) to avoid possible name clashes.Currently AddToCart and SKU blocks were missing as the only blocks that did not support global styles yet, this adds basic setup so that we can unify how
Before AllProducts.php took the responsibility for registering all chunks which was easy to miss or not understand implications. Registered chunks in AllProducts were making them available for usage outside of AllProducts as well.
The theme
emptythemewas added as the most minimal block theme possible, in order to cleanly test Site Editor related features. As its purpose was supposed to be getting away with the most minimal of themes, thefootertemplate part had not originally been included.This caused 404 error as this template part is always fetched when user navigates in the site template editor.
Previously, we used a util called
trashAllPostswhich navigated to the post UI and deleted all the posts to tear down any side-effects of template editing tests.However, with a recent change introduced in WordPress 5.9.1, Core removed the UI for those and that made our tests meet a 500 error.
Using the REST API should also make all the tests faster.
waitForPuppeteer has deprecated the use of the generic
waitFormethod for more specific methods to avoid confusion.Deprecation warnings were polluting the logs, so we switched to more specific methods (i.e. mostly changed
waitForwithwaitForTimeout).Fixes #5945
Fixes #5957
Testing
Automated Tests
Manual Testing
How to test the changes in this Pull Request:
User Facing Testing
These are steps for user testing (where "user" is someone interacting with this change that is not editing any code).