Skip to content
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

Open
wants to merge 6 commits into
base: trunk
Choose a base branch
from

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Oct 3, 2024

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?)

  1. Get composer on your sandbox if you don't have it already (I just downloaded the composer.phar to my home directory, there are also brew instructions in the dotorg repo at /wordpress/api/README.md)
  2. Install deps: composer install
  3. Run the tests: composer run test -- --group patterns
  4. The tests take about a minute, but all should pass except for one incomplete

The events tests are also failing, but that can be fixed in another PR.

ryelle added 6 commits October 3, 2024 17:37
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.
@ryelle ryelle self-assigned this Oct 3, 2024
Comment on lines -84 to +86
$response = send_request( '/patterns/1.0/?per_page=100' );
$response = send_request( '/patterns/1.0/?per_page=20' );
Copy link
Contributor Author

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.

Copy link
Contributor

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?

Copy link
Contributor Author

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).

Copy link
Contributor

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

Comment on lines -300 to +295
$search_term = 'image';
$search_term = 'heading';
Copy link
Contributor Author

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.

Comment on lines -263 to +258
'search_term' => 'two buttons', // Post ID 727.
'search_term' => 'Two images with text and buttons', // Post ID 727.
Copy link
Contributor Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants