-
Notifications
You must be signed in to change notification settings - Fork 147
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
Patterns API: Fix endpoint tests #393
base: trunk
Are you sure you want to change the base?
Conversation
This isn't testing much of anything, and the dynamic nature of the live content means this could just be 1 if the latest batch of patterns are all translations of a single pattern.
"image" no longer returns patterns without the term in the title, so the array is arbitrarily resorted (all sort weights are 0). Switching to "heading" returns some items without the term in the title.
The previous search term now matches a core pattern.
$response = send_request( '/patterns/1.0/?per_page=100' ); | ||
$response = send_request( '/patterns/1.0/?per_page=20' ); |
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.
Decreased this to 20 to prevent the API response from timing out. The number doesn't matter as much as getting a set number. 20 is also arbitrary.
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.
in the wpcut tests that were added some known patterns were used with structure that did break requests - I wonder if we should either do similar here, or have a mix of known and random patterns?
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.
some known patterns were used with structure that did break requests
Which known patterns are breaking requests? do you mean the category_slugs thing? This particular test would not catch that, but the other tests (that run assertResponseHasPattern
) will check individual pattern return types (like test_results_limited_to_requested_locale
& test_browse_patterns_by_category
).
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.
it was pattern id 199
which caused the errors - unsure if we want to do it statically or not like: https://api.wordpress.org/patterns/1.0/?include=199,229080,446939,482686,278112
$search_term = 'image'; | ||
$search_term = 'heading'; |
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.
"image" no longer returns patterns without the term in the title, so the array is arbitrarily resorted (all sort weights are 0). Switching to "heading" returns some items without the term in the title, so the sorting works.
'search_term' => 'two buttons', // Post ID 727. | ||
'search_term' => 'Two images with text and buttons', // Post ID 727. |
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.
"two buttons" matches a core pattern now.
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.
does this cover: https://github.com/WordPress/wordpress.org/pull/393/files#r1792990280?
We recently had an issue causing errors on client WP sites because the Patterns API returned malformed category data — see WordPress/pattern-directory#711. If these tests were working and easy to run, it would have been caught earlier.
These tests function as e2e tests running against production data, checking that the output behaves as expected. This PR fixes the tests by updating the test cases to better match the real content, and to check the expected type of all patterns in a response.
Note: I'll also add a schema test to the pattern directory plugin, which could run as part of the CI, and catch this before it even merged.
To test
Requires a sandbox (I think?)
composer.phar
to my home directory, there are also brew instructions in the dotorg repo at/wordpress/api/README.md
)composer install
composer run test -- --group patterns
The events tests are also failing, but that can be fixed in another PR.