Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Phpunit (part 1) #7344

Merged
merged 2 commits into from
Apr 6, 2024
Merged

Upgrade Phpunit (part 1) #7344

merged 2 commits into from
Apr 6, 2024

Conversation

mamazu
Copy link
Contributor

@mamazu mamazu commented Apr 3, 2024

What's in this PR?

  • replacing assertObjectHasAttribute
  • Fixing some tests

Why?

Having it in the bigger #7339 PR makes it hard.


$this->assertObjectHasAttribute('logo', $form);
$this->assertObjectHasAttribute('account', $form);
$response = \json_decode($this->client->getResponse()->getContent(), true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going for array deserialization would make it easier in some places, as we can just compare arrays.

@@ -59,7 +59,7 @@ class MediaDataProviderRepositoryTest extends SuluTestCase
/**
* @var array
*/
private $mediaData = [
private static $mediaData = [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This data is static and doesn't need to live on the test. Helpful for making the dataproviders static later.

* Class AbstractScaleTest.
*/
abstract class AbstractTransformationTest extends SuluTestCase
abstract class AbstractTransformation extends SuluTestCase
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renaming the test as it otherwise will be picked up as a test but can't be instantiated as a test.

Comment on lines -316 to -319
$this->client->getContainer()->get('event_dispatcher')->addListener(
'sulu.tag.delete',
[$mockedEventListener, 'onDelete']
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way to check if this method is called can be simplified by just passing a closure and setting a variable inside to true. That way we don't need to create a mock object.


namespace Sulu\Component\Rest\Tests\Unit;

interface MockInterface
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class was used in the RestHelperTest. Phpunit 10 will not allow to Mock classes that don't exist so, just created this interface to use it in mocking.

Comment on lines +356 to +359
$mockedObject = $this->createConfiguredMock(MockInterface::class, [
'getId' => 1,
'getValue' => 2,
]);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be reverted but it's the same as above just shorter.

return [
[\stdClass::class, false],
[\get_class($securedEntity->reveal()), true],
[SecuredEntityInterface::class, true],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why create a mock when we just need the class of it?

@alexander-schranz alexander-schranz merged commit bd45922 into sulu:2.6 Apr 6, 2024
8 checks passed
@alexander-schranz
Copy link
Member

@mamazu Thank you!

@alexander-schranz alexander-schranz added the DX Affecting the end developer label Apr 6, 2024
@mamazu mamazu deleted the phpunit_part_1 branch April 24, 2024 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Affecting the end developer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants