search

zenstruck / messenger-test

Assertions and helpers for testing your symfony/messenger queues.
MIT License
218 stars 15 forks source link

Implement Symfony\Contracts\Service\ResetInterface in the TestTransport #82

Open sidz opened 3 months ago

sidz commented 3 months ago

Implement Symfony\Contracts\Service\ResetInterface so TestTransport could be cleared automatically after each tearDown

Discussed in: https://github.com/zenstruck/messenger-test/pull/77#issuecomment-2060798321

sidz commented 3 months ago

Hey @kbond, @nikophil,.

I've raised this pull request in order to speedup improvement discussed in https://github.com/zenstruck/messenger-test/pull/77#issuecomment-2060798321. But it seems like there are a couple of test cases which have been written to be sure that state is persisting between tests.

https://github.com/zenstruck/messenger-test/blob/7164fce81adab79b2a98bdf8f0df8047ac6d1c91/tests/InteractsWithMessengerTest.php#L522-L536

https://github.com/zenstruck/messenger-test/blob/7164fce81adab79b2a98bdf8f0df8047ac6d1c91/tests/InteractsWithMessengerTest.php#L584-L602

so my PR seems like a BC break.

kbond commented 3 months ago

But it seems like there are a couple of test cases which have been written to be sure that state is persisting between tests.

Not between tests but between kernel reboots within a single test. We need to keep this behaviour.

I think the crux of the issue is in tests NOT using InteractsWithMessenger, we do want the transport reset on reboot.

nikophil commented 2 months ago

hi @sidz

we should detect if the current test uses the trait InteractWithMessenger (maybe by doing something similar than here)

And if we detect that the trait was used, we should do nothing in reset()

but it makes me think that this is also a BC break, because TestTransport::reset() is a public method, which is documented... we should not change its behavior this way.... WDYT @kbond?