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
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:
Comments
Comment #2
xjmComment #4
xjmComment #6
xjmComment #7
phenaproximaRe-titling for accuracy. The test itself is accurately testing what it should; it's just not properly using the Guzzle API.
Comment #8
phenaproximaAlso, the test will pass (it did in the other issue). No reason to hold up RTBC!
Comment #9
xjmComment #11
larowlanCommitted 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.