-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add regression test for editor JS crash caused by rtlcss parsing exception, take 2 #29598
Merged
fullofcaffeine
merged 2 commits into
trunk
from
add/regression-test-rtlcss-parser-comment-crash-take-2
Mar 8, 2021
Merged
Add regression test for editor JS crash caused by rtlcss parsing exception, take 2 #29598
fullofcaffeine
merged 2 commits into
trunk
from
add/regression-test-rtlcss-parser-comment-crash-take-2
Mar 8, 2021
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
… regress-test issues in rtlcss
6 tasks
Size Change: +1.37 kB (0%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
ockham
reviewed
Mar 8, 2021
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.
Thanks Marcelo, this is working great now -- and I love how elegant it turned out! 👏
The linter is flagging two phpcs errors -- I might just go ahead and fix those 😄
ockham
approved these changes
Mar 8, 2021
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.
Description
Issue: #29250.
Follow up to #29326.
The previous approach in #29326 worked but was too hacky. @ockham kindly pointed out that the WP instance used by the GB E2E tests can include plugins to help with testing, and that there are already two helper functions:
activatePlugin
anddeactivatePlugin
that automate the whole process of activate/deactivating a given plugin in the UI. Neat!This approach based on the suggestion by @ockham uses some modified code from https://github.com/yoavf/RTL-Tester (with due credit) to properly set the direction to RTL upon activation. It sets the RTL before all tests which acts as a regression test for the CSS parsing issue we experienced (and any similar issues that might happen at this level).
This also simplifies the given test by not requiring the RTL to be changed in the frontend using JS.
How to test
Easiest way is to checkout
v10.0.0
(or revert commit2f93c432a3
), thennpm ci && npm run build
and start a wp-env instance from there. Finally, in another Gutenberg copy, check out this branch (add/regression-test-rtlcss-parser-comment-crash
) and run this test (you might also callonly
in theit
to restrict the run to the single test added in this PR, if you'd like). All tests from thertl.test.js
should fail 🔴.Now, where you had
v10.0.0
checkout, checkoutv10.0.2
(ormaster
) instead, runnpm ci && npm run build
and go back to the other Gutenberg instance, and run the test again. All tests should pass 🟢.Types of changes
Change in E2E test (JS) + addition of a new test plugin (PHP).
Checklist: