Skip to content
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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: trunk
Choose a base branch
from
+142 −5
Open

Improve Test Coverage for Speculation Rules Plugin #1845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3af6650
Add missing test case for improving code coverage
Feb 4, 2025
04a08ac
Used get_echo() function to reduce code
Feb 5, 2025
ddc02d0
Merge branch 'trunk' into add/code_coverage_speculation_rules
sarthak-19 Feb 5, 2025
66eddb7
Add @codeCoverageIgnore in uninstall.php
Feb 5, 2025
7519de2
Code refactor and function rename
Feb 11, 2025
8fd7597
Modified test function name
Feb 11, 2025
66b31f2
Merge branch 'trunk' into add/code_coverage_speculation_rules
sarthak-19 Feb 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion plugins/speculation-rules/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
if ( ! defined( 'ABSPATH' ) ) {
exit; // Exit if accessed directly.
}
// @codeCoverageIgnoreEnd

(
/**
Expand Down Expand Up @@ -83,3 +82,4 @@ static function ( string $version ): void {
require_once __DIR__ . '/settings.php';
}
);
// @codeCoverageIgnoreEnd
3 changes: 2 additions & 1 deletion plugins/speculation-rules/settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,8 @@ function plsr_render_settings_field( array $args ): void {
$choices = plsr_get_eagerness_labels();
break;
default:
return; // Invalid (and this case should never occur).
// Invalid (and this case should never occur).
return; // @codeCoverageIgnore
}

$value = $option[ $args['field'] ];
Expand Down
138 changes: 138 additions & 0 deletions plugins/speculation-rules/tests/test-speculation-rules-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ class Test_Speculation_Rules_Settings extends WP_UnitTestCase {

/**
* @covers ::plsr_register_setting
* @covers ::plsr_get_mode_labels
* @covers ::plsr_get_eagerness_labels
* @covers ::plsr_get_setting_default
*/
public function test_plsr_register_setting(): void {
unregister_setting( 'reading', 'plsr_speculation_rules' );
Expand All @@ -18,6 +21,14 @@ public function test_plsr_register_setting(): void {
plsr_register_setting();
$settings = get_registered_settings();
$this->assertArrayHasKey( 'plsr_speculation_rules', $settings );

$settings = plsr_get_setting_default();
$this->assertArrayHasKey( 'mode', $settings );
$this->assertArrayHasKey( 'eagerness', $settings );

// Test default settings applied correctly.
$default_settings = plsr_get_setting_default();
$this->assertEquals( $default_settings, get_option( 'plsr_speculation_rules' ) );
}

/**
Expand Down Expand Up @@ -122,4 +133,131 @@ public function test_plsr_add_settings_action_link(): void {
plsr_add_settings_action_link( $default_action_links )
);
}

/**
* @covers ::plsr_get_stored_setting_value
*/
public function test_get_stored_setting_value(): void {
update_option(
'plsr_speculation_rules',
array(
'mode' => 'prefetch',
'eagerness' => 'moderate',
)
);
$settings = plsr_get_stored_setting_value();
$this->assertEquals(
array(
'mode' => 'prefetch',
'eagerness' => 'moderate',
),
$settings
);

// Test default when no option is set.
delete_option( 'plsr_speculation_rules' );
$settings = plsr_get_stored_setting_value();
$this->assertEquals( plsr_get_setting_default(), $settings );
}

/**
* Function to test sanitize_setting() with various inputs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Function to test sanitize_setting() with various inputs.
* Function to test sanitize_setting() with various inputs.
*
* @covers ::plsr_sanitize_setting

Copy link
Contributor Author

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 image
  • With coverage annotation :
image image

cc : @westonruter

Copy link
Member

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?

Copy link
Contributor Author

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

  • However when adding these test in another function and calling plsr_sanitize_setting() then it seems to recognise the line being covered.
    image

I renamed the function as per @felixarntz suggestion.

cc : @westonruter

*/
public function test_plsr_sanitize_setting_with_invalid_inputs(): void {

$input = array(
'mode' => 'invalid_mode',
'eagerness' => 'conservative',
);
$sanitized = plsr_sanitize_setting( $input );
$this->assertEquals( 'prerender', $sanitized['mode'] );

$input = array(
'mode' => 'prefetch',
'eagerness' => 'invalid_eagerness',
);
$sanitized = plsr_sanitize_setting( $input );
$this->assertEquals( 'moderate', $sanitized['eagerness'] );

$input = 'invalid_input';
$sanitized = plsr_sanitize_setting( $input );
$this->assertEquals( plsr_get_setting_default(), $sanitized );
}

/**
* @covers ::plsr_add_setting_ui
*/
public function test_plsr_add_setting_ui(): void {
do_action( 'load-options-reading.php' );// phpcs:ignore WordPress.NamingConventions.ValidHookName.UseUnderscores

// Check if the settings section has been added.
global $wp_settings_sections;
$this->assertArrayHasKey( 'reading', $wp_settings_sections );
$this->assertArrayHasKey( 'plsr_speculation_rules', $wp_settings_sections['reading'] );

// Check the output of the callback function for the section.
$output = get_echo( $wp_settings_sections['reading']['plsr_speculation_rules']['callback'] );
$this->assertStringContainsString( 'This section allows you to control how URLs that your users navigate to are speculatively loaded to improve performance.', $output );
}

/**
* Data provider for testing plsr_render_settings_field.
*
* @return array<string, array<mixed>> Data for testing settings fields.
*/
public function data_provider_to_test_render_settings_field(): array {
return array(
'mode' => array(
array(
'field' => 'mode',
'title' => 'Speculation Mode',
'description' => 'Prerendering will lead to faster load times than prefetching.',
),
array(
'mode' => 'prefetch',
'eagerness' => 'moderate',
),
'name="plsr_speculation_rules[mode]"',
'value="prefetch"',
'Prerendering will lead to faster load times than prefetching.',
),
'eagerness' => array(
array(
'field' => 'eagerness',
'title' => 'Eagerness',
'description' => 'The eagerness setting defines the heuristics based on which the loading is triggered.',
),
array(
'mode' => 'prefetch',
'eagerness' => 'moderate',
),
'name="plsr_speculation_rules[eagerness]"',
'value="moderate"',
'The eagerness setting defines the heuristics based on which the loading is triggered.',
),
);
}

/**
* Test rendering of settings fields using data provider.
*
* @dataProvider data_provider_to_test_render_settings_field
* @param array<mixed> $args Arguments for the settings field.
* @param array<mixed> $stored_settings Stored settings values.
* @param string $name_check HTML name attribute check.
* @param string $value_check HTML value attribute check.
* @param string $description_check Description check.
*/
public function test_plsr_render_settings_field( array $args, array $stored_settings, string $name_check, string $value_check, string $description_check ): void {
// Simulate getting stored settings.
update_option( 'plsr_speculation_rules', $stored_settings );

// Capture the output of the settings field rendering.
$output = get_echo( 'plsr_render_settings_field', array( $args ) );

// Check for the presence of form elements.
$this->assertStringContainsString( $name_check, $output );
$this->assertStringContainsString( $value_check, $output );
$this->assertStringContainsString( $description_check, $output );
}
}
2 changes: 0 additions & 2 deletions plugins/speculation-rules/tests/test-speculation-rules-uninstall.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ private function require_uninstall(): void {

/**
* Test option deletion.
*
* @covers ::plsr_delete_plugin_option
Copy link
Member

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.

Copy link
Contributor Author

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 image

After adding @covers ::plsr_delete_plugin_option

image image

cc : @westonruter

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange!

*/
public function test_delete_plugin_option(): void {
unregister_setting( 'reading', 'plsr_speculation_rules' );
Expand Down
2 changes: 1 addition & 1 deletion plugins/speculation-rules/uninstall.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

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

// For a multisite, delete the option for all sites (however limited to 100 sites to avoid memory limit or timeout problems in large scale networks).
Expand Down