Improve Test Coverage for Auto Sizes Plugin #1879
Conversation
sarthak-19
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## trunk #1879 +/- ##
==========================================
+ Coverage 68.21% 68.53% +0.32%
==========================================
Files 93 91 -2
Lines 7947 7930 -17
==========================================
+ Hits 5421 5435 +14
+ Misses 2526 2495 -31
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
sarthak-19
requested review from
joemcgill and
mukeshpanchal27
as code owners
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @[email protected]. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
westonruter
added
[Plugin] Enhanced Responsive Images
| public function test_auto_sizes_prime_attachment_caches_with_empty_class(): void { | ||
| $img_tag = '<img src="https://example.com/foo.png">'; | ||
|
|
||
| $result = auto_sizes_prime_attachment_caches( $img_tag ); | ||
|
|
||
| // Assert that the output is the same as the input since no processing should occur. | ||
| $this->assertSame( $img_tag, $result ); | ||
| } |
There was a problem hiding this comment.
Note that this test doesn't actually test that the attachment caches were primed. Ideally there would be a data provider for this, one which tells the test function to create an attachment and get the image HTML for it via wp_get_attachment_image(). In this case, it should assert somehow that _prime_post_caches() was called. Then there could be another case in the data provider for an external image without a class attribute as you have here, and in this case it should assert that _prime_post_caches() was not called.
There was a problem hiding this comment.
Hi @westonruter thanks for the feedback however I'm a bit lost here in implementing this, could you help me out in this test implementation.
- Also what's the recommended approach to verify that _prime_post_caches() was called inside our function? Should I use a mock, a filter, or is there another WordPress-specific testing pattern I should follow?
There was a problem hiding this comment.
It's a good question. It boils down to testing for the side effects of the _prime_post_caches() function. In the case here, there would be no side effects. In particular, there should be no DB query. So you can add a callback for the query filter to increment whenever it is called. In the case here, since there is no class attribute, it would be expected that the query would never be made. However, if there was another IMG tag for an actual attachment then it would have a class attribute that would reference the attachment post ID, in which case the query filter should apply.
Note that you probably need to call flush_cache() right before calling auto_sizes_prime_attachment_caches() to ensure that none of the posts are cached so that the query will occur.
I think this test should have a data provider that has two entries:
- Test the case where there is no
classattribute and thus the expected query count is zero (0). - Test the case where the
IMGhas a comes from an actual attachment, and in this case the expected query count is one (1).
This comment was marked as spam.
This comment was marked as spam.
|
@sarthak-19 Could you please resolve the conflict and address the feedbacks? |
|
@mukeshpanchal27 Will do today 😅 |
There was a problem hiding this comment.
@sarthak-19 Thanks for the PR!
Overall, it looks solid to me. I’ve left a few suggestions. Also, could you please re-run the coverage report? I recently merged some accurate sizes PRs that might impact the results.
@westonruter When you have a moment, could you please take a look as well?
|
@sarthak-19 @westonruter I’m going to merge these PR. Since they don’t affect production code and only improve test coverage, any remaining items can be addressed in a separate follow-up PR. |
Summary
This is part of #1789:
@coversAnnotationscc : @westonruter