Problem/Motivation

Drupal\Tests\media\Functional\ProviderRepositoryTest::testEmptyProviderList() has the following code:

    $response = $this->prophesize('\GuzzleHttp\Psr7\Response');
    $response->getBody()->willReturn($content);

    $client = $this->createMock('\GuzzleHttp\Client');
    $client->method('request')->withAnyParameters()->willReturn($response->reveal());
    $this->container->set('http_client', $client);

    $this->expectException(ProviderException::class);
    $this->expectExceptionMessage('Remote oEmbed providers database returned invalid or empty list.');
    $this->container->get('media.oembed.provider_repository')->getAll();

The data provider for the method tests two values of $content, an empty array and an empty string.

This is problematic, because \GuzzleHttp\Psr7\Response::getBody() is documented to only return either StreamInterface or NULL. It cannot return an empty string or an empty array, so the test is not actually testing the intended case of an empty JSON response. It's instead testing impossible return values for the response body getter.

This issue is major because it blocks #3220220: Update guzzlehttp/psr7 to 2.1.0, as the 2.x branch of the Guzzle psr7 package adds an actual return typehint, so the test will start failing explicitly with a TypeError instead of the intended exception.

Proposed resolution

Change the test to actually test a response stream containing empty JSON values.

Remaining tasks

Rebase #3220220: Update guzzlehttp/psr7 to 2.1.0 on top of this once this lands.

Issue fork drupal-3220450

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm created an issue. See original summary.

xjm’s picture

Title: OEmbed ProviderRepositoryTest::testEmptyProviderList() does not test what it thinks itʻs testing » OEmbed ProviderRepositoryTest::testEmptyProviderList() does not test what it thinks it's testing

xjm credited phenaproxima.

xjm’s picture

Status: Active » Needs review

xjm’s picture

Issue summary: View changes
phenaproxima’s picture

Title: OEmbed ProviderRepositoryTest::testEmptyProviderList() does not test what it thinks it's testing » OEmbed ProviderRepositoryTest::testEmptyProviderList() does not interact with Guzzle's API correctly

Re-titling for accuracy. The test itself is accurately testing what it should; it's just not properly using the Guzzle API.

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

Also, the test will pass (it did in the other issue). No reason to hold up RTBC!

xjm’s picture

  • larowlan committed 0127852 on 9.2.x
    Issue #3220450 by xjm, phenaproxima: OEmbed ProviderRepositoryTest::...
  • larowlan committed 2c57cbe on 9.3.x
    Issue #3220450 by xjm, phenaproxima: OEmbed ProviderRepositoryTest::...
larowlan’s picture

Status: Reviewed & tested by the community » Fixed

Committed 2c57cbe and pushed to 9.3.x. Thanks!

Backported to 9.2.x as this is only a test change and we have a green test-run above.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.