Outbound HTTP requests fail with KernelTestBase

mh

Just found this. Tried testing Commerce integration with a payment gateway's API in kernel test to find it exploded.

I tried unsetting test.http_client.middleware definition from the container in ::setUp but that failed. I think we need to add a check next to drupal_valid_test_ua that sees if DRUPAL_TEST_IN_CHILD_SITE is defined.

1) Drupal\Tests\commerce_authnet\Kernel\PaymentMethodTest::testRemotePaymentMethods
Use of undefined constant DRUPAL_TEST_IN_CHILD_SITE - assumed 'DRUPAL_TEST_IN_CHILD_SITE'

/Users/mglaman/Drupal/sites/commerce2x/web/core/includes/bootstrap.inc:666
/Users/mglaman/Drupal/sites/commerce2x/web/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:29
/Users/mglaman/Drupal/sites/commerce2x/vendor/guzzlehttp/guzzle/src/PrepareBodyMiddleware.php:72
/Users/mglaman/Drupal/sites/commerce2x/vendor/guzzlehttp/guzzle/src/Middleware.php:30
/Users/mglaman/Drupal/sites/commerce2x/vendor/guzzlehttp/guzzle/src/RedirectMiddleware.php:68
/Users/mglaman/Drupal/sites/commerce2x/vendor/guzzlehttp/guzzle/src/Middleware.php:59
/Users/mglaman/Drupal/sites/commerce2x/vendor/guzzlehttp/guzzle/src/HandlerStack.php:67
/Users/mglaman/Drupal/sites/commerce2x/vendor/guzzlehttp/guzzle/src/Client.php:275
/Users/mglaman/Drupal/sites/commerce2x/vendor/guzzlehttp/guzzle/src/Client.php:123
/Users/mglaman/Drupal/sites/commerce2x/vendor/guzzlehttp/guzzle/src/Client.php:129
/Users/mglaman/Drupal/sites/commerce2x/vendor/guzzlehttp/guzzle/src/Client.php:87
/Users/mglaman/Drupal/sites/commerce2x/vendor/commerceguys/authnet/src/Request/BaseRequest.php:82
/Users/mglaman/Drupal/sites/commerce2x/vendor/commerceguys/authnet/src/BaseApiRequest.php:67
/Users/mglaman/Drupal/sites/commerce2x/web/modules/contrib/commerce_authnet/src/Plugin/Commerce/PaymentGateway/AuthorizeNet.php:415
/Users/mglaman/Drupal/sites/commerce2x/web/modules/contrib/commerce_authnet/src/Plugin/Commerce/PaymentGateway/AuthorizeNet.php:339
/Users/mglaman/Drupal/sites/commerce2x/web/modules/contrib/commerce_authnet/tests/src/Kernel/PaymentMethodTest.php:65

This patch is allowing me to send an HTTP request to the service. This is a big blocker with using kernel tests to test integration with third party SDKs.

I have the same issue, but I don't know if Kernel tests are supposed to be used to test third-party APIs for example.
Maybe exists other way of doing this.

Interesting issue!

Forgot, here's my workaround for now. Implement ServiceModifierInterface, then

  public function alter(ContainerBuilder $container) {
    $container->removeDefinition('test.http_client.middleware');
  }

I believe #10 is pretty good, we just need to ensure that the constants exists, by probably not having the constant defined in the module.

Just adding my experience with this issue. I tried defining the constant, for kicks, in my test setup, but that just led to this:

Drupal\Tests\mymodule\Kernel\MyTest::testMethod
file_put_contents(/vagrant/web/sites/simpletest/90225451/.htkey): failed to
open stream: No such file or directory

I ended up changing my tests that made HTTP requests to extend the BrowserTestBase instead.

dawehner should we try to handle the constant definition, (I had same issue reported in #18) or just have base kernel test remove the service definition?

Hey there, just jumping in to mention I've experienced this problem too when trying to move convert FileDenormalizeTest (Simpletest) from the "hal" module into a KernelTest.

Not a problem for that one, as it can be simply extend BrowserTestBase instead and keep working as a functional test, but thought I'd mention it!

Patch at #10 works fine for me.

Patch #10 also works for me.

Had this issue yesterday. I lost 4 hours trying to fix the test in different ways. I defined constant DRUPAL_TEST_IN_CHILD_SITE and ended up with

Drupal\Tests\mymodule\Kernel\MyTest::testMethod
file_put_contents(/webs/drupal8/sites/simpletest/XXXXXXX/.htkey): failed to
open stream: No such file or directory

I converted to BrowserTest and it still didn't work because BrowserTestBase doesn't have drupalPost(), only drupalPostForm(), and I needed to post JSON.

Eventually, I had to give up and revert to WebTestBase - bummer.

Can patch in #10 be committed?

#16 worked for me actually!
Thanks @mglman

I hit the same issue today.

Queued #10 for retesting.

Test failed

Got this error today while implementing kernel tests for webpack. The workaround from #16 solved it.

Un-needed re-roll

never mind pointless patch re-roll.

Hit the same problem. Temporary workaround in #16 works for me!
Thanks @mglman

++ workaround in #16 works for me too!

#16 works for me.

Rerolled against 8.8.x.

#37 Worked for me.

Works for me, and patch looks good.

Uploading a test only patch (just copied from 37) to prove the bug. Will mark as RTBC if that fails.

🎊️

That was just the test only patch

+++ b/core/lib/Drupal/Core/CoreServiceProvider.php
@@ -180,7 +180,7 @@ public function alter(ContainerBuilder $container) {
     // Do nothing if we are not in a test environment.
-    if (!drupal_valid_test_ua()) {
+    if (!defined('DRUPAL_TEST_IN_CHILD_SITE') || !drupal_valid_test_ua()) {
       return;
     }

Unfortunately the comment makes no sense. We're do nothing and we're in a test environment :)

The reason we have this problem is that under kernel tests \Drupal\Core\DrupalKernel::bootEnvironment() is never called and therefore DRUPAL_TEST_IN_CHILD_SITE is not defined. I spent some time considering if we should define DRUPAL_TEST_IN_CHILD_SITE in \Drupal\KernelTests\KernelTestBase::bootEnvironment() or \Drupal\KernelTests\KernelTestBase::bootKernel() and what that would look like. However I don't think that's right.

So I think we have 2 options here. The simple one is document this properly... i.e add something like

    // The test middleware is not required for kernel tests as there is no child site. DRUPAL_TEST_IN_CHILD_SITE is not defined is this case.
    if (!defined('DRUPAL_TEST_IN_CHILD_SITE')) {
       return;
     }

The comment probably can be improved but I think being explicit is useful.

The other option is to somehow check the Kernel environment in \Drupal\Core\CoreServiceProvider::registerTest() because in kernel tests we set it to 'testing' whereas in other tests its 'prod'. That said this solution also feels more fragile than the DRUPAL_TEST_IN_CHILD_SITE solution so let's do that - but with a more explicit comment.

Any updates on this?

I am using the workaround from #16

Ran into this problem in #3041885: Display relevant Security Advisories data for Drupal when we just moved this functionality to the system module.

This bug makes \Drupal\KernelTests\Core\File\FileSystemRequirementsTest fail because we fetch the security advisories feed in system_requirements

Updated #37 with @alexpott's suggestion in #44 I agree it is better without it's own comment. I think the comment suggested is fine.

The test only patch in #39 can still work as the test only patch since the test has not changed. Will re-queue it.

Should we fix this first in 9.2.x now?

1. +++ b/core/lib/Drupal/Core/CoreServiceProvider.php
   @@ -183,6 +183,11 @@ protected function registerTest(ContainerBuilder $container) {
   +    // site. DRUPAL_TEST_IN_CHILD_SITE is not defined is this case.
   

   That should be "...in this case".

2. +++ b/core/tests/Drupal/KernelTests/KernelTestBaseTest.php
   @@ -164,6 +165,21 @@ public function testSubsequentContainerIsolation() {
   +      // Ignore any HTTP errors,
   

   That should end with a period, not a comma.

Both of these are fixable on commit, so I'm sending this right to RTBC on the assumption that the tests will come back green. Otherwise, this patch makes sense to me. I'm also re-titling the issue because I don't think we have the "old" KernelTestBase anymore anyway.

I would guess this will land in 9.2.x first and be backported, so I'm also changing the target version and queuing the latest patch up against 9.2.x.

Committed 51e0c38 and pushed to 9.2.x. Thanks!

Will backport once bugfix releases are out and we're unfrozen.

diff --git a/core/lib/Drupal/Core/CoreServiceProvider.php b/core/lib/Drupal/Core/CoreServiceProvider.php
index fa989906b1..92e9594c34 100644
--- a/core/lib/Drupal/Core/CoreServiceProvider.php
+++ b/core/lib/Drupal/Core/CoreServiceProvider.php
@@ -132,7 +132,7 @@ protected function registerTest(ContainerBuilder $container) {
       return;
     }
     // The test middleware is not required for kernel tests as there is no child
-    // site. DRUPAL_TEST_IN_CHILD_SITE is not defined is this case.
+    // site. DRUPAL_TEST_IN_CHILD_SITE is not defined in this case.
     if (!defined('DRUPAL_TEST_IN_CHILD_SITE')) {
       return;
     }
diff --git a/core/tests/Drupal/KernelTests/KernelTestBaseTest.php b/core/tests/Drupal/KernelTests/KernelTestBaseTest.php
index 0f1d379e74..4775bf53fa 100644
--- a/core/tests/Drupal/KernelTests/KernelTestBaseTest.php
+++ b/core/tests/Drupal/KernelTests/KernelTestBaseTest.php
@@ -176,7 +176,7 @@ public function testOutboundHttpRequest() {
       $this->container->get('http_client')->get('http://example.com');
     }
     catch (GuzzleException $e) {
-      // Ignore any HTTP errors,
+      // Ignore any HTTP errors.
     }
   }
 

Fixed on commit.

I'm glad that this finally fixed, but testOutboundHttpRequest() does not perform any actual assertion. Isn't that an antipattern that PHPunit also warns when it runs the test?

@mxr576 good point - can you create a follow-up issue to add an assertion. It doesn't fail because we have an assertion in \Drupal\KernelTests\KernelTestBaseTest::tearDown()

Added KernelTestBaseTest::testOutboundHttpRequest() does not perform any assertion

Backported to 9.1.x.

Whilst debugging a test, @tedbow and I found a flaw in this logic: it doesn't account for DRUPAL_TEST_IN_CHILD_SITE being explicitly defined as FALSE. If the constant exists and is FALSE, then it registers the test middleware anyway, and things can get weird.

We only have been encountering this while working on #3041885: Display relevant Security Advisories data for Drupal, which adds code to the System module that uses the http_client service to make outbound HTTP requests. Adding this in System, which is a very foundational module that's close the bootstrapping and installation process, surfaces these sorts of things.

So I think we'll need to either reopen this issue, or open a follow-up, to handle the case where DRUPAL_TEST_IN_CHILD_SITE is FALSE.

Got some help from @mglaman on the problem in #57. Turns out we were able to solve it a different way
https://git.drupalcode.org/issue/drupal-3041885/-/compare/991f955548ef61...

I think in our case DRUPAL_TEST_IN_CHILD_SITE was actually to be TRUE anyways. So probably makes sense to open up a follow up for checking DRUPAL_TEST_IN_CHILD_SITE is FALSE but that won't have solved our problem

It looks like this should be able to be backported to Drupal 8.9 as well since it's a bug fix that touches no APIs. I've run into this several times with making HTTP requests in tests.

Just ran into this as well (was writing a custom kernel test for 8.9.x project).

AFAIK, only Major bugs are eligible to be backported to 8.9.x. I'm gonna bump the priority because it's terrible for developer experience and would be great to have fixed in the next release.

I've applied a patch from commit to 9.1.x and it did the trick for me, therefore moving to RTBC.
Here is a link for quick access: https://git.drupalcode.org/project/drupal/commit/ba2c6fb.patch

Please do not ask the testbot to try again until #3207086: [HEAD BROKEN] Consistent failure in MonthDatePluginTest is fixed.

#3207086: [HEAD BROKEN] Consistent failure in MonthDatePluginTest is fixed.

The fix for #3207086: [HEAD BROKEN] Consistent failure in MonthDatePluginTest still needs to be backported to the 8.9.x branch, so currently tests are still failing.

#3207086: [HEAD BROKEN] Consistent failure in MonthDatePluginTest has now been backported to branch 8.9.x.
Ordered a retest on the latest patch, which should come back green.

Backported to 8.9.x, thanks!