What happens
MockVWS inherits from contextlib.ContextDecorator:
class MockVWS(ContextDecorator):
so an instance can be used as a decorator. The target manager is built in __init__ rather than in __enter__:
self._target_manager = TargetManager()
so every use of the same instance shares one set of databases and targets. Decorating two functions with one instance:
mock = MockVWS()
mock.add_cloud_database(cloud_database=db)
@mock
def add_one_target(): ... # POST /targets with name "only-one"
@mock
def count_targets(): ... # GET /targets
gives:
first call add_one_target -> 201
targets now -> 1
second call add_one_target -> 403
targets now -> 1
The second call fails with TargetNameExist because the target from the first call is still there.
Why it matters
The decorator form is the one where per-call isolation is most expected, because that is what the comparable tools do — @responses.activate and @mock.patch both give each call a clean slate. Someone reaching for @MockVWS() on a set of test methods gets order-dependent tests and a 403 that does not obviously point at the cause.
ContextDecorator is not currently exercised or described anywhere: there is no use of MockVWS as a decorator in tests/, in docs/, or in README.rst. Every documented example uses with MockVWS() as mock:. So this is a public capability with no tests, no documentation, and a sharp edge.
I am not sure which way this should go, which is why this is a question rather than a bug report.
Suggested resolution
Pick one.
Make each use isolated. Move the TargetManager construction into __enter__. Every documented example adds its databases after entering the context, so this does not break the documented usage, and it makes @MockVWS() behave the way the decorator idiom implies. It does change behaviour for anyone relying on add_cloud_database before __enter__, which is a real pattern even if it is not the documented one, so it needs a changelog note.
Or document the current behaviour. State in the API reference that a MockVWS instance carries state across uses and that a fresh instance is wanted per test, and add a decorator example to the docs so the supported usage is visible.
Or drop ContextDecorator. If decorator use is not intended to be supported, inheriting from it advertises something the project does not want to promise, and removing it makes the context manager the single documented entry point.
Whichever is chosen, the decorator path wants a test, since it currently has none.
What happens
MockVWSinherits fromcontextlib.ContextDecorator:so an instance can be used as a decorator. The target manager is built in
__init__rather than in__enter__:so every use of the same instance shares one set of databases and targets. Decorating two functions with one instance:
gives:
The second call fails with
TargetNameExistbecause the target from the first call is still there.Why it matters
The decorator form is the one where per-call isolation is most expected, because that is what the comparable tools do —
@responses.activateand@mock.patchboth give each call a clean slate. Someone reaching for@MockVWS()on a set of test methods gets order-dependent tests and a 403 that does not obviously point at the cause.ContextDecoratoris not currently exercised or described anywhere: there is no use ofMockVWSas a decorator intests/, indocs/, or inREADME.rst. Every documented example useswith MockVWS() as mock:. So this is a public capability with no tests, no documentation, and a sharp edge.I am not sure which way this should go, which is why this is a question rather than a bug report.
Suggested resolution
Pick one.
Make each use isolated. Move the
TargetManagerconstruction into__enter__. Every documented example adds its databases after entering the context, so this does not break the documented usage, and it makes@MockVWS()behave the way the decorator idiom implies. It does change behaviour for anyone relying onadd_cloud_databasebefore__enter__, which is a real pattern even if it is not the documented one, so it needs a changelog note.Or document the current behaviour. State in the API reference that a
MockVWSinstance carries state across uses and that a fresh instance is wanted per test, and add a decorator example to the docs so the supported usage is visible.Or drop
ContextDecorator. If decorator use is not intended to be supported, inheriting from it advertises something the project does not want to promise, and removing it makes the context manager the single documented entry point.Whichever is chosen, the decorator path wants a test, since it currently has none.