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

Pattern directory: Add pattern endpoint test matching schema #712

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

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Oct 7, 2024

We recently had an issue causing errors on client WP sites because the Patterns API returned malformed category data — see #711. We did not have any tests to catch that change. This adds a very simple test to check the pattern API response against its own schema. This is not a perfect check (since the validation normalizes arrays), so I've also added some extra checks to make sure the pattern content is never empty, and the two array values used in core (category_slugs & meta.wpop_block_types) are sequential arrays.

See related tests in WordPress/wordpress.org#393

How to test the changes in this Pull Request:

  1. Start up the wp-env
  2. Run yarn test:php or yarn test:php -- --group rest-api for just this test
  3. It should pass ✅
  4. Comment out the array_values fix in plugins/pattern-directory/includes/pattern-post-type.php
  5. Run the test again
  6. It should fail ❌

@ryelle ryelle added [Component] Pattern Directory API The pattern API on WordPress.org, and/or the CPT endpoint [Component] Tooling Build tools, automated testing, source control, linting, etc labels Oct 7, 2024
@ryelle ryelle self-assigned this Oct 7, 2024
$result = rest_validate_value_from_schema( $pattern, $schema );
$this->assertTrue( $result );

// Pattern content should always exist.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should these comments be passed as messages in the assert functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, done in 61c8e97

// Pattern content should always exist.
$this->assertNotEmpty( $pattern['pattern_content'] );

// Check that these arrays are sequential, not associative arrays.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Assuming yes, but can't these be empty? Do we need to test for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They can be, and as long as they're arrays it shouldn't cause any issues in the core API, so I don't think we need to test that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Pattern Directory API The pattern API on WordPress.org, and/or the CPT endpoint [Component] Tooling Build tools, automated testing, source control, linting, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants