test: Add unit testing for php classes by sabbir1991 · Pull Request #103 · rtCamp/onemedia

sabbir1991 · 2026-04-28T17:05:34Z

What

In this PR, we have covered the maximum number of PHP files test coverage.

Testing Instructions

Run the commands below to check the local.

npm run test:php -- tests/phpunit/Unit

Checklist

I have read the Contribution Guidelines.
I have read the Development Guidelines.
My code is tested to the best of my abilities.
My code passes all lints (ESLint etc.).
My code has detailed inline documentation.
I have updated the project documentation as needed.
I have added a changeset for this PR using npm run changeset.

Copilot

Pull request overview

Adds PHPUnit unit tests to increase coverage across the OneMedia PHP codebase (settings, REST controllers, media sharing, media library, and core bootstrap/helpers).

Changes:

Introduces new unit test suites for Settings/Admin, REST controllers, MediaSharing, MediaLibrary, and Core modules.
Adds coverage for option sanitization/encryption, hook registration, REST validation branches, and media-sync related helpers.
Adds tests for plugin bootstrap utilities (Autoloader, Singleton trait, Main, Assets, Rest).

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/phpunit/Unit/Modules/Settings/SettingsTest.php	Exercises Settings option registration, sanitizers, and helper getters/setters.
tests/phpunit/Unit/Modules/Settings/AdminTest.php	Tests settings admin hook registration and admin UI helpers.
tests/phpunit/Unit/Modules/Rest/MediaSharingControllerTest.php	Covers Media_Sharing_Controller validation branches and mocked HTTP paths.
tests/phpunit/Unit/Modules/Rest/BasicOptionsControllerTest.php	Tests basic options REST endpoints and validation.
tests/phpunit/Unit/Modules/Rest/AbstractRestControllerTest.php	Tests shared REST permission logic for same-origin and token auth.
tests/phpunit/Unit/Modules/MediaSharing/UserInterfaceTest.php	Verifies media UI hook wiring and rendering behavior.
tests/phpunit/Unit/Modules/MediaSharing/MediaReplacementTest.php	Covers image replacement behavior in post content/meta.
tests/phpunit/Unit/Modules/MediaSharing/MediaProtectionTest.php	Tests consumer capability restrictions and transient notice behavior.
tests/phpunit/Unit/Modules/MediaSharing/MediaActionsTest.php	Covers sync actions hooks, sanitization, and version history behavior.
tests/phpunit/Unit/Modules/MediaSharing/AttachmentTest.php	Covers attachment meta helpers and health-check behaviors with HTTP mocking.
tests/phpunit/Unit/Modules/MediaSharing/AdminTest.php	Tests media sharing admin submenu/screen output and script enqueue bails.
tests/phpunit/Unit/Modules/MediaLibrary/ConsumerAdminTest.php	Tests consumer-site restrictions and notices for synced attachments.
tests/phpunit/Unit/Modules/MediaLibrary/AdminTest.php	Tests media library filters and upload page behavior.
tests/phpunit/Unit/Modules/Core/RestTest.php	Covers CORS header allowlist behavior.
tests/phpunit/Unit/Modules/Core/AssetsTest.php	Tests asset registration, localized data caching, and defer-tag filtering.
tests/phpunit/Unit/MainTest.php	Tests Main singleton bootstrapping and hook wiring.
tests/phpunit/Unit/Contracts/Traits/SingletonTest.php	Tests Singleton trait instance/clone/wakeup protections.
tests/phpunit/Unit/AutoloaderTest.php	Tests autoloader idempotence and missing-autoload file branch.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov-commenter · 2026-04-28T17:10:31Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.85%. Comparing base (4746dea) to head (d16b8b5).

Additional details and impacted files

@@             Coverage Diff              @@
##              main     #103       +/-   ##
============================================
+ Coverage     1.89%   96.85%   +94.95%     
+ Complexity     666      654       -12     
============================================
  Files           22       22               
  Lines         2748     2667       -81     
============================================
+ Hits            52     2583     +2531     
+ Misses        2696       84     -2612

Flag	Coverage Δ
unit	`96.85% <100.00%> (+94.95%)`	⬆️

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.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

justlevine

Great first pass!

Most of these comments boil down to suggesting we just test the outcomes and edge cases, instead of the specific implementation details. This will keep our tests robust, and more importantly mean we don't need to rewrite most of it when we refactor some of these class internals.

justlevine · 2026-04-29T13:45:23Z

+	public function test_assets_class_instantiation(): void {
+		$assets = new Assets();
+
+		$assets->register_hooks();


We want to trigger all the lifecycle actions we're not directly testing

Suggested change

$assets->register_hooks();

$assets->register_hooks();

$assets->register_assets();

$assets->enqueue_scripts();

There's still a bunch of these you missed...

Maybe it wasnt clear from the phone call, but this was literally all you needed to do. I'm seeing all these mocks and directly calling and checking if assets are registered and random stuff like that, but as already noted multiple times we don't need to test that wordpress works. and we don't need to retest the functions that we're testing else where. Literally just a class instantiation test where you call the functions directly and simply ensure they don't throw a fatal error...

justlevine · 2026-05-12T21:52:19Z

+		// @codeCoverageIgnoreStart
 		exit;
+		// @codeCoverageIgnoreEnd
 	}


Personally, I'd rather know what's not covered then ignore it, but 🤷

justlevine · 2026-05-12T21:53:58Z

+
+	/**
+	 * Read a filtered input value.
+	 *
+	 * @param 0|1|2|4|5             $type     Input type.
+	 * @param string                $var_name Input name.
+	 * @param int                   $filter   Filter id.
+	 * @param array<int, mixed>|int $options  Filter options.
+	 *
+	 * @codeCoverageIgnore
+	 */
+	protected function get_filtered_input( int $type, string $var_name, int $filter = FILTER_DEFAULT, array|int $options = 0 ): mixed {
+		return Utils::get_filtered_input( $type, $var_name, $filter, $options );
+	}


Are we extending this class? Why not just use Utils::get_filtered_input() directly? If we do need it, We definitely shouldn't be @codeCoverageIngoreing it.

justlevine · 2026-05-12T21:55:10Z

+	 * @param int                   $filter   Filter id.
+	 * @param array<int, mixed>|int $options  Filter options.
+	 */
+	protected function get_filtered_input( int $type, string $var_name, int $filter = FILTER_DEFAULT, array|int $options = 0 ): mixed {


#103 (comment)

justlevine · 2026-05-12T21:57:07Z

+		// phpcs:ignore Squiz.Commenting.InlineComment.InvalidEndChar -- PHPUnit coverage marker.
+		// @codeCoverageIgnoreStart


This should have been your sign that throwing in all of these @codeCoverageIgnore*s were a bad idea...

justlevine · 2026-05-12T21:59:58Z

 use OneMedia\Modules\Rest\Abstract_REST_Controller;
 use OneMedia\Modules\Settings\Settings;
 use OneMedia\Utils;
+use WP_Error; // phpcs:ignore SlevomatCodingStandard.Namespaces.UnusedUses.UnusedUse -- Required for editor resolution of \WP_Error annotations.


We're using FQCN for basic WordPress classes (\WP_Error). You can just remove this like the linters tell you... That's why the rule exists.

Suggested change

use WP_Error; // phpcs:ignore SlevomatCodingStandard.Namespaces.UnusedUses.UnusedUse -- Required for editor resolution of \WP_Error annotations.

Same with WP_Post, we use it once in this entire file, as a param, might as well \WP_Post.

justlevine · 2026-05-12T22:19:21Z

I'm not through code review for this PR but im struggling to believe that we need to_any_ of these *-shims. These are integration tests.....

Added generic file unit testing

8b438ea

Copilot AI review requested due to automatic review settings April 28, 2026 17:05

Copilot started reviewing on behalf of sabbir1991 April 28, 2026 17:06 View session

sabbir1991 requested a review from justlevine April 28, 2026 17:06

sabbir1991 changed the title ~~Added unit testing for php classes~~ chore: Added unit testing for php classes Apr 28, 2026

Copilot AI reviewed Apr 28, 2026

View reviewed changes

Added gitkeep in test output folder

1d44694

justlevine requested changes Apr 28, 2026

View reviewed changes

Fixed few PR review issues

1cddc59

sabbir1991 requested a review from justlevine April 29, 2026 10:50