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

Don't mock the service container #1701

Merged
merged 1 commit into from
Sep 5, 2023

Conversation

derrabus
Copy link
Member

@derrabus derrabus commented Sep 4, 2023

Spotted while reviewing #1692.

Mocking the service container for tests requires us to write some pretty complicated mocking logic for little to no gain. I think, our tests are easier to comprehend and maintain, if we use Symfony's Conatiner class instead.

@ostrolucky
Copy link
Member

Mocking allows us to verify that tested class is trying to pull the service out of container. Don't we need that? But yeah I am not opposed, although benefit is not that high. And what's up with gh actions?

@derrabus
Copy link
Member Author

derrabus commented Sep 4, 2023

Mocking allows us to verify that tested class is trying to pull the service out of container. Don't we need that?

I don't think we do. Our tests verify that a service returned by the registry this the very same that we've put into the container. I think that is enough.

benefit is not that high.

Looking at the withConsecutive()/willReturnOnConsecutiveCalls() gymnastics that I could remove, I'd say it's worth it. 🙂

And what's up with gh actions?

Dunno. I restarted the jobs and they passed. 🤷🏻‍♂️

@ostrolucky ostrolucky merged commit 417143b into doctrine:2.10.x Sep 5, 2023
@derrabus derrabus deleted the improvement/no-container-mock branch September 5, 2023 07:23
@derrabus derrabus added this to the 2.10.3 milestone Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants