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

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

Open
wants to merge 1 commit into
base: 1.x
Choose a base branch
from

Conversation

sidz
Copy link

@sidz sidz commented Apr 23, 2024

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

Discussed in: #77 (comment)

…ould be cleared automatically after each tearDown
@sidz
Copy link
Author

sidz commented Apr 23, 2024

Hey @kbond, @nikophil,.

I've raised this pull request in order to speedup improvement discussed in #77 (comment). But it seems like there are a couple of test cases which have been written to be sure that state is persisting between tests.

public function transport_data_is_persisted_between_requests_and_kernel_shutdown(): void
{
self::bootKernel();
self::getContainer()->get(MessageBusInterface::class)->dispatch(new MessageA());
self::getContainer()->get(MessageBusInterface::class)->dispatch(new MessageA(true));
$this->transport()->queue()->assertCount(2);
self::ensureKernelShutdown();
self::bootKernel();
$this->transport()->queue()->assertCount(2);
$this->transport()->process();

public function disabling_intercept_is_remembered_between_kernel_reboots(): void
{
self::bootKernel();
$this->transport()->unblock();
self::getContainer()->get(MessageBusInterface::class)->dispatch(new MessageA());
$this->transport()->queue()->assertEmpty();
$this->transport()->dispatched()->assertCount(1);
self::ensureKernelShutdown();
self::bootKernel();
self::getContainer()->get(MessageBusInterface::class)->dispatch(new MessageA());
$this->transport()->queue()->assertEmpty();
$this->transport()->dispatched()->assertCount(2);
}

so my PR seems like a BC break.

@kbond
Copy link
Member

kbond commented Apr 24, 2024

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
Copy link
Member

nikophil commented May 6, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants