-
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 Speculation Rules Plugin #1845
base: trunk
Are you sure you want to change the base?
Improve Test Coverage for Speculation Rules Plugin #1845
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## trunk #1845 +/- ##
==========================================
+ Coverage 66.64% 68.31% +1.67%
==========================================
Files 88 87 -1
Lines 7015 6982 -33
==========================================
+ Hits 4675 4770 +95
+ Misses 2340 2212 -128
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
plugins/speculation-rules/tests/test-speculation-rules-settings.php
Outdated
Show resolved
Hide resolved
plugins/speculation-rules/tests/test-speculation-rules-settings.php
Outdated
Show resolved
Hide resolved
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: @sarthak.jaiswal@rtCamp.com. 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. |
plugins/speculation-rules/tests/test-speculation-rules-settings.php
Outdated
Show resolved
Hide resolved
plugins/speculation-rules/tests/test-speculation-rules-settings.php
Outdated
Show resolved
Hide resolved
} | ||
|
||
/** | ||
* Function to test sanitize_setting() with various inputs. |
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.
* Function to test sanitize_setting() with various inputs. | |
* Function to test sanitize_setting() with various inputs. | |
* | |
* @covers ::plsr_sanitize_setting |
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.
Initially I kept the @covers
annotation just like suggested changes however that's resulting in reduced code coverage.
If covers annotation is not there it automatically finds the function that is being tested.
However if we are specifying like this then coverage reduced. Also tested with ClassName::FunctionName
annotation.
- Without coverage annotation :
![image](https://private-user-images.githubusercontent.com/69689387/410170740-b60510d0-3248-4a58-8448-5458e86ef0ec.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NDI1ODksIm5iZiI6MTczOTY0MjI4OSwicGF0aCI6Ii82OTY4OTM4Ny80MTAxNzA3NDAtYjYwNTEwZDAtMzI0OC00YTU4LTg0NDgtNTQ1OGU4NmVmMGVjLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDE3NTgwOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTE3ZGYwZjgzNzlkMzIxN2E2Y2MyZDQ2YzkyZTZkODZhMGZiODNjM2I4Y2YwMGMwZWFhMWJhMWM4MTNkNGQwODMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.irpFH0jmZxZBUjGNhUIEW1-nJNfQTKMxgKuInY9K594)
![image](https://private-user-images.githubusercontent.com/69689387/410170619-926c064f-9246-4a50-97a0-a41deeca1146.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NDI1ODksIm5iZiI6MTczOTY0MjI4OSwicGF0aCI6Ii82OTY4OTM4Ny80MTAxNzA2MTktOTI2YzA2NGYtOTI0Ni00YTUwLTk3YTAtYTQxZGVlY2ExMTQ2LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDE3NTgwOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTRkZWY2NWY3YjlmNTk2ZWVmNWUwZDRlYjIxNTFlN2JlMzI2ZTRkMGI1MmZkYzUxMjIyMGZiZDIyZDk1ZmI3MWMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.pEuGgy42dzByk-CqDpS8hFOietSPS8ImAc65R18y7Ec)
- With coverage annotation :
![image](https://private-user-images.githubusercontent.com/69689387/410170176-efb4369b-d8ea-4ee0-92f3-600baa69fea7.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NDI1ODksIm5iZiI6MTczOTY0MjI4OSwicGF0aCI6Ii82OTY4OTM4Ny80MTAxNzAxNzYtZWZiNDM2OWItZDhlYS00ZWUwLTkyZjMtNjAwYmFhNjlmZWE3LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDE3NTgwOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTcwMDQ0YWQxNzU4N2Y5NzJhM2UzM2IyYTU4NjdiM2YyYTNjOWJmM2Q0YjgwMWJlMTVmOTIwODUwNDRkMWM0NDQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.A9FHW2_95w1OWWbrv6vR-U3O3PusNt8pIbHrRIVOL34)
![image](https://private-user-images.githubusercontent.com/69689387/410170373-5ef2c3b6-b2ab-430e-9500-cde5229d0b1c.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NDI1ODksIm5iZiI6MTczOTY0MjI4OSwicGF0aCI6Ii82OTY4OTM4Ny80MTAxNzAzNzMtNWVmMmMzYjYtYjJhYi00MzBlLTk1MDAtY2RlNTIyOWQwYjFjLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDE3NTgwOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWYyYzJiNjQ2Yjg1ODM2MGQ2NjdhMDg4NjlhZmVhY2Q3MGY5NzA1NDgyY2Q2NmExMTQwNWZlZjU3ZjZlYjEyMzQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.1hTtNxkB-gBt03qAofd-h3Rf8ULIYcsbPswYsIk_e8A)
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.
Nevertheless, I see that the test_plsr_sanitize_setting()
test is already covering plsr_sanitize_setting
. Therefore, do we even need this test_sanitize_setting
test? Should its test cases not be put into the data_plsr_sanitize_setting
data provider?
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.
-
Ideally it should be added in data_plsr_sanitize_setting data provider but Code coverage is unable to track these 3 lines that are being tested if passed via data provider.
-
However when adding these test in another function and calling
plsr_sanitize_setting()
then it seems to recognise the line being covered.
I renamed the function as per @felixarntz suggestion.
cc : @westonruter
In performance/plugins/speculation-rules/uninstall.php Lines 9 to 12 in 68c6deb
Can be updated to follow Optimization Detective to add the performance/plugins/optimization-detective/uninstall.php Lines 9 to 12 in 68c6deb
|
plugins/speculation-rules/tests/test-speculation-rules-settings.php
Outdated
Show resolved
Hide resolved
plugins/speculation-rules/tests/test-speculation-rules-settings.php
Outdated
Show resolved
Hide resolved
plugins/speculation-rules/tests/test-speculation-rules-settings.php
Outdated
Show resolved
Hide resolved
plugins/speculation-rules/tests/test-speculation-rules-settings.php
Outdated
Show resolved
Hide resolved
plugins/speculation-rules/tests/test-speculation-rules-settings.php
Outdated
Show resolved
Hide resolved
@@ -29,8 +29,6 @@ private function require_uninstall(): void { | |||
|
|||
/** | |||
* Test option deletion. | |||
* | |||
* @covers ::plsr_delete_plugin_option |
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.
Why remove this? The function is located in uninstall.php
so when that file is being required, then that function is indeed being loaded and executed.
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.
As mentioned here #1845 (comment)
for some functions adding @Covers annotation is reducing code coverage and exactly why it's happening is still not known. I thought it might be a case where some functions are not accessible (private or protected) but tested that as well, it's happening for public functions as well.
Without adding @covers ::plsr_delete_plugin_option
![image](https://private-user-images.githubusercontent.com/69689387/410362059-8eac2f66-a81d-4e34-a3a9-a5be8c696a2b.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NDI1ODksIm5iZiI6MTczOTY0MjI4OSwicGF0aCI6Ii82OTY4OTM4Ny80MTAzNjIwNTktOGVhYzJmNjYtYTgxZC00ZTM0LWEzYTktYTViZThjNjk2YTJiLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDE3NTgwOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTM2ZmQ4NGY0ZGUwNzBhZTE3MTZlZTViNzE5YTJkZDk1YTJiM2I3NjNjMTkzYmI1ZWE1MDYxYTVmYTJkOWFkMjAmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.aaEwphLomTIRfZeLVvq2ordB_glTzM_7TL6m5qWbT5Y)
![image](https://private-user-images.githubusercontent.com/69689387/410362208-7f7aa1f0-dc86-4511-aabd-30a6448d39fc.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NDI1ODksIm5iZiI6MTczOTY0MjI4OSwicGF0aCI6Ii82OTY4OTM4Ny80MTAzNjIyMDgtN2Y3YWExZjAtZGM4Ni00NTExLWFhYmQtMzBhNjQ0OGQzOWZjLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDE3NTgwOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWM3ODg3OWEyNGIwNzdlOWI5Y2JkODdlYTQ3ZjEyMWViNWU2MjBhNDg5NDI2NmY4ZWY4NmE3YTg5MzFlMWU5YjYmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.FiciTfhA7WkcDSCoPtteUikCKrAn5xf-j8AHG3NMWY4)
After adding @covers ::plsr_delete_plugin_option
![image](https://private-user-images.githubusercontent.com/69689387/410362545-c18252ab-258d-4485-a0d1-8b2093a3d4e3.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NDI1ODksIm5iZiI6MTczOTY0MjI4OSwicGF0aCI6Ii82OTY4OTM4Ny80MTAzNjI1NDUtYzE4MjUyYWItMjU4ZC00NDg1LWEwZDEtOGIyMDkzYTNkNGUzLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDE3NTgwOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWY3Mjg5ODE2NThiNDQ5NTIwNTBmMjQ4YmVkYWYyODdmZDY3YjA3MzY4NzI5NTVhNTg4MzA4Yjc3OTYyZGIwY2QmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.98koa4yZhpwD4D3ozzlbv-fffXgEnPB-zmDLJnXM568)
![image](https://private-user-images.githubusercontent.com/69689387/410362622-cf43317b-f5f7-413b-bdff-52f63d283ff7.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NDI1ODksIm5iZiI6MTczOTY0MjI4OSwicGF0aCI6Ii82OTY4OTM4Ny80MTAzNjI2MjItY2Y0MzMxN2ItZjVmNy00MTNiLWJkZmYtNTJmNjNkMjgzZmY3LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDE3NTgwOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWQ2NzY4NjM0MTI5ZWQ0ZGFiYTlkZmFhODVjN2I3YjUxZWY3NjMwMWI3YmM2NzJjMDAyMTM5Y2JiYWIzMTIxZGMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.qO4yJyuw-WaMzP5HJQ7ivPxqV1mLlBOcuAyHpeZI4QM)
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!
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.
LGTM, although the issue with @covers
is still mystifying.
Summary
This is part of #1789:
@covers
Annotationscc : @westonruter