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

[Font Library] Update PHPUnit tests per Core coding standards and practices #58502

Merged
merged 11 commits into from
Feb 1, 2024
Merged

[Font Library] Update PHPUnit tests per Core coding standards and practices #58502

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
64625ec
Rename test classes.
hellofromtonya Jan 31, 2024
cdffdc8
Update covers tag
hellofromtonya Jan 31, 2024
5507b12
Use set_up_before_class() instead of __construct().
hellofromtonya Jan 31, 2024
fb28513
Use assertSame() instead of assertEquals().
hellofromtonya Jan 31, 2024
860ddb4
Rename test file to match func, class, or method.
hellofromtonya Jan 31, 2024
6122917
Update dataProvider usage, formatting, and docs.
hellofromtonya Jan 31, 2024
de5d6cb
Add assertion failure messages.
hellofromtonya Jan 31, 2024
58059df
Reorder resets / cleanup.
hellofromtonya Feb 1, 2024
4fe255e
Put expected as first arg in each assert.
hellofromtonya Feb 1, 2024
d002d7b
Add fonts and font-library group annotation.
hellofromtonya Feb 1, 2024
022869a
Messages: add endstop; single "exist"
hellofromtonya Feb 1, 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
118 changes: 75 additions & 43 deletions phpunit/tests/fonts/font-library/fontFamilyBackwardsCompatibility.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,28 @@
*
* @package WordPress
* @subpackage Font Library
*
* @group fonts
* @group font-library
*/
class Tests_Font_Family_Backwards_Compatibility extends WP_UnitTestCase {
private $post_ids_to_delete = array();
Copy link
Contributor

@anton-vlasenko anton-vlasenko Feb 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I can say that this is a redundant initialization of the post_ids_to_delete property since it gets initialized in the set_up() method anyway.
However, this could also be considered good coding practice to always initialize variables before using them.
Nonetheless, this falls into the category of an ultra-nitpick and personal preference; in my opinion, it doesn't justify creating a new pull request to address it. The rest of the code looks good to me. Great work.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that the backwards compatibility code for Font Families shouldn't go into Core. This is only to handle the refactor in the latest Gutenberg release, to keep installed fonts working for sites that were already using the Font Library feature from Gutenberg prior to 17.6.

I should have added a @core-merge comment in this test, as well, sorry about that.

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case should we break this test into its own file?

Copy link
Contributor

Choose a reason for hiding this comment

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

The top of this file includes the @core-merge "do not merge" comment referenced above, so I believe this can be marked resolved?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think you're right. As long as gutenberg_convert_legacy_font_family_format continues to run for anyone who may have installed fonts previous to Gutenberg 17.6, this should be fine--and it looks like the function will run from this file without issue.

I also think it makes sense to delete this code after WP 6.5 is released--anticipating that almost all who were using the font library before would have upgraded Gutenberg and no longer need the legacy wp_font_family migration.


public function set_up() {
parent::set_up();

$this->post_ids_to_delete = array();
delete_option( 'gutenberg_font_family_format_converted' );
}

public function tear_down() {
parent::tear_down();
foreach ( $this->post_ids_to_delete as $post_id ) {
wp_delete_post( $post_id, true );
}

delete_option( 'gutenberg_font_family_format_converted' );

parent::tear_down();
}

public function test_font_faces_with_remote_src() {
Expand All @@ -27,79 +37,74 @@ public function test_font_faces_with_remote_src() {

gutenberg_convert_legacy_font_family_format();

$font_family = get_post( $font_family_id );
$font_family = $this->get_post( $font_family_id );
$font_faces = $this->get_font_faces( $font_family_id );

list( $font_face1, $font_face2, $font_face3 ) = $font_faces;

// Updated font family post.
$this->assertSame( 'wp_font_family', $font_family->post_type );
$this->assertSame( 'publish', $font_family->post_status );
$this->assertSame( 'wp_font_family', $font_family->post_type, 'The font family post type should be wp_font_family.' );
$this->assertSame( 'publish', $font_family->post_status, 'The font family post status should be publish.' );

$font_family_title = 'Open Sans';
$this->assertSame( $font_family_title, $font_family->post_title );
$this->assertSame( $font_family_title, $font_family->post_title, 'The font family post title should be Open Sans.' );

$font_family_slug = 'open-sans';
$this->assertSame( $font_family_slug, $font_family->post_name );
$this->assertSame( $font_family_slug, $font_family->post_name, 'The font family post name should be open-sans.' );

$font_family_content = wp_json_encode( json_decode( '{"fontFamily":"\'Open Sans\', sans-serif","preview":"https://s.w.org/images/fonts/16.7/previews/open-sans/open-sans.svg"}', true ) );
$this->assertSame( $font_family_content, $font_family->post_content );
$this->assertSame( $font_family_content, $font_family->post_content, 'The font family post content should match.' );

$meta = get_post_meta( $font_family_id, '_gutenberg_legacy_font_family', true );
$this->assertSame( $legacy_content, $meta );
$this->assertSame( $legacy_content, $meta, 'The _gutenberg_legacy_font_family post meta content should match.' );

// First font face post.
$this->assertSame( 'wp_font_face', $font_face1->post_type );
$this->assertSame( $font_family_id, $font_face1->post_parent );
$this->assertSame( 'publish', $font_face1->post_status );
$this->assertSame( 'wp_font_face', $font_face1->post_type, 'The 1st font face post type should be wp_font_face.' );
$this->assertSame( $font_family_id, $font_face1->post_parent, 'The 1st font face post parent should match.' );
$this->assertSame( 'publish', $font_face1->post_status, 'The 1st font face post status should be publish.' );

$font_face1_title = 'open sans;normal;400;100%;U+0-10FFFF';
$this->assertSame( $font_face1_title, $font_face1->post_title );
$this->assertSame( sanitize_title( $font_face1_title ), $font_face1->post_name );
$this->assertSame( $font_face1_title, $font_face1->post_title, 'The 1st font face post title should match.' );
$this->assertSame( sanitize_title( $font_face1_title ), $font_face1->post_name, 'The 1st font face post name should match.' );

$font_face1_content = wp_json_encode( json_decode( '{"fontFamily":"Open Sans","fontStyle":"normal","fontWeight":"400","preview":"https://s.w.org/images/fonts/16.7/previews/open-sans/open-sans-400-normal.svg","src":"https://fonts.gstatic.com/s/opensans/v35/memSYaGs126MiZpBA-UvWbX2vVnXBbObj2OVZyOOSr4dVJWUgsjZ0C4nY1M2xLER.ttf"}' ) );
$this->assertSame( $font_face1_content, $font_face1->post_content );
$this->assertSame( $font_face1_content, $font_face1->post_content, 'The 1st font face post content should match.' );

// With a remote url, file post meta should not be set.
$meta = get_post_meta( $font_face1->ID, '_wp_font_face_file', true );
$this->assertSame( '', $meta );
$this->assertSame( '', $meta, 'The _wp_font_face_file post meta for the 1st font face should be an empty string.' );

// Second font face post.
$this->assertSame( 'wp_font_face', $font_face2->post_type );
$this->assertSame( $font_family_id, $font_face2->post_parent );
$this->assertSame( 'publish', $font_face2->post_status );
$this->assertSame( 'wp_font_face', $font_face2->post_type, 'The 2nd font face post type should be wp_font_face.' );
$this->assertSame( $font_family_id, $font_face2->post_parent, 'The 2md font face post type should be wp_font_face.' );
$this->assertSame( 'publish', $font_face2->post_status, 'The 2nd font face post status should be publish.' );

$font_face2_title = 'open sans;italic;400;100%;U+0-10FFFF';
$this->assertSame( $font_face2_title, $font_face2->post_title );
$this->assertSame( sanitize_title( $font_face2_title ), $font_face2->post_name );
$this->assertSame( $font_face2_title, $font_face2->post_title, 'The 2nd font face post title should match.' );
$this->assertSame( sanitize_title( $font_face2_title ), $font_face2->post_name, 'The 2nd font face post name should match.' );

$font_face2_content = wp_json_encode( json_decode( '{"fontFamily":"Open Sans","fontStyle":"italic","fontWeight":"400","preview":"https://s.w.org/images/fonts/16.7/previews/open-sans/open-sans-400-italic.svg","src":"https://fonts.gstatic.com/s/opensans/v35/memQYaGs126MiZpBA-UFUIcVXSCEkx2cmqvXlWq8tWZ0Pw86hd0Rk8ZkaVcUwaERZjA.ttf"}' ) );
$this->assertSame( $font_face2_content, $font_face2->post_content );
$this->assertSame( $font_face2_content, $font_face2->post_content, 'The 2nd font face post content should match.' );

// With a remote url, file post meta should not be set.
$meta = get_post_meta( $font_face2->ID, '_wp_font_face_file', true );
$this->assertSame( '', $meta );
$this->assertSame( '', $meta, 'The _wp_font_face_file post meta for the 2nd font face should be an empty string.' );

// Third font face post.
$this->assertSame( 'wp_font_face', $font_face3->post_type );
$this->assertSame( $font_family_id, $font_face3->post_parent );
$this->assertSame( 'publish', $font_face3->post_status );
$this->assertSame( 'wp_font_face', $font_face3->post_type, 'The 3rd font face post type should be wp_font_face.' );
$this->assertSame( $font_family_id, $font_face3->post_parent, 'The 3rd font face post type should be wp_font_face.' );
$this->assertSame( 'publish', $font_face3->post_status, 'The 3rd font face post status should be publish.' );

$font_face3_title = 'open sans;normal;700;100%;U+0-10FFFF';
$this->assertSame( $font_face3_title, $font_face3->post_title );
$this->assertSame( sanitize_title( $font_face3_title ), $font_face3->post_name );
$this->assertSame( $font_face3_title, $font_face3->post_title, 'The 3rd font face post title should match.' );
$this->assertSame( sanitize_title( $font_face3_title ), $font_face3->post_name, 'The 3rd font face post name should match.' );

$font_face3_content = wp_json_encode( json_decode( '{"fontFamily":"Open Sans","fontStyle":"normal","fontWeight":"700","preview":"https://s.w.org/images/fonts/16.7/previews/open-sans/open-sans-700-normal.svg","src":"https://fonts.gstatic.com/s/opensans/v35/memSYaGs126MiZpBA-UvWbX2vVnXBbObj2OVZyOOSr4dVJWUgsg-1y4nY1M2xLER.ttf"}' ) );
$this->assertSame( $font_face3_content, $font_face3->post_content );
$this->assertSame( $font_face3_content, $font_face3->post_content, 'The 3rd font face post content should match.' );

// With a remote url, file post meta should not be set.
$meta = get_post_meta( $font_face3->ID, '_wp_font_face_file', true );
$this->assertSame( '', $meta );

wp_delete_post( $font_family_id, true );
wp_delete_post( $font_face1->ID, true );
wp_delete_post( $font_face2->ID, true );
wp_delete_post( $font_face3->ID, true );
$this->assertSame( '', $meta, 'The _wp_font_face_file post meta for the 3rd font face should be an empty string.' );
}

public function test_font_faces_with_local_src() {
Expand All @@ -110,16 +115,14 @@ public function test_font_faces_with_local_src() {
gutenberg_convert_legacy_font_family_format();

$font_faces = $this->get_font_faces( $font_family_id );
$this->assertCount( 1, $font_faces );

$this->assertCount( 1, $font_faces, 'There should be 1 font face.' );
$font_face = reset( $font_faces );

// Check that file meta is present.
$file_path = 'open-sans_normal_400.ttf';
$meta = get_post_meta( $font_face->ID, '_wp_font_face_file', true );
$this->assertSame( $file_path, $meta );

wp_delete_post( $font_family_id, true );
wp_delete_post( $font_face->ID, true );
$this->assertSame( $file_path, $meta, 'The _wp_font_face_file should match.' );
}

public function test_migration_only_runs_once() {
Expand All @@ -135,12 +138,10 @@ public function test_migration_only_runs_once() {
// Meta with backup content will not be present if migration isn't triggered.
$meta = get_post_meta( $font_family_id, '_gutenberg_legacy_font_family', true );
$this->assertSame( '', $meta );

wp_delete_post( $font_family_id, true );
}

protected function create_font_family( $content ) {
return wp_insert_post(
$post_id = wp_insert_post(
array(
'post_type' => 'wp_font_family',
'post_status' => 'publish',
Expand All @@ -149,16 +150,47 @@ protected function create_font_family( $content ) {
'post_content' => $content,
)
);

$this->store_id_for_cleanup_in_teardown( $post_id );

return $post_id;
}

private function get_post( $post_id ) {
$post = get_post( $post_id );

$this->store_id_for_cleanup_in_teardown( $post );

return $post;
}

protected function get_font_faces( $font_family_id ) {
return get_posts(
$posts = get_posts(
array(
'post_parent' => $font_family_id,
'post_type' => 'wp_font_face',
'order' => 'ASC',
'orderby' => 'id',
)
);

$this->store_id_for_cleanup_in_teardown( $posts );

return $posts;
}

private function store_id_for_cleanup_in_teardown( $post ) {
if ( null === $post ) {
return;
}

$posts = is_array( $post ) ? $post : array( $post );

foreach ( $posts as $post ) {
if ( null === $post ) {
continue;
}
$this->post_ids_to_delete[] = is_int( $post ) ? $post : $post->ID;
}
}
}
13 changes: 8 additions & 5 deletions phpunit/tests/fonts/font-library/fontLibraryHooks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@
*
* @package WordPress
* @subpackage Font Library
*
* @group fonts
* @group font-library
*/
class Tests_Fonts_FontLibraryHooks extends WP_UnitTestCase {

class Tests_Font_Library_Hooks extends WP_UnitTestCase {
public function test_deleting_font_family_deletes_child_font_faces() {
$font_family_id = self::factory()->post->create(
array(
Expand All @@ -33,8 +36,8 @@ public function test_deleting_font_family_deletes_child_font_faces() {

wp_delete_post( $font_family_id, true );

$this->assertNull( get_post( $font_face_id ) );
$this->assertNotNull( get_post( $other_font_face_id ) );
$this->assertNull( get_post( $font_face_id ), 'Font face post should also have been deleted.' );
$this->assertNotNull( get_post( $other_font_face_id ), 'The other post should exist.' );
}

public function test_deleting_font_faces_deletes_associated_font_files() {
Expand All @@ -43,8 +46,8 @@ public function test_deleting_font_faces_deletes_associated_font_files() {

wp_delete_post( $font_face_id, true );

$this->assertFalse( file_exists( $font_path ) );
$this->assertTrue( file_exists( $other_font_path ) );
$this->assertFalse( file_exists( $font_path ), 'The font file should have been deleted when the post was deleted' );
$this->assertTrue( file_exists( $other_font_path ), 'The other font file should exist' );
hellofromtonya marked this conversation as resolved.
Show resolved Hide resolved
}

protected function create_font_face_with_file( $filename ) {
Expand Down
8 changes: 5 additions & 3 deletions phpunit/tests/fonts/font-library/wpFontCollection/__construct.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@
* @covers WP_Font_Collection::__construct
*/
class Tests_Fonts_WpFontCollection_Construct extends WP_UnitTestCase {

/**
* @dataProvider data_should_assign_properties_from_php_config
*
* @param array $config Font collection config options.
* @param array $expected_data Expected output data.
* @param string $slug Font collection slug.
* @param array $config Font collection config options.
* @param array $expected_data Expected output data.
*/
public function test_should_assign_properties_from_php_config( $slug, $config, $expected_data ) {
$collection = new WP_Font_Collection( $slug, $config );
Expand All @@ -32,7 +34,7 @@ public function test_should_assign_properties_from_php_config( $slug, $config, $
/**
* Data provider.
*
* @return array[]
* @return array
*/
public function data_should_assign_properties_from_php_config() {
return array(
Expand Down
16 changes: 8 additions & 8 deletions phpunit/tests/fonts/font-library/wpFontCollection/loadFromJson.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ public function test_should_load_json_from_url() {
add_filter( 'pre_http_request', array( $this, 'mock_request' ), 10, 3 );

$config = WP_Font_Collection::load_from_json( 'https://localhost/fonts/mock-font-collection.json' );
$this->assertSame( self::$mock_collection_data, $config );

remove_filter( 'pre_http_request', array( $this, 'mock_request' ) );

$this->assertSame( self::$mock_collection_data, $config );
}

public function test_should_error_with_invalid_file_path() {
Expand All @@ -54,9 +54,9 @@ public function test_should_error_with_invalid_url_response() {
add_filter( 'pre_http_request', array( $this, 'mock_request_invalid_response' ), 10, 3 );

$config = WP_Font_Collection::load_from_json( 'https://localhost/fonts/missing-collection.json' );
$this->assertWPError( $config, 'font_collection_json_missing' );

remove_filter( 'pre_http_request', array( $this, 'mock_request_invalid_response' ) );

$this->assertWPError( $config, 'font_collection_json_missing' );
}

public function test_should_error_with_invalid_json_from_file() {
Expand All @@ -72,9 +72,9 @@ public function test_should_error_with_invalid_json_from_url() {
add_filter( 'pre_http_request', array( $this, 'mock_request_invalid_json' ), 10, 3 );

$config = WP_Font_Collection::load_from_json( 'https://localhost/fonts/invalid-collection.json' );
$this->assertWPError( $config, 'font_collection_decode_error' );

remove_filter( 'pre_http_request', array( $this, 'mock_request_invalid_json' ) );

$this->assertWPError( $config, 'font_collection_decode_error' );
}

public function test_should_error_with_json_from_file_missing_slug() {
Expand All @@ -93,9 +93,9 @@ public function test_should_error_with_json_from_url_missing_slug() {
$this->setExpectedIncorrectUsage( 'WP_Font_Collection::load_from_url' );

$config = WP_Font_Collection::load_from_json( 'https://localhost/fonts/missing-slug.json' );
$this->assertWPError( $config, 'font_collection_invalid_json' );

remove_filter( 'pre_http_request', array( $this, 'mock_request_missing_slug' ) );

$this->assertWPError( $config, 'font_collection_invalid_json' );
}

public function mock_request( $preempt, $args, $url ) {
Expand Down
9 changes: 4 additions & 5 deletions phpunit/tests/fonts/font-library/wpFontLibrary/getMimeTypes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,20 @@
class Tests_Fonts_WpFontLibrary_GetMimeTypes extends WP_Font_Library_UnitTestCase {

/**
*
* @dataProvider data_should_supply_correct_mime_type_for_php_version
*
* @param array $php_version_id PHP_VERSION_ID value.
* @param array $expected Expected mime types.
* @param int $php_version_id PHP_VERSION_ID value.
* @param array $expected Expected mime types.
*/
public function test_should_supply_correct_mime_type_for_php_version( $php_version_id, $expected ) {
$mimes = WP_Font_Library::get_expected_font_mime_types_per_php_version( $php_version_id );
$this->assertEquals( $mimes, $expected );
$this->assertSame( $expected, $mimes );
}

/**
* Data provider.
*
* @return array[]
* @return array
*/
public function data_should_supply_correct_mime_type_for_php_version() {
return array(
Expand Down
6 changes: 3 additions & 3 deletions phpunit/tests/fonts/font-library/wpFontUtils/formatFontFamily.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ class Tests_Fonts_WpFontUtils_FormatFontFamily extends WP_UnitTestCase {
/**
* @dataProvider data_should_format_font_family
*
* @param string $font_family Font family.
* @param string $expected Expected family.
* @param string $font_family Font family to test.
* @param string $expected Expected family.
*/
public function test_should_format_font_family( $font_family, $expected ) {
$this->assertSame(
Expand All @@ -30,7 +30,7 @@ public function test_should_format_font_family( $font_family, $expected ) {
/**
* Data provider.
*
* @return array[]
* @return array
*/
public function data_should_format_font_family() {
return array(
Expand Down
Loading
Loading