- Issue created by @mondrake
- Status changed to Needs review
over 1 year ago 1:28pm 7 June 2023 - last update
over 1 year ago 29,420 pass - Status changed to Needs work
over 1 year ago 9:58pm 7 June 2023 - 🇺🇸United States smustgrave
Woo, assuming since the tests were green your approach worked? I feel there could be a lot of these though. Should they be broken up across a few tickets? Might be easier to review and get in.
- Status changed to Needs review
over 1 year ago 7:15am 8 June 2023 - 🇮🇹Italy mondrake 🇮🇹
@smustgrave why NW then? What work is needed on this issue?
If this approach (replacing PHPUnit mocks with prophecies where possible) is endorsed, then this may become a recommended way forward that we could summarize in https://www.drupal.org/node/3365413 → . But that should happen after commit IMHO.
We need that endorsment and hopefully scope guidance on how to address the rest from a committer. Scope discussion in this type of things tend to take longer than doing the conversions themselves...
BTW, a list of dataprovider methods that need the conversion can be seen in the latest test runs of the MR in 📌 [PHPUnit 10] @dataProvider methods must be declared static and public Fixed .
- 🇳🇱Netherlands spokje
Would the tag
Needs framework manager review
help here?If we have a blessing on the approach taken here, we can then either commit this one as the first of many others. or (which I see incoming) do multiple/all of them in one/multiple issues.
- 🇺🇸United States smustgrave
That was my mistake didn’t mean to change the status
- Status changed to RTBC
over 1 year ago 3:04pm 21 June 2023 - 🇺🇸United States smustgrave
Marking this to get in front of committer eyes.
- last update
over 1 year ago 29,506 pass - Status changed to Fixed
over 1 year ago 10:50am 22 June 2023 - 🇬🇧United Kingdom catch
+++ b/core/modules/config_translation/tests/src/Unit/ConfigMapperManagerTest.php @@ -151,13 +153,11 @@ public function providerTestHasTranslatable() { */ - protected function getElement(array $definition) { + protected static function getElement(array $definition) { $data_definition = new DataDefinition($definition); - $element = $this->createMock('Drupal\Core\TypedData\TypedDataInterface'); - $element->expects($this->any()) - ->method('getDataDefinition') - ->willReturn($data_definition); - return $element; + $element = (new Prophet())->prophesize(TypedDataInterface::class); + $element->getDataDefinition()->willReturn($data_definition); + return $element->reveal(); } /** @@ -169,20 +169,16 @@ protected function getElement(array $definition) { @@ -169,20 +169,16 @@ protected function getElement(array $definition) { * @return \Drupal\Core\Config\Schema\Mapping * A nested schema element, containing the passed-in elements. */ - protected function getNestedElement(array $elements) { + protected static function getNestedElement(array $elements) { // ConfigMapperManager::findTranslatable() checks for // \Drupal\Core\TypedData\TraversableTypedDataInterface, but mocking that // directly does not work, because we need to implement \IteratorAggregate // in order for getIterator() to be called. Therefore we need to mock // \Drupal\Core\Config\Schema\ArrayElement, but that is abstract, so we // need to mock one of the subclasses of it. - $nested_element = $this->getMockBuilder('Drupal\Core\Config\Schema\Mapping') - ->disableOriginalConstructor() - ->getMock(); - $nested_element->expects($this->once()) - ->method('getIterator') - ->willReturn(new \ArrayIterator($elements)); - return $nested_element; + $nested_element = (new Prophet())->prophesize(Mapping::class); + $nested_element->getIterator()->shouldBeCalledTimes(1)->willReturn(new \ArrayIterator($elements)); + return $nested_element->reveal(); } }
For me this diff looks a lot cleaner than the original, so seems like a straightforward change.
Agreed we should document the pattern somewhere but shouldn't hold up the conversion.
- 🇮🇹Italy mondrake 🇮🇹
Thanks! Filed 📌 [PHPUnit 10] Provide a static alternative to @dataproviders using PHPUnit mocks in CommentLinkBuilderTest and MailHandlerTest Needs review to follow up with the rest.
Automatically closed - issue fixed for 2 weeks with no activity.