Integration tests: remove client ID from fixtures #38685
Merged
Conversation
This file contains hidden or 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.14 MB ℹ️ View Unchanged
|
dmsnell
approved these changes
Feb 9, 2022
Member
dmsnell
left a comment
dmsnell
left a comment
There was a problem hiding this comment.
Thanks for the follow-up. I had wondered if clientId is necessary. Same for isValid but we had one isValid: false in there and I didn't want to poke it.
In testing I removed all the .json files and regenerated the fixture data with GENERATE_MISSING_FIXTURES and apart from a formatting difference, they were semantically identical.
Contributor
An The tests should probably fail if |
Member
I guess there are two: core cover gradient and core file pdf. Both of these tests are testing deprecated blocks and their updates and so I think the point of the test is to ensure we don't break that update process. If you see otherwise then yeah, let's zap the property. I'm reluctant to remove something though that I think people are relying on. |
Contributor
|
For the file block one, the fixture is wrong. It looks like it happened after this change - #37100. I think one of the anchor elements (the button one) should have an href that matches the It is difficult to adjust a deprecated fixture. It's not straightforward to use the editor to produce the markup, so I imagine this adjustment was done by hand, and that's how the error happened. (edit: here's a fix for the file fixture - #38725) The cover fixture looks like it was only just added - #38392. |
8 tasks
8 tasks
Member
|
Fix for the Cover Block deprecation: Thanks for the heads up, folks. |
Member
Member
Contributor
|
I can't think of a reason for an invalid fixture. So I agree, I think There might be added complexity in recursing through inner blocks and also checking their validity. Though I don't know whether that's strictly necessary, as an inner block should have its own fixture. |
ramonjd
added a commit
that referenced
this pull request
Feb 14, 2022
glendaviesnz
pushed a commit
that referenced
this pull request
Feb 21, 2022
glendaviesnz
pushed a commit
that referenced
this pull request
Feb 22, 2022
glendaviesnz
pushed a commit
that referenced
this pull request
Feb 24, 2022
cbravobernal
added
[Type] Automated Testing
glendaviesnz
pushed a commit
that referenced
this pull request
Feb 28, 2022
glendaviesnz
pushed a commit
that referenced
this pull request
Mar 16, 2022
glendaviesnz
pushed a commit
that referenced
this pull request
Mar 16, 2022
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
Similarly to #38638, the client ID is a useless piece of data to have in the test fixtures since it's just the block index.
JS change to review: https://github.com/WordPress/gutenberg/pull/38685/files?file-filters%5B%5D=.js&show-viewed-files=true
The fixtures are regenerated.
Testing Instructions
Screenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).