Add lightbox tests covering theme.json configurations, including edge cases #60150
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: +18.6 kB (+1%) Total Size: 1.73 MB
ℹ️ View Unchanged
|
gziolo
added
[Type] Automated Testing
gziolo
reviewed
Mar 27, 2024
gziolo
reviewed
Mar 27, 2024
packages/block-editor/src/components/url-popover/image-url-input-ui.js
Outdated
Show resolved
Hide resolved
|
Nice job covering the scenarios with configuring the lightbox using the
It would be great to explore an alternative approach so don’t have to create 4 new test themes to only validate the settings. It doesn’t seem like a scalable approach, given how many settings are there in the |
gziolo
reviewed
Mar 28, 2024
Mamaduka
reviewed
Mar 28, 2024
|
Flaky tests detected in 683318f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8482711228
|
@gziolo Thanks for the suggestion! I removed the extra themes and moved the |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
There is some flakiness in these tests that I'm still hoping to address, but I do think this is ready for reviewing. |
|
I don't fully understand why we need a new theme for these tests. If we use plugins with filters, can't we use |
In order to avoid having 4 plugins maybe it could have only 1 plugin that registers two site settings (with register_setting?): one for The plugin could use just one filter that can read from those settings and those settings can be updated in the tests by using the util Could something like that work? |
I initially included the new theme to avoid conflicting with other tests in case changes to the site templates somehow persisted — the cleanup code is a bit flaky when run locally. But you're right that we can do without this. Will continue looking at the cleanup code and remove the extra theme in a future push.
Thanks for the suggestion! I'm actually thinking to simplify these tests to tackle just one |
|
This PR is very big and I hadn't been able to resolve the flakiness. It's been a few weeks now and looking at it with fresh eyes, I'd like to extract pieces of this PR to work on and review separately, which should be easier on me and reviewers, so will close this. |
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?
Since the UI for the lightbox has now been updated in #57608 with extensive edge cases covered via #59890 (comment), this PR reenables and updates the old lightbox tests while adding many new ones.
Why?
We want to have test coverage on the lightbox to prevent regressions.
How?
theme.jsonconfigurations via test plugins to reach various scenarios, testing both the UI and the frontend.Testing Instructions
Run the tests locally with Docker running, using the command
npm run test:e2e:playwright -- image.spec.js --debug