Codeception / module-symfony

Codeception module for testing apps using Symfony framework
MIT License
89 stars 24 forks source link

persistent services break shared services #151

Open ctrl-f5 opened 2 years ago

ctrl-f5 commented 2 years ago

This module sets a instance of the entity manager in the container because this is hard coded as a persistent service.

This results in all doctrine event listeners being instantiated on the first call, and those references end up in the entity manager.

However, when the kernel reboots, and a new test is started only the entity manager itself is set on the container. All listeners and their dependencies are still marked as uninitialized in the new container.

When a Doctrine event listener and another random service have the same service as a dependency, the latter will end up with a new instance of that dependency every test. while the doctrine listener will always be stuck with the original instance of the dependency.

This effectively breaks the shared services since there are now multiple instances, And the original service only known to doctrine can never be accessed anymore by the new kernel or the test code.

This is a simple use case to show the issue:

class TestSubscriber implements EventSubscriberInterface
{
    public $flushes = 0;

    public function getSubscribedEvents(): array { return [Events::onFlush]; }

    public function __invoke() { $this->flushed++; }
}

class MyTest extends FunctionalTestCase
{
    public function it_can_inspect_doctrine_event_listeners(): void
    {
        $subscriber = $this->tester->grabService(TestSubscriber::class);
        $this->tester->assertSame(1, $subscriber->flushes);
    }
}

This test will always fail, because the instance of TestSubscriber in the test case's container is never the same as the one in the container's entity manager instance.

Again, this is a very simple test case, but when subscribers also have dependencies from the container they all get decoupled from the actual test case's container, and can never be accessed anymore.

ctrl-f5 commented 2 years ago

I havent used another testing framework for a couple of years now, and this is somewhat of an edge case.

Currently I see 3 options to fix this issue:

Since option 2 is the quickest and easiest, I'll have a look if this fixes the issue for my case and is somewhat workable.

vdanielpop commented 1 year ago

@ctrl-f5 A bit late to the party, but isn't this a potential memory leak since those decoupled services are just "lost" ?

We're facing a problem where the memory allocated when running our codeception suite increases dramatically, the more tests we run. I haven't dove deep enough to make sure I fully understand what's going on under the hood, but after reading your post it makes a lot of sense and it also explains what we've encountered.

ctrl-f5 commented 1 year ago

This indeed also causes memory leaks due to lingering services. The lingering references cause a chain effect where the entire kernel and container can not be garbage collected, and thus kernels keep piling up with every test that is ran.

We have codeception running in a relatively large codebase (600 codeception tests of which 200+ kernel tests) and had to split up the test runs in multiple suites (just for the kernel tests) to be able to run it on a CI worker with 2GB memory.

We have dug deep and tried a lot of things to solve this, but we have not yet found a sufficient solution.

vdanielpop commented 10 months ago

Hey @ctrl-f5 , forgot to post an update to my adventure and then Christmas happened 😄

We ended up doing a couple of small changes, that together improved our duration by quite a lot:

  1. It turns out the debug flag's default value is true, so we disabled that first 🤦‍♂️
  2. While doing some memory analysis we noticed that PHP's default garbage collection was triggering a bit too often. So we implemented a custom Garbage Collector that triggers more rarely:

    snippet ```php class GarbageCollector extends Module { private const GC_ROOTS_THRESHOLD = 250000; /** * @param array $settings */ public function _beforeSuite($settings = []): void { \gc_disable(); } public function _afterSuite(): void { \gc_enable(); } public function _after(TestInterface $test): void { $status = \gc_status(); if ($status['roots'] < self::GC_ROOTS_THRESHOLD) { return; } \gc_collect_cycles(); } } ```

These reduced the duration of the suite from ~1 hour to 5-6 minutes. It's important to note though that the memory leak is still there, however it's no longer an issue. We also upgraded to v5 which comes with sharding, so in case we end up adding enough tests so that it becomes an issue again, we'll just split the tests up. Hope this is useful for you too!

ctrl-f5 commented 10 months ago

Thanks for the update!

I tried it in our pipeline but sadly, no change for our use case.

simonberger commented 2 months ago

I am debugging a failure in a newly added api test which seem to be related to this issue and most likely also the cause of #150

I have an entity listener, which has some injected services. One of those injects the RequestStack. The result is requestStack is always empty when used, because it is injected before the first reboot and when doing the actual request it not added to this requestStack.

I made a patch storing the very first requestStack and always push the current request to it. This was before I understand the context. it ist most likely no general requestStack issue, but when it is used in an entity manager listener.

The doctrine service stores the whole first container without resetting it. I am not sure how this could be fixed in a proper way. I tried to just reset the entity listener part or the container of the doctrine services, but failed so far.