Youtube oEmbed Test is not loading scripts

[SirLouen]

After breaking my head for half an hour because the tests were not passing, I started debugging to find that a random test in the restapi group is failing.

Personally I thought that the full restapi tests were passing in the GH CI, but its seems not.

The particular test is: Test_oEmbed_Controller::test_proxy_with_classic_embed_provider

Easily run with: npm run test:php -- --group 45447

There was 1 error:

1) Test_oEmbed_Controller::test_proxy_with_classic_embed_provider
Attempt to read property "queue" on null

/var/www/src/wp-includes/class-wp-oembed-controller.php:211
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1292
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1125
/var/www/tests/phpunit/includes/spy-rest-server.php:71
/var/www/tests/phpunit/tests/oembed/controller.php:613

ERRORS!
Tests: 1, Assertions: 2, Errors: 1.

The problem is that wp_scripts() are not being called at any point in the tests, ​so it fails here with an empty array.

Patch first introduced in [48135], I can't really see when and how the test was broken.

After spending a ton of time rolling back into WP 5.5 wordpress-develop 8368cc2b4410fe2834dd7e9012fe6d4f4919a721 testing suite (but with current trunk code) to check tests with my eyes this is what I found:

There was 1 error:

1) Test_oEmbed_Controller::test_proxy_with_classic_embed_provider
Trying to get property 'queue' of non-object

/var/www/src/wp-includes/class-wp-oembed-controller.php:211
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1292
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1125
/var/www/tests/phpunit/includes/spy-rest-server.php:71
/var/www/tests/phpunit/tests/oembed/controller.php:624
phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:60

ERRORS!
Tests: 1, Assertions: 2, Errors: 1.

Exactly the same results but different lines, because as I say I was running 5.5 test suite on trunk, this is why core files have the same lines.

Since this test has never run on the GH CI and it was failing since day one, we could say that it safe to be patched (for sanity purposes, when running restapi group). I have not run the test straight on WP 5.5 but its safe to say that the test suite has never been complete to cover this case.

Conclusion, we simply need to add wp_scripts() somewhere in the code (maybe in setUp()) so the global wp_scripts is initialized in order to run successfully though

Trac ticket: https://core.trac.wordpress.org/ticket/63590

[abcd95]

Test Report

Description

This report validates that the patch works as expected.

Patch tested: ​PR 9008

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.25
• Server: nginx/1.27.2
• Database: mysqli (Server: 8.4.4 / Client: mysqlnd 8.2.25)
• Browser: Chrome 137.0.0.0
• OS: macOS
• Theme: Twenty Twenty-Five 1.2

Actual Results

✅ Issue resolved with patch.

Additional Notes

Although the patch works, I think we can do better. I think we can initialize the script handler while setting up the tests.

Thanks @SirLouen for the PR, while the current implementation works fine, I think we can do better -

tests/phpunit/tests/oembed/controller.php

@@ -47 +47 @@
         public function set_up() {
                 parent::set_up();
 
+                // Initialize the scripts handler.
+                wp_scripts();
+
                 /** @var WP_REST_Server $wp_rest_server */
                 global $wp_rest_server;
                 $wp_rest_server = new Spy_REST_Server();

This is a cleaner, more robust solution because it follows WordPress testing best practices for environment setup. It clearly states the intent and correctly isolates the setup logic from the test's execution logic. Also, any following tests wouldn't have to initialize the scripts like test_proxy_with_classic_embed_provider currently does manually.

[SirLouen]

Hey @abcd95

First, thanks for the testing report

I was thinking about this originally. But I'm not 100% confident that this should be only applied to that class. Or this is applied globally, or this is applied test level, targeting utils or wp-embed.

But if no other test in that class is benefiting from this, or any test could benefit from this, why not any overall test would benefit from this? Why not set this on parent::set_up()? Or why not in wpSetUpBeforeClass just like self::touch( ABSPATH . WPINC . '/js/wp-embed.js' );, again, just for utils or wp-embed instead of a script-wide loading?

In fact ​its not uncommon to laser load the scripts in specific tests or ​here

My advice: When suggesting things regarding unit testing, I would first check examples all over the place to make an argument before throwing just the idea of improvement.

[SirLouen]

According with today's bug scrub, commenting with @johnbillion we should try to find out why this test is not actually running in GHA. Making it run in GHA (or finding the cause), could possibly mean that CI tests would fail, and it could be particularly worthy to push this forward.

Meanwhile, as explained, running single tests or oembed group due to this test, tests are failing and this problematic, specially while testing oembed related topics.

I don't really know which tag to put because this is a weird scenario, so I will put needs-test-info