Improve Test Coverage for Speculation Rules Plugin #1845

sarthak-19 · 2025-02-04T08:11:54Z

Summary

This is part of #1789:

Ignore Coverage for Non-Critical Code Blocks
Add Missing @covers Annotations
Add Missing Tests

Before : 50.00% ⚠️	After: 92.00% ✅

cc : @westonruter

codecov · 2025-02-04T08:20:16Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.31%. Comparing base (d21b74d) to head (66b31f2).
Report is 10 commits behind head on trunk.

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

Flag	Coverage Δ
multisite	`68.31% <ø> (+1.67%)`	⬆️
single	`39.18% <ø> (+1.90%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

plugins/speculation-rules/tests/test-speculation-rules-settings.php

github-actions · 2025-02-05T13:44:54Z

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 props-bot label.

Unlinked Accounts

The 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.

Unlinked contributors: [email protected].

Co-authored-by: sarthak-19 <[email protected]>
Co-authored-by: westonruter <[email protected]>
Co-authored-by: felixarntz <[email protected]>

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

westonruter · 2025-02-05T19:36:49Z

plugins/speculation-rules/tests/test-speculation-rules-settings.php

+	}
+
+	/**
+	 * Function to test sanitize_setting() with various inputs.


Suggested change

* Function to test sanitize_setting() with various inputs.

* Function to test sanitize_setting() with various inputs.

*

* @covers ::plsr_sanitize_setting

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 :

With coverage annotation :

cc : @westonruter

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?

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

westonruter · 2025-02-05T19:40:52Z

In uninstall.php:

performance/plugins/speculation-rules/uninstall.php

Lines 9 to 12 in 68c6deb

    
           // If uninstall.php is not called by WordPress, bail. 
        
           if ( ! defined( 'WP_UNINSTALL_PLUGIN' ) ) { 
        
           	exit; 
        
           }

Can be updated to follow Optimization Detective to add the // @codeCoverageIgnore comment:

performance/plugins/optimization-detective/uninstall.php

Lines 9 to 12 in 68c6deb

    
           // If uninstall.php is not called by WordPress, bail. 
        
           if ( ! defined( 'WP_UNINSTALL_PLUGIN' ) ) { 
        
           	exit; // @codeCoverageIgnore 
        
           }

plugins/speculation-rules/tests/test-speculation-rules-settings.php

westonruter · 2025-02-05T21:33:41Z

plugins/speculation-rules/tests/test-speculation-rules-uninstall.php

@@ -29,8 +29,6 @@ private function require_uninstall(): void {

 	/**
 	 * Test option deletion.
-	 *
-	 * @covers ::plsr_delete_plugin_option


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.

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

After adding @covers ::plsr_delete_plugin_option

cc : @westonruter

westonruter

LGTM, although the issue with @covers is still mystifying.

Add missing test case for improving code coverage

3af6650

sarthak-19 changed the title ~~Add missing test case for improving code coverage~~ Improve Test Coverage for Speculation Rules Plugin Feb 4, 2025

sarthak-19 mentioned this pull request Feb 4, 2025

Improve code coverage through annotations #1789

Open

westonruter added this to the speculation-rules n.e.x.t milestone Feb 5, 2025

westonruter added [Type] Enhancement A suggestion for improvement of an existing feature [Plugin] Speculative Loading Issues for the Speculative Loading plugin (formerly Speculation Rules) skip changelog PRs that should not be mentioned in changelogs labels Feb 5, 2025

westonruter reviewed Feb 5, 2025

View reviewed changes

plugins/speculation-rules/tests/test-speculation-rules-settings.php Outdated Show resolved Hide resolved

westonruter reviewed Feb 5, 2025

View reviewed changes

plugins/speculation-rules/tests/test-speculation-rules-settings.php Outdated Show resolved Hide resolved

Sarthak Jaiswal and others added 2 commits February 5, 2025 19:13

Used get_echo() function to reduce code

04a08ac

Merge branch 'trunk' into add/code_coverage_speculation_rules

ddc02d0

sarthak-19 marked this pull request as ready for review February 5, 2025 13:44

sarthak-19 requested a review from felixarntz as a code owner February 5, 2025 13:44

sarthak-19 requested a review from westonruter February 5, 2025 13:44

westonruter reviewed Feb 5, 2025

View reviewed changes

Add @codeCoverageIgnore in uninstall.php

66eddb7

felixarntz reviewed Feb 5, 2025

View reviewed changes

westonruter reviewed Feb 5, 2025

View reviewed changes

Sarthak Jaiswal added 2 commits February 11, 2025 17:33

Code refactor and function rename

7519de2

Modified test function name

8fd7597

westonruter approved these changes Feb 11, 2025

View reviewed changes

Merge branch 'trunk' into add/code_coverage_speculation_rules

66b31f2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve Test Coverage for Speculation Rules Plugin #1845

Improve Test Coverage for Speculation Rules Plugin #1845

sarthak-19 commented Feb 4, 2025 •

edited

Loading

codecov bot commented Feb 4, 2025 •

edited

Loading

github-actions bot commented Feb 5, 2025 •

edited

Loading

westonruter Feb 5, 2025

sarthak-19 Feb 5, 2025

westonruter Feb 5, 2025

sarthak-19 Feb 11, 2025

westonruter commented Feb 5, 2025

westonruter Feb 5, 2025

sarthak-19 Feb 6, 2025

westonruter Feb 6, 2025

westonruter left a comment

Improve Test Coverage for Speculation Rules Plugin #1845

Are you sure you want to change the base?

Improve Test Coverage for Speculation Rules Plugin #1845

Conversation

sarthak-19 commented Feb 4, 2025 • edited Loading

Summary

codecov bot commented Feb 4, 2025 • edited Loading

Codecov Report

github-actions bot commented Feb 5, 2025 • edited Loading

Unlinked Accounts

westonruter Feb 5, 2025

Choose a reason for hiding this comment

sarthak-19 Feb 5, 2025

Choose a reason for hiding this comment

westonruter Feb 5, 2025

Choose a reason for hiding this comment

sarthak-19 Feb 11, 2025

Choose a reason for hiding this comment

westonruter commented Feb 5, 2025

westonruter Feb 5, 2025

Choose a reason for hiding this comment

sarthak-19 Feb 6, 2025

Choose a reason for hiding this comment

Without adding @covers ::plsr_delete_plugin_option

After adding @covers ::plsr_delete_plugin_option

westonruter Feb 6, 2025

Choose a reason for hiding this comment

westonruter left a comment

Choose a reason for hiding this comment

sarthak-19 commented Feb 4, 2025 •

edited

Loading

codecov bot commented Feb 4, 2025 •

edited

Loading

github-actions bot commented Feb 5, 2025 •

edited

Loading

Without adding `@covers ::plsr_delete_plugin_option`

After adding `@covers ::plsr_delete_plugin_option`