Reset TestTransport on ensureKernelShutdown

[HypeMC]

An alternative approach to #82, requires https://github.com/symfony/symfony/pull/58240

[HypeMC]

requires symfony/symfony#58240

why don't we implement ResetInterface? So we won't need Symfony 7.2 to benefit from this feature.

@nikophil Hi, the reason is your comment about introducing a BC break. With this approach, the existing method's behavior remains unchanged.

Also, symfony/symfony#58240 is a bugfix, so it won't require 7.2.

Would you mind adding a test which confirms that everything is working? We should test this in both a test which uses InteractsWithMessenger trait, and a test which doesn't, I guess

Sure, no problem, but I don't think I can actually cover the original scenario as it depends on the execution order and a specific test being run first:

So if we have this scenario :

1. Test1 add messages on transport but doesn't care about messenger assertions so doesn't implement InteractsWithMessenger
2. messages remains in transports
3. Test2 implements InteractsWithMessenger and should reset collected messages before tests.

[nikophil]

the reason is https://github.com/zenstruck/messenger-test/pull/82#issuecomment-2096198752. With this approach, the existing method's behavior remains unchanged.

oups yes, you're right :hand_over_mouth: let's do your way then!

Also, https://github.com/symfony/symfony/pull/58240 is a bugfix, so it won't require 7.2.

perfect

depends on the execution order and a specific test being run first

yeah... we're doing strange things to test behavior between tests in this library :sweat_smile: ... basically, not using --order-by nor any executionOrder in the xml will play the tests in the natural order (alphabetically). I know this is not ideal, but it is the only way to test such behaviors. No problem if you don't want to bother with this, I might do it at some point. Maybe I'll upgrade first to PHPUnit 10, in order to leverage #[DependsExternal] attribute...

[HypeMC]

depends on the execution order and a specific test being run first

yeah... we're doing strange things to test behavior between tests in this library 😅 ... basically, not using --order-by nor any executionOrder in the xml will play the tests in the natural order (alphabetically). I know this is not ideal, but it is the only way to test such behaviors. No problem if you don't want to bother with this, I might do it at some point. Maybe I'll upgrade first to PHPUnit 10, in order to leverage #[DependsExternal] attribute...

@nikophil Actually, I just noticed the testsuite name="zenstruck/messenger-test transports are reset correctly" which is exactly what I need, so I think the test is possible.

[HypeMC]

@nikophil Funny thing, the test was kind of already there, it just didn't work when all tests were run cause it requires TestTransport::$enableMessagesCollection to be set to its initial value of true for the bug to manifest. The tests are now failing cause they require https://github.com/symfony/symfony/pull/58240. See https://github.com/zenstruck/messenger-test/pull/90/commits/ab2123d74dbb586b92c6bfb35095deea08c44eea

[nikophil]

Don't hesitate to ping me once the fix is released in SF, and I'll merge and release this PR

It seems there is a bunch of issues and PR that can be closed as well. Thanks!

[HypeMC]

@nikophil The fix has been released by Symfony, however, I don't think it's possible to get the tests for 6.3 and 7.0 to pass since those versions are no longer supported. I'm not sure what to do about this, the fix is valid for all supported versions.

[nikophil]

hey @HypeMC

I just discovered that some tests are failing, maybe because of a new version of Symfony. I'll fix them asap, before merging your PR

[nikophil]

@HypeMC would you mind to rebase your PR?

[HypeMC]

@HypeMC would you mind to rebase your PR?

@nikophil Done

[nikophil]

released in 1.10.0