Add regression E2E test for the bug that caused some wp_options to get corrupted data #32797
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: +6.07 kB (+1%) Total Size: 1.05 MB
ℹ️ View Unchanged
|
|
The flow being tested here also ends up testing the issue for the flow described in the issue above, as the underlying cause is the same. I reckon that it'd probably be better to test this at a lower level (see questions in the PR description), but maybe this test is good enough for now. |
fullofcaffeine
requested review from
ajitbohra,
nerrad,
ntwb,
spacedmonkey and
TimothyBJacobs
as code owners
fullofcaffeine
changed the title
fullofcaffeine
changed the title
fullofcaffeine
force-pushed
the
regress-test/wp-option-data-corruption-bug
branch
from
a16f0b5 to
eefbff7
Compare
aristath
approved these changes
Jun 18, 2021
There was a problem hiding this comment.
Assuming the changes in lib/rest-api.php get reverted and this PR eventually only includes the packages/e2e-tests/specs/misc/settings.test.js file, the code looks good and makes sense.
Thank you for this PR, if we had this test earlier, we(I) never would have made that error in v10.7.x 👍
|
It's great to cover against this regression from re-surfacing 👍 My only concern here is that strictly speaking, the e2e test doesn't really cover any Gutenberg-specific behavior; one could argue that it really should live in https://github.com/WordPress/wordpress-develop instead (and GB could arguably run those tests to make sure that it doesn't break any of Core's expected behavior, see #26418). It's also a fair point that we might be able to cover this with a unit test, if we find the function that updates the relevant option, and check the resulting changes 🤔 However, on a practical level, I agree that it's better to cover this behavior somehow in GB for now, since implementing #26418 is non-trivial, and not even without controversy. Furthermore, Maybe we can:
cc/ @desrosj, since you're my go-to person for Core e2e tests 😬 |
|
As a post-scriptum to my previous comment, I noticed that there is some unit test coverage in |
|
@ockham Thanks for the thorough review and experiments!
I can help with 3 now, I just don't know how to file an issue in |
… the wp_option data corruption bug
…eproduce the wp_option data corruption bug" This reverts commit 17eb33b.
fullofcaffeine
force-pushed
the
regress-test/wp-option-data-corruption-bug
branch
from
b8a11ed to
9ffad29
Compare
Ah right, it's kind of a mix 😬 It's on GH (https://github.com/WordPress/wordpress-develop/); however, it's not actually possible to file issues there (only PRs); you'd have to use Core Trac for that, https://core.trac.wordpress.org/. Apologies for my misleading earlier instructions! 😅
Yep, works for me! 👍 |
Trac ticket created here: https://core.trac.wordpress.org/ticket/53520#ticket. |
I agree that this test doesn't cover any functionality specific to the Gutenberg plugin so it should be moved to a proper place. |
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
Gutenberg v10.7.x introduced a bug that caused some
wp_options to get blank or corrupted data after (any) settings were updated in theoptions-general.phppage. This was eventually fixed in v10.7.4, with this PR: https://github.com/WordPress/gutenberg/releases/v10.7.4. See Automattic/wp-calypso#53447 and Automattic/wp-calypso#53431.This test doesn't check only for the fields mentioned in the issues above, but tries to see if other attributes had any changes after the settings were updated when they shouldn't have. I'm excluding some options that seem to change automatically (matching
_transientin the id: I don't know anything about those and didn't find specific docs about them) and of course, the option for the setting we updated in order to test, or it would always make the test fail :)How has this been tested?
It's been a while since v10.7.4, and I couldn't automatically revert #32229 in order to reproduce the change. I then did that manually in this commit that's part of this changeset. This commit will be removed before this PR is merged.
Negative scenario (default in this PR as of now):
specs/misc/settings.test.jsand watch it fail, like this:Positive scenario
17eb33b3c1;specs/misc/settings.test.jsand it should pass:Questions
Checklist:
*.native.jsfiles for terms that need renaming or removal).