-
Notifications
You must be signed in to change notification settings - Fork 203
Theme Directory: Guard against repopackage posts missing _status meta #705
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
Open
obenland
wants to merge
4
commits into
WordPress:trunk
Choose a base branch
from
obenland:fix/theme-directory-missing-status-meta
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
e127784
Theme Directory: Guard against repopackage posts missing _status meta.
obenland 7636e59
Theme Directory: Add unit tests for posts missing the _status meta.
obenland c412cc5
Theme Directory: Tests: Fix coding standards violations.
obenland aedd22e
Theme Directory: Fix cleanup of the theme post when a new upload fails.
obenland File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "core": "WordPress/WordPress#master", | ||
| "phpVersion": "8.4", | ||
| "plugins": [ | ||
| "../wordpress.org/public_html/wp-content/plugins/theme-directory" | ||
| ], | ||
| "lifecycleScripts": { | ||
| "afterStart": "bash theme-directory/bin/after-start-test.sh" | ||
| } | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| #!/bin/bash | ||
| # | ||
| # Runs after wp-env start for the test environment. | ||
| # Installs PHPUnit 11 and Yoast polyfills in the test container. | ||
| # | ||
|
|
||
| CONFIG="--config theme-directory/.wp-env.test.json" | ||
| RUN="npx wp-env $CONFIG run tests-cli" | ||
|
|
||
| echo "Installing PHPUnit 11 and polyfills..." | ||
| $RUN composer global require -W phpunit/phpunit:^11.0 2>&1 | ||
| $RUN composer require --dev yoast/phpunit-polyfills:^4.0 --working-dir=/wordpress-phpunit 2>&1 |
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
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
12 changes: 12 additions & 0 deletions
12
wordpress.org/public_html/wp-content/plugins/theme-directory/phpunit.xml
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| <phpunit | ||
| bootstrap="tests/bootstrap.php" | ||
| backupGlobals="false" | ||
| colors="true" | ||
| > | ||
| <testsuites> | ||
| <testsuite name="theme-directory"> | ||
| <directory suffix=".php">tests/</directory> | ||
| <exclude>tests/bootstrap.php</exclude> | ||
| </testsuite> | ||
| </testsuites> | ||
| </phpunit> |
185 changes: 185 additions & 0 deletions
185
...ess.org/public_html/wp-content/plugins/theme-directory/tests/Missing_Status_Meta_Test.php
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,185 @@ | ||
| <?php | ||
| /** | ||
| * Tests for handling repopackage posts that are missing the `_status` post meta. | ||
| * | ||
| * A hard failure during WPORG_Themes_Upload::import() can leave a repopackage | ||
| * post without `_status` (and `_ticket_id`) meta. WP_Post::__get() returns an | ||
| * empty string for missing meta, which used to fatal on PHP 8 wherever an | ||
| * array was assumed. | ||
| * | ||
| * @package theme-directory | ||
| */ | ||
|
|
||
| use PHPUnit\Framework\TestCase; | ||
|
|
||
| /** | ||
| * @group upload | ||
| * @group themes-api | ||
| */ | ||
| class Missing_Status_Meta_Test extends TestCase { | ||
|
|
||
| /** | ||
| * IDs of posts created during a test, deleted again on teardown. | ||
| * | ||
| * @var array | ||
| */ | ||
| protected $post_ids = array(); | ||
|
|
||
| /** | ||
| * Deletes the posts created during the test. | ||
| */ | ||
| protected function tearDown(): void { | ||
| /* | ||
| * The plugin prevents repopackages from being deleted; detach that | ||
| * specific guard while cleaning up the fixture posts. | ||
| */ | ||
| remove_filter( 'before_delete_post', 'wporg_theme_no_delete_repopackage' ); | ||
| foreach ( $this->post_ids as $post_id ) { | ||
| wp_delete_post( $post_id, true ); | ||
| } | ||
| add_filter( 'before_delete_post', 'wporg_theme_no_delete_repopackage' ); | ||
|
|
||
| $this->post_ids = array(); | ||
|
|
||
| parent::tearDown(); | ||
| } | ||
|
|
||
| /** | ||
| * Creates a repopackage post. | ||
| * | ||
| * @param array $meta Optional. Post meta to add to the post. | ||
| * @return WP_Post The created post. | ||
| */ | ||
| protected function create_repopackage( $meta = array() ) { | ||
| $post_id = wp_insert_post( array( | ||
| 'post_type' => 'repopackage', | ||
| 'post_status' => 'publish', | ||
| 'post_title' => 'Test Theme', | ||
| 'post_name' => 'test-theme', | ||
| 'post_author' => 1, | ||
| ) ); | ||
|
|
||
| $this->post_ids[] = $post_id; | ||
|
|
||
| foreach ( $meta as $key => $value ) { | ||
| add_post_meta( $post_id, $key, $value ); | ||
| } | ||
|
|
||
| return get_post( $post_id ); | ||
| } | ||
|
|
||
| /** | ||
| * A post with no meta at all should not fatal and yield no max_version. | ||
| */ | ||
| public function test_populate_post_with_meta_without_any_meta() { | ||
| $post = $this->create_repopackage(); | ||
| $upload = new WPORG_Themes_Upload(); | ||
|
|
||
| $theme = $upload->populate_post_with_meta( $post ); | ||
|
|
||
| $this->assertFalse( $theme->max_version ); | ||
| } | ||
|
|
||
| /** | ||
| * A post with versioned meta but no `_status` (an orphan from a failed | ||
| * import) should not fatal and yield no max_version. | ||
| */ | ||
| public function test_populate_post_with_meta_without_status_meta() { | ||
| $post = $this->create_repopackage( array( | ||
| '_requires' => array( '1.0.0' => '5.0' ), | ||
| '_author' => array( '1.0.0' => 'Test Author' ), | ||
| ) ); | ||
| $upload = new WPORG_Themes_Upload(); | ||
|
|
||
| $theme = $upload->populate_post_with_meta( $post ); | ||
|
|
||
| $this->assertFalse( $theme->max_version ); | ||
| $this->assertSame( array( '1.0.0' => '5.0' ), $theme->_requires ); | ||
| } | ||
|
|
||
| /** | ||
| * A post with valid `_status` meta should yield the highest version. | ||
| */ | ||
| public function test_populate_post_with_meta_with_status_meta() { | ||
| $post = $this->create_repopackage( array( | ||
| '_status' => array( | ||
| '1.1' => 'old', | ||
| '1.0' => 'old', | ||
| '1.10' => 'live', | ||
| ), | ||
| ) ); | ||
| $upload = new WPORG_Themes_Upload(); | ||
|
|
||
| $theme = $upload->populate_post_with_meta( $post ); | ||
|
|
||
| $this->assertSame( '1.10', $theme->max_version ); | ||
| } | ||
|
|
||
| /** | ||
| * Cleaning up a failed new upload should delete the theme post despite | ||
| * the fail-safe against repopackage deletion, and restore the fail-safe | ||
| * afterwards. | ||
| */ | ||
| public function test_delete_theme_post_bypasses_deletion_fail_safe() { | ||
| $post = $this->create_repopackage(); | ||
| $upload = new WPORG_Themes_Upload(); | ||
|
|
||
| $upload->theme_post = $post; | ||
| $result = $upload->delete_theme_post(); | ||
|
|
||
| $this->assertNotEmpty( $result ); | ||
| $this->assertNull( get_post( $post->ID ) ); | ||
| $this->assertSame( 10, has_filter( 'before_delete_post', 'wporg_theme_no_delete_repopackage' ) ); | ||
| } | ||
|
|
||
| /** | ||
| * The themes API should return an empty `versions` array for a published | ||
| * theme missing the `_status` meta, rather than fataling. | ||
| */ | ||
| public function test_theme_information_versions_without_status_meta() { | ||
| $this->create_repopackage(); | ||
|
|
||
| $api = new Themes_API( | ||
| 'theme_information', | ||
| array( | ||
| 'slug' => 'test-theme', | ||
| 'fields' => array( | ||
| 'versions' => true, | ||
| 'downloaded' => false, | ||
| 'screenshot_url' => false, | ||
| ), | ||
| ) | ||
| ); | ||
|
|
||
| $this->assertObjectNotHasProperty( 'error', $api->response ); | ||
| $this->assertSame( array(), $api->response->versions ); | ||
| } | ||
|
|
||
| /** | ||
| * The themes API should return all versions for a theme with valid | ||
| * `_status` meta. | ||
| */ | ||
| public function test_theme_information_versions_with_status_meta() { | ||
| $this->create_repopackage( array( | ||
| '_status' => array( | ||
| '1.0' => 'old', | ||
| '1.1' => 'live', | ||
| ), | ||
| ) ); | ||
|
|
||
| $api = new Themes_API( | ||
| 'theme_information', | ||
| array( | ||
| 'slug' => 'test-theme', | ||
| 'fields' => array( | ||
| 'versions' => true, | ||
| 'downloaded' => false, | ||
| 'screenshot_url' => false, | ||
| ), | ||
| ) | ||
| ); | ||
|
|
||
| $this->assertObjectNotHasProperty( 'error', $api->response ); | ||
| $this->assertSame( array( '1.0', '1.1' ), array_keys( $api->response->versions ) ); | ||
| } | ||
| } |
60 changes: 60 additions & 0 deletions
60
wordpress.org/public_html/wp-content/plugins/theme-directory/tests/bootstrap.php
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| <?php | ||
| /** | ||
| * PHPUnit bootstrap file. | ||
| * | ||
| * @package theme-directory | ||
| */ | ||
|
|
||
| namespace WordPressdotorg\Theme_Directory\Tests; | ||
|
|
||
| if ( 'cli' !== php_sapi_name() ) { | ||
| return; | ||
| } | ||
|
|
||
| $_tests_dir = getenv( 'WP_TESTS_DIR' ); | ||
|
|
||
| if ( ! $_tests_dir ) { | ||
| $pos = stripos( __FILE__, '/src/wp-content/plugins/' ); | ||
|
|
||
| if ( false !== $pos ) { | ||
| // Installed in a src checkout. | ||
| $_tests_dir = substr( __FILE__, 0, $pos ) . '/tests/phpunit/'; | ||
| } elseif ( file_exists( '/wordpress-phpunit/includes/functions.php' ) ) { | ||
| // wp-env test directory. | ||
| $_tests_dir = '/wordpress-phpunit/'; | ||
| } else { | ||
| // Assume a temp directory path. | ||
| $_tests_dir = rtrim( sys_get_temp_dir(), '/\\' ) . '/wordpress-tests-lib/tests/phpunit/'; | ||
| } | ||
| } | ||
|
|
||
| if ( ! file_exists( $_tests_dir . '/includes/functions.php' ) ) { | ||
| fwrite( STDERR, "Could not find {$_tests_dir}/includes/functions.php\n" ); | ||
| exit( 1 ); | ||
| } | ||
|
|
||
| // Set polyfills path if available (required by WP test suite). | ||
| if ( ! defined( 'WP_TESTS_PHPUNIT_POLYFILLS_PATH' ) && file_exists( $_tests_dir . '/vendor/yoast/phpunit-polyfills' ) ) { | ||
| define( 'WP_TESTS_PHPUNIT_POLYFILLS_PATH', $_tests_dir . '/vendor/yoast/phpunit-polyfills' ); | ||
| } | ||
|
|
||
| // Give access to tests_add_filter() function. | ||
| require_once $_tests_dir . '/includes/functions.php'; | ||
|
|
||
| /** | ||
| * Manually load the plugin being tested. | ||
| */ | ||
| function manually_load_plugin() { | ||
| require_once dirname( __DIR__ ) . '/theme-directory.php'; | ||
|
|
||
| /* | ||
| * These classes are only included on demand at runtime (on upload, or when | ||
| * serving an API request); load them up front for the tests. | ||
| */ | ||
| require_once dirname( __DIR__ ) . '/class-wporg-themes-upload.php'; | ||
| require_once dirname( __DIR__ ) . '/class-themes-api.php'; | ||
| } | ||
| tests_add_filter( 'muplugins_loaded', __NAMESPACE__ . '\manually_load_plugin' ); | ||
|
|
||
| // Start up the WP testing environment. | ||
| require $_tests_dir . '/includes/bootstrap.php'; |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.