-
Notifications
You must be signed in to change notification settings - Fork 128
Improve Test Coverage for Auto Sizes Plugin #1879
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
sarthak-19
wants to merge
14
commits into
WordPress:trunk
Choose a base branch
from
sarthak-19:add/code_coverage_auto_sizes
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
14 commits
Select commit
Hold shift + click to select a range
3d75ba9
Add missing tests to improve code coverage for auto-sizes plugin
914d191
Update test data provider array
f9b0351
Add coverage ignore notation
1b51f1f
Add coverage ignore notation
fa97ef2
Merge branch 'trunk' into add/code_coverage_auto_sizes
sarthak-19 b501973
Update doc comments of test function
6038a46
Fixed comment spacing
a248246
Merge branch 'trunk' into add/code_coverage_auto_sizes
sarthak-19 8efea15
Update test case for ignored lined
b15d7fe
Add code test coverage for a continue statement
15a5c85
Removed test for test_auto_sizes_prime_attachment_caches
18d4d5e
Merge branch 'trunk' into add/code_coverage_auto_sizes
sarthak-19 ab81e22
Merge branch 'trunk' into add/code_coverage_auto_sizes
mukeshpanchal27 e764b51
Apply suggestions from code review
mukeshpanchal27 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
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
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 |
---|---|---|
|
@@ -266,6 +266,15 @@ public function data_provider_to_test_auto_sizes_update_content_img_tag(): array | |
'input' => '<img src="https://example.com/foo-300x225.jpg" srcset="https://example.com/foo-300x225.jpg 300w, https://example.com/foo-1024x768.jpg 1024w, https://example.com/foo-768x576.jpg 768w, https://example.com/foo-1536x1152.jpg 1536w, https://example.com/foo-2048x1536.jpg 2048w" sizes=",,,,,,,,,auto, (max-width: 650px) 100vw, 650px" loading="lazy">', | ||
'expected' => '<img src="https://example.com/foo-300x225.jpg" srcset="https://example.com/foo-300x225.jpg 300w, https://example.com/foo-1024x768.jpg 1024w, https://example.com/foo-768x576.jpg 768w, https://example.com/foo-1536x1152.jpg 1536w, https://example.com/foo-2048x1536.jpg 2048w" sizes="auto, ,,,,,,,,,auto, (max-width: 650px) 100vw, 650px" loading="lazy">', | ||
), | ||
'expected_when_no_img_tag' => array( | ||
'input' => '<p>No image here</p>', | ||
'expected' => '<p>No image here</p>', | ||
), | ||
'expected_when_not_responsive' => array( | ||
'input' => '<img src="https://example.com/foo.jpg" loading="lazy">', | ||
'expected' => '<img src="https://example.com/foo.jpg" loading="lazy">', | ||
), | ||
|
||
); | ||
} | ||
|
||
|
@@ -279,4 +288,101 @@ public function test_auto_sizes_update_content_img_tag( string $input, string $e | |
auto_sizes_update_content_img_tag( $input ) | ||
); | ||
} | ||
|
||
/** | ||
* @covers ::auto_sizes_attribute_includes_valid_auto | ||
*/ | ||
public function test_auto_sizes_attribute_includes_valid_auto(): void { | ||
// Test when 'auto' is the first item in the list. | ||
$this->assertTrue( auto_sizes_attribute_includes_valid_auto( 'auto, 100vw' ) ); | ||
|
||
// Test when 'auto' is not the first item in the list. | ||
$this->assertFalse( auto_sizes_attribute_includes_valid_auto( '100vw, auto' ) ); | ||
} | ||
|
||
/** | ||
* Provides data for the auto_sizes_update_image_attributes test. | ||
* | ||
* @return array<int,mixed[]> The data for the test cases. | ||
*/ | ||
public function data_provider_for_auto_sizes_update_image_attributes(): array { | ||
return array( | ||
array( | ||
array( | ||
'loading' => 'eager', | ||
'sizes' => '100vw', | ||
), | ||
array( | ||
'loading' => 'eager', | ||
'sizes' => '100vw', | ||
), | ||
), | ||
|
||
// Test when the image is not responsive. | ||
array( array( 'loading' => 'lazy' ), array( 'loading' => 'lazy' ) ), | ||
|
||
// Test when 'auto' already exists in the sizes attribute. | ||
array( | ||
array( | ||
'loading' => 'lazy', | ||
'sizes' => 'auto, 100vw', | ||
), | ||
array( | ||
'loading' => 'lazy', | ||
'sizes' => 'auto, 100vw', | ||
), | ||
), | ||
|
||
// Test when 'auto' needs to be added to the sizes attribute. | ||
array( | ||
array( | ||
'loading' => 'lazy', | ||
'sizes' => '100vw', | ||
), | ||
array( | ||
'loading' => 'lazy', | ||
'sizes' => 'auto, 100vw', | ||
), | ||
), | ||
Comment on lines
+310
to
+346
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would be better as associative arrays as then it becomes self-documenting, and the array keys of the top-level can be used in filtering. For example for the first item it would be better as: 'eager_something_blah_blah' => array(
'attr' => array(
'loading' => 'eager',
'sizes' => '100vw',
),
'expected' => array(
'loading' => 'eager',
'sizes' => '100vw',
),
), Naturally you can use the appropriate labels for the keys 😄 |
||
); | ||
} | ||
|
||
/** | ||
* @covers ::auto_sizes_update_image_attributes | ||
* | ||
* @dataProvider data_provider_for_auto_sizes_update_image_attributes | ||
* | ||
* @param array<string, mixed> $attr Attributes to be updated. | ||
* @param array<string, mixed> $expected Expected updated attributes. | ||
*/ | ||
public function test_auto_sizes_update_image_attributes( array $attr, array $expected ): void { | ||
$this->assertSame( $expected, auto_sizes_update_image_attributes( $attr ) ); | ||
} | ||
|
||
/** | ||
* @covers ::auto_sizes_update_content_img_tag | ||
*/ | ||
public function test_auto_sizes_update_content_img_tag_non_string_input(): void { | ||
/* | ||
* These tests are separate from the data provider approach because the function | ||
* auto_sizes_update_content_img_tag() expects a string as an argument. Passing a non-string | ||
* value would cause a TypeError. These tests ensure that the function behaves as expected | ||
* when it receives non-string inputs. | ||
*/ | ||
$this->assertSame( '', auto_sizes_update_content_img_tag( array() ) ); | ||
} | ||
|
||
/** | ||
* @covers ::auto_sizes_update_image_attributes | ||
*/ | ||
public function test_auto_sizes_update_image_attributes_with_null_input(): void { | ||
/* | ||
* This test is separate because the main test method uses a data provider | ||
* that expects the $attr parameter to be an array. Passing null directly | ||
* would cause a TypeError due to the type hint. By testing null separately, | ||
* we ensure that the function can handle null inputs gracefully without | ||
* modifying the type hint in the main test method. | ||
*/ | ||
$this->assertSame( array(), auto_sizes_update_image_attributes( null ) ); | ||
} | ||
} |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto. I don't think this should be ignored because technically this is a code path that can be tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The assertion to test for with
auto_sizes_prime_attachment_caches()
is a bit tricky though. For one, an assertion can assert that the input$content
is identical to the return value. But then otherwise, when there is aclass
with an attachment ID in it or noclass
attribute at all, then this means_prime_post_caches()
is called or not. I guess you could check to see if a query was done on the DB or not?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.