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

Add option to override services #13

Merged
merged 11 commits into from
Dec 30, 2023
Merged

Add option to override services #13

merged 11 commits into from
Dec 30, 2023

Conversation

maldoinc
Copy link
Owner

@maldoinc maldoinc commented Dec 21, 2023

Adds container.override providing methods to set/remove and context managers to override one/many services.

  • ctx manager
    • container.override.service(target=RandomService, new=random_mock)
    • container.override.services(overrides)
  • container.override.set(target=RandomService, new=random_mock)
  • container.override.delete(RandomService)

Fixes #10.

self.delete(target, qualifier)

@contextmanager
def services(self, overrides: list[ServiceOverride]) -> Iterator[None]:

Choose a reason for hiding this comment

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

+1 for the list variant, I've found it quickly became useful/necessary to keep tests tidy.

@snewell92
Copy link

This has worked great for us, the only friction was when an eng on the team ran up against the Singleton nature of the container and overrides where they expected the override container to dynamically swap out a subdependency, which isn't what it can do if the initial target has already been initialized. They had a funny symptom of this where their test passed when ran in isolation, but when the whole suite was ran their new test failed.

The solution for such a situation that we did , for now imo, is to override the service being directly requested by the code under test, and go from there, rather than trying to surgically overrid only some deep depenendency. You can still use the real impls and just provide mocks to the services one needs for the test at hand.

An alternate solution may be to introduce lifetimes/scopes such that, in a test at least, the entire di container resets every test (or perhaps certain deps are?) so each test gets a fresh instance of everything. The other kind of application that makes sense to me for this would be flask request scoped services that exist for things on the request (jwt auth stuff maybe?) - I'm not too worried about this tho as there is some code I've seen thrown around about how we use threadlocal to get a kind of scoping now and I'd actually prefer just to use singletons and factories when I have specific scoping needs.

Copy link

@snewell92 snewell92 left a comment

Choose a reason for hiding this comment

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

The core impl and context manager look great, thanks for adding the list variant and breaking out a small dataclass to encapsulate a ServiceOverride 👍

(Edit: I'm out on holiday rn, otherwise I could swap our impl with this to port over but I'm pretty confident in what I read)

@maldoinc
Copy link
Owner Author

This has worked great for us, the only friction was when an eng on the team ran up against the Singleton nature of the container and overrides where they expected the override container to dynamically swap out a subdependency, which isn't what it can do if the initial target has already been initialized. They had a funny symptom of this where their test passed when ran in isolation, but when the whole suite was ran their new test failed.

Replacing transitive dependencies is not possible with the current design as services are not proxy objects. This has the benefit of not introducing performance penalty when using them but you can't do magic stuff like this. I can see why the engineer would think that though.

The solution for such a situation that we did , for now imo, is to override the service being directly requested by the code under test, and go from there, rather than trying to surgically overrid only some deep depenendency. You can still use the real impls and just provide mocks to the services one needs for the test at hand.

This is what I'd have recommended. Assuming some db application, the endpoint should not interact directly with the db but it goes through some service or repository which can be easily mocked. Same thing for anything that might call some third-party api, you create a wrapper around it.

Additionally consider testing against the real thing via some testcontainer which hosts whatever service and simply point to the new instance by updating the corresponding parameter containing some DSN.

An alternate solution may be to introduce lifetimes/scopes such that, in a test at least, the entire di container resets every test (or perhaps certain deps are?) so each test gets a fresh instance of everything. The other kind of application that makes sense to me for this would be flask request scoped services that exist for things on the request (jwt auth stuff maybe?) - I'm not too worried about this tho as there is some code I've seen thrown around about how we use threadlocal to get a kind of scoping now and I'd actually prefer just to use singletons and factories when I have specific scoping needs.

Scoped dependencies will make an appearance at some point and I'll add a function to clear any initialized singleton instances which would be useful in some setUp method.

@maldoinc maldoinc merged commit ca68dd2 into master Dec 30, 2023
7 checks passed
@maldoinc maldoinc deleted the container-override branch December 30, 2023 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an easy way to override dependencies
2 participants