-
Notifications
You must be signed in to change notification settings - Fork 109
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
Improve Test Coverage for Dominant Color Images Plugin #1837
Open
sarthak-19
wants to merge
3
commits into
WordPress:trunk
Choose a base branch
from
sarthak-19:update/dci-test-coverage
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.
+273
−6
Open
Changes from all commits
Commits
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 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 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 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 | ||||
---|---|---|---|---|---|---|
|
@@ -42,7 +42,7 @@ public function test_dominant_color_metadata( string $image_path, array $expecte | |||||
* | ||||||
* @dataProvider provider_get_dominant_color | ||||||
* | ||||||
* @covers ::dominant_color_get_dominant_color | ||||||
* @covers helper::dominant_color_get_dominant_color | ||||||
* | ||||||
* @param string $image_path Image path. | ||||||
* @param string[] $expected_color Expected color. | ||||||
|
@@ -91,7 +91,7 @@ public function test_has_transparency_metadata( string $image_path, array $expec | |||||
* | ||||||
* @dataProvider provider_get_dominant_color | ||||||
* | ||||||
* @covers ::dominant_color_get_dominant_color | ||||||
* @covers helper::dominant_color_get_dominant_color | ||||||
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.
Suggested change
|
||||||
* | ||||||
* @param string $image_path Image path. | ||||||
* @param string[] $expected_color Expected color. | ||||||
|
@@ -113,7 +113,7 @@ public function test_dominant_color_has_transparency( string $image_path, array | |||||
* | ||||||
* @dataProvider provider_get_dominant_color | ||||||
* | ||||||
* @covers ::dominant_color_img_tag_add_dominant_color | ||||||
* @covers hooks::dominant_color_img_tag_add_dominant_color | ||||||
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.
Suggested change
|
||||||
* | ||||||
* @param string $image_path Image path. | ||||||
* @param string[] $expected_color Expected color. | ||||||
|
@@ -199,7 +199,7 @@ public function data_dominant_color_img_tag_add_dominant_color_requires_proper_q | |||||
* | ||||||
* @dataProvider data_provider_dominant_color_check_inline_style | ||||||
* | ||||||
* @covers ::dominant_color_img_tag_add_dominant_color | ||||||
* @covers hooks::dominant_color_img_tag_add_dominant_color | ||||||
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.
Suggested change
|
||||||
* | ||||||
* @param string $filtered_image The filtered image markup. | ||||||
* Must include `src="%s" width="%d" height="%d"`. | ||||||
|
@@ -408,4 +408,105 @@ public function test_dominant_color_render_generator(): void { | |||||
$this->assertStringContainsString( 'generator', $tag ); | ||||||
$this->assertStringContainsString( 'dominant-color-images ' . DOMINANT_COLOR_IMAGES_VERSION, $tag ); | ||||||
} | ||||||
|
||||||
/** | ||||||
* @covers Dominant_Color_Image_Editor_GD::get_dominant_color | ||||||
*/ | ||||||
public function test_invalid_image_type(): void { | ||||||
$editor = new Dominant_Color_Image_Editor_GD( '/invalid/type' ); | ||||||
$result = $editor->get_dominant_color(); | ||||||
$this->assertWPError( $result ); | ||||||
$this->assertEquals( 'image_editor_dominant_color_error_no_image', $result->get_error_code() ); | ||||||
} | ||||||
|
||||||
/** | ||||||
* @covers Dominant_Color_Image_Editor_GD::get_dominant_color | ||||||
*/ | ||||||
public function test_corrupted_image_file(): void { | ||||||
$editor = new Dominant_Color_Image_Editor_GD( 'path/to/corrupted/file.jpg' ); | ||||||
$result = $editor->get_dominant_color(); | ||||||
$this->assertWPError( $result ); | ||||||
$this->assertEquals( 'image_editor_dominant_color_error_no_image', $result->get_error_code() ); | ||||||
} | ||||||
|
||||||
/** | ||||||
* @covers ::dominant_color_add_inline_style | ||||||
*/ | ||||||
public function test_dominant_color_add_inline_style(): void { | ||||||
dominant_color_add_inline_style(); | ||||||
|
||||||
// Verify the style was registered. | ||||||
global $wp_styles; | ||||||
$this->assertTrue( isset( $wp_styles->registered['dominant-color-styles'] ) ); | ||||||
|
||||||
// Verify the style was enqueued. | ||||||
$this->assertTrue( wp_style_is( 'dominant-color-styles', 'enqueued' ) ); | ||||||
|
||||||
// Verify the inline style was added. | ||||||
$inline_styles = $wp_styles->get_data( 'dominant-color-styles', 'after' ); | ||||||
$this->assertNotEmpty( $inline_styles ); | ||||||
$this->assertStringContainsString( | ||||||
'img[data-dominant-color]:not(.has-transparency) { background-color: var(--dominant-color); }', | ||||||
implode( '', $inline_styles ) | ||||||
); | ||||||
} | ||||||
|
||||||
/** | ||||||
* @covers ::dominant_color_admin_inline_style | ||||||
*/ | ||||||
public function test_dominant_color_admin_inline_style(): void { | ||||||
dominant_color_admin_inline_style(); | ||||||
|
||||||
// Verify the style was registered. | ||||||
global $wp_styles; | ||||||
$this->assertTrue( isset( $wp_styles->registered['dominant-color-admin-styles'] ) ); | ||||||
|
||||||
// Verify the style was enqueued. | ||||||
$this->assertTrue( wp_style_is( 'dominant-color-admin-styles', 'enqueued' ) ); | ||||||
|
||||||
// Verify the inline style was added. | ||||||
$inline_styles = $wp_styles->get_data( 'dominant-color-admin-styles', 'after' ); | ||||||
$this->assertNotEmpty( $inline_styles ); | ||||||
$this->assertStringContainsString( | ||||||
'.wp-core-ui .attachment-preview[data-dominant-color]:not(.has-transparency) { background-color: var(--dominant-color); }', | ||||||
implode( '', $inline_styles ) | ||||||
); | ||||||
} | ||||||
|
||||||
/** | ||||||
* @covers ::dominant_color_admin_script | ||||||
*/ | ||||||
public function test_dominant_color_admin_script(): void { | ||||||
ob_start(); | ||||||
dominant_color_admin_script(); | ||||||
$output = ob_get_clean(); | ||||||
|
||||||
// Verify if script tag exists. | ||||||
$this->assertStringEndsWith( '</script>', trim( $output ) ); | ||||||
|
||||||
// Verify the key elements of the script. | ||||||
$this->assertStringContainsString( 'tmpl.textContent.replace', $output ); | ||||||
$this->assertStringContainsString( 'data-dominant-color', $output ); | ||||||
$this->assertStringContainsString( 'data-has-transparency', $output ); | ||||||
$this->assertStringContainsString( '--dominant-color', $output ); | ||||||
} | ||||||
|
||||||
/** | ||||||
* @covers ::dominant_color_prepare_attachment_for_js | ||||||
*/ | ||||||
public function test_dominant_color_prepare_attachment_for_js(): void { | ||||||
$attachment = self::factory()->post->create_and_get( array( 'post_type' => 'attachment' ) ); | ||||||
|
||||||
$meta = array( | ||||||
'dominant_color' => 'ff0000', | ||||||
'has_transparency' => true, | ||||||
); | ||||||
|
||||||
$response = dominant_color_prepare_attachment_for_js( array(), $attachment, $meta ); | ||||||
|
||||||
$this->assertArrayHasKey( 'dominantColor', $response ); | ||||||
$this->assertArrayHasKey( 'hasTransparency', $response ); | ||||||
$this->assertEquals( 'ff0000', $response['dominantColor'] ); | ||||||
$this->assertTrue( $response['hasTransparency'] ); | ||||||
} | ||||||
} |
This file contains 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
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.
This doesn't seem right? The
::
here refers to the global namespace.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.
::
refers to the global namespace, and it works for rest of the functions however, when I'm trying to generate coverage report, for the functions where I've addedhelper
&hooks
it's showing 0 coverage without file reference.Previous Result :
When setting annotation to
* @covers ::dominant_color_get_dominant_color
&* @covers ::dominant_color_img_tag_add_dominant_color
helper.php
hooks.php
New Result :
When setting annotation to
* @covers helper::dominant_color_get_dominant_color
&* @covers hooks::dominant_color_img_tag_add_dominant_color
helper.php
hooks.php
So how should I proceed with this since
helper.php
&hooks.php
file don't have a class soClassName:functionName
annotations will not work I also tried adding namespace tohelper.php
&hooks.php
but it results in failing all the test cases and throws error?cc : @westonruter
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.
Strange. I don't know why that would be. I also don't understand adding
hooks
andhelper
to the beginning of those functions. They would seem to indicate there are classes calledhooks
andhelper
, which there aren't.You're seeing this with local coverage report, but are you also seeing it with the Codecov coverage report?
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.
Exactly, still for some reason it seems to work.
Yes, in Codecov coverage report also the same is happening.
@westonruter
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.
@thelovekesh any ideas?
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.
@sarthak-19 can you please upload the xml coverage report? maybe that can help analyze how code coverage driver is looking for things?
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.
@thelovekesh Here's the link to xml report : https://drive.google.com/drive/folders/1Mj8KLo6IcnqsYstaZkRekREtkBzn0-9z?usp=sharing