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.

Sign up for GitHub

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

Jump to bottom

Patterns API: Fix endpoint tests #393

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

Patterns API: Fix endpoint tests #393

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1af6d22
Return fewer patterns to avoid timeouts during test runs
ryelle Oct 3, 2024
ade8440
Remove terms count
ryelle Oct 3, 2024
43c6a17
Update title boosted test term
ryelle Oct 3, 2024
786960d
Update test search term
ryelle Oct 3, 2024
2b857ed
Add link to relevant issue for this incomplete test
ryelle Oct 3, 2024
c96caea
Test full response for matching the pattern format
ryelle Oct 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 18 additions & 21 deletions api.wordpress.org/public_html/patterns/1.0/tests/test-index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
*/
class Test_Patterns extends TestCase {
/**
* Asserts that an HTTP response is valid and contains a pattern.
* Asserts that an HTTP response is valid and contains items matching the pattern format.
*
* @param Requests_Response $response
*/
Expand All @@ -19,10 +19,12 @@ public function assertResponseHasPattern( $response ) {
$patterns = json_decode( $response->body );
$this->assertIsArray( $patterns );
$this->assertGreaterThan( 0, count( $patterns ) );
$this->assertIsString( $patterns[0]->title->rendered );
$this->assertIsInt( $patterns[0]->meta->wpop_viewport_width );
$this->assertIsArray( $patterns[0]->category_slugs );
$this->assertIsArray( $patterns[0]->keyword_slugs );
foreach ( $patterns as $pattern ) {
$this->assertIsString( $pattern->title->rendered );
$this->assertIsInt( $pattern->meta->wpop_viewport_width );
$this->assertIsArray( $pattern->category_slugs );
$this->assertIsArray( $pattern->keyword_slugs );
}
}

/**
Expand Down Expand Up @@ -81,19 +83,12 @@ public function get_term_slugs( $patterns ) {
* @group e2e
*/
public function test_browse_all_patterns() : void {
$response = send_request( '/patterns/1.0/?per_page=100' );
$response = send_request( '/patterns/1.0/?per_page=20' );
Comment on lines -84 to +86
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

$this->assertResponseHasPattern( $response );

// When all locales and keywords are included, there should be at least 100 patterns.
// When all locales and keywords are included, there should be at least 20 patterns.
$patterns = json_decode( $response->body );
$this->assertSame( 100, count( $patterns ) );

/*
* The exact number of unique categories will vary based on which cohort of pattens happen to be returned,
* but `3` seems like a safe minimum in practice.
*/
$term_slugs = $this->get_term_slugs( $patterns );
$this->assertGreaterThan( 3, count( $term_slugs ) );
$this->assertSame( 20, count( $patterns ) );
}

/**
Expand All @@ -108,7 +103,7 @@ public function test_browse_patterns_by_category() : void {
* This can't include a `pattern-keyword` param because of the workaround in
* `WordPressdotorg\Pattern_Directory\Pattern_Post_Type\register_rest_fields()`.
*/
$response = send_request( '/patterns/1.0/?pattern-categories=' . $button_term_id . '&locale=en_US' );
$response = send_request( '/patterns/1.0/?per_page=20&pattern-categories=' . $button_term_id . '&locale=en_US' );
$this->assertResponseHasPattern( $response );

$patterns = json_decode( $response->body );
Expand Down Expand Up @@ -260,7 +255,7 @@ public function data_search_patterns() {

// The Core keyword (11) is hardcoded in `test_search_patterns()`, so don't need to specify it here.
"only match Core posts" => array(
'search_term' => 'two buttons', // Post ID 727.
'search_term' => 'Two images with text and buttons', // Post ID 727.
Comment on lines -263 to +258
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.

does this cover: https://github.com/WordPress/wordpress.org/pull/393/files#r1792990280?

'locale' => 'en_US',
'match_expected' => false,
'expected_post_ids' => false,
Expand Down Expand Up @@ -297,7 +292,7 @@ public function data_search_patterns() {
* @group e2e
*/
public function test_search_title_match_boosted_above_description_match() : void {
$search_term = 'image';
$search_term = 'heading';
Comment on lines -300 to +295
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.

$locale = 'en_US';

$response = send_request( "/patterns/1.0/?search=$search_term&pattern-keywords=11&locale=$locale" );
Expand All @@ -314,8 +309,8 @@ public function test_search_title_match_boosted_above_description_match() : void

usort( $expectedPatterns, function( $a, $b ) use ( $search_term ) {
$adjustment = 0;
$found_in_title_a = false === stripos( $search_term, $a->title->rendered );
$found_in_title_b = false === stripos( $search_term, $b->title->rendered );
$found_in_title_a = false !== stripos( $search_term, $a->title->rendered );
$found_in_title_b = false !== stripos( $search_term, $b->title->rendered );

if ( $found_in_title_a && ! $found_in_title_b ) {
$adjustment = -1;
Expand Down Expand Up @@ -347,7 +342,9 @@ public function test_search_locale_sort() : void {

$this->assertAllPatternsMatchSearchTerm( $patterns, $search_term );

$this->markTestIncomplete(); // todo the following code works, but `WordPressdotorg\Pattern_Directory\Search\modify_es_query_args` isn't boosting the primary locale yet
// The following test works, but `WordPressdotorg\Pattern_Directory\Search\modify_es_query_args` isn't boosting the primary locale yet.
// See https://github.com/WordPress/pattern-directory/issues/347
$this->markTestIncomplete();

$actualOrder = array_column( array_column( $patterns, 'meta' ), 'wpop_locale' );

Expand Down