Replacing the mocks in phpunit dataproviders with actual objects #7412
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mamazu
force-pushed
the
no_mocking_in_dataprovider
branch
3 times, most recently
from
a22c0fd
to
a00fe76
Compare
| use PHPCR\NodeInterface; | ||
| use Sulu\Component\Content\Extension\AbstractExtension; | ||
|
|
||
| class TestExtension extends AbstractExtension |
There was a problem hiding this comment.
would not create a TestExtension class here instead use anonymous class inside the specific test case: e.g.:
private static function createTestExtension(): AbstractExtension
{
return new class extends AbstractExtension() {
// ..
};
}
|
We should target the 2.5 to make backmerge of bugfixes easier. |
mamazu
force-pushed
the
no_mocking_in_dataprovider
branch
2 times, most recently
from
fc3949e
to
19905d0
Compare
mamazu
force-pushed
the
no_mocking_in_dataprovider
branch
from
19905d0
to
266f65e
Compare
mamazu
force-pushed
the
no_mocking_in_dataprovider
branch
from
8d23d10
to
7a5587f
Compare
mamazu
force-pushed
the
no_mocking_in_dataprovider
branch
from
7a5587f
to
1741ea8
Compare
mamazu
commented
May 8, 2024
Comment on lines
244
to
253
| self::setPrivateProperty($entity, 'id', $id); | ||
| self::setPrivateProperty($entity, 'number', $id); | ||
| self::setPrivateProperty($entity, 'name', $name); | ||
| self::setPrivateProperty($entity, 'tags', new ArrayCollection($tags)); | ||
| self::setPrivateProperty($entity, 'placeOfJurisdiction', ''); | ||
| self::setPrivateProperty($entity, 'uid', ''); | ||
| self::setPrivateProperty($entity, 'corporation', ''); | ||
| self::setPrivateProperty($entity, 'created', new \DateTime()); | ||
| self::setPrivateProperty($entity, 'changed', new \DateTime()); | ||
| self::setPrivateProperty($entity, 'medias', new ArrayCollection([])); |
There was a problem hiding this comment.
This is highly in efficient because we are reflecting on this class with every method call. But it's just the tests.
There was a problem hiding this comment.
@mamazu for properties providing a setter I would directly call the setter method and not go over reflection. So in our case the reflection mostly only used for the primary keys.
There was a problem hiding this comment.
Fixed. The only place I have also left with the function is the cache in the MediaDataProvider.
mamazu
force-pushed
the
no_mocking_in_dataprovider
branch
from
af9ad1d
to
421d4f9
Compare
alexander-schranz
approved these changes
May 9, 2024
|
@mamazu Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What's in this PR?
Replacing the mock object with real objects in the dataproviders
Why?
For the phpunit 10 upgrade and mocking hides implementation problems.