dmaicher / doctrine-test-bundle

Symfony bundle to isolate your app's doctrine database tests and improve the test performance
MIT License
1.08k stars 60 forks source link

Using @depends - Transaction for 2 methods #151

Open tarach opened 3 years ago

tarach commented 3 years ago

Hi, Great package btw.

Does this package allow to reuse the code using @depends annotation? For example.

I test resource POST /api/posts this test return an id of newly created post than I reuse it in test that checks the update endpoint.


    public function testShouldCreatePost(): string
    {
        // ...
    }

    /**
     * @depends testShouldCreatePost
     */
    public function testShouldUpdatePost(string $id): void
    {
        // ...
    }
dmaicher commented 3 years ago

No this is currently not supported. This bundle really aims to have all tests independent of each other when it comes to the database state.

I do see that this might be useful in some scenarios. If this would be possible to implement it would need to be configurable (by default inactive) though as it might break existing tests for other users of this bundle that have @depends annotations because of some non-database related state maybe.

I will keep this open as a feature request. Feel free to dig into it @tarach .

AntonioCS commented 2 years ago

@dmaicher here is a 👍 for this feature as I'm creating API tests. I have a test where I create something and then a test where I patch that something I created (using @depends) but since the DB is "cleared" between tests the @depends is useless.. There should be something I can add to the code (maybe a flag or something) that would commit all that I did on the test.

NicoHaase commented 2 years ago

Why not simply call the other test function first? Why not refactor your tests to have a single method that inserts the data, and call this method from both test cases?

AntonioCS commented 2 years ago

@NicoHaase why not just use something that is available in phpunit called @depends? There are many ways to achieve this, I just gave an example.

NicoHaase commented 2 years ago

@NicoHaase why not just use something that is available in phpunit called @depends?

Because that is not supported by this bundle, and I wanted to show you a way to circumvent this. But as this bundle is open source: if you need the combination of both features, feel free to open a pull request

AntonioCS commented 2 years ago

@NicoHaase I know it's not supported but I just wanted to show support for @tarach and his use case because I'm suffering from the same issue.

BenMorel commented 2 years ago

Add additional +1 from me, it would be nice to be able to configure the bundle to keep the transaction open between two tests when one @depends on the other.

tsetsee commented 2 years ago

+1

chriskaya commented 2 years ago

Hi all

I'm currently facing the same issue, for which I'm simply disabling the feature for the class (see #182 ) A way to make it work would be using a "manual" flag, which would disable auto-begin and auto-rollback before and after test, what do you guys think? I'm ok to create a PR if you're ok with this :)

public static function setUpBeforeClass(): void
{
    StaticDriver::setManualOperations(true);
    StaticDriver::beginTransaction();
}

public static function tearDownAfterClass(): void
{
    StaticDriver::rollBack();
    StaticDriver::setManualOperations(false);
}

This would maybe be also possible with annotations, but I'm not really familiar with them :p

EDIT: I've already created the PR: https://github.com/dmaicher/doctrine-test-bundle/pull/203

chriskaya commented 2 years ago

My PR having been rejected by @dmaicher for reasons I totally understand, I'm now using a custom phpunit extension built on top of doctrine-test-bundle.

If you need this, you may do the same: https://github.com/chriskaya/custom-dama-extension/blob/main/CustomDamaExtension.php

BTW, thx @dmaicher for your work :)

pmontoya commented 2 months ago

Hello,

What about a rollback at the before the test instead of after to check if current has dependencies.

Here is an example of implementation :

final class DatabaseExtension implements Extension
{
    public static bool $transactionStarted = false;

    public static function rollBack(): void
    {
        if (!self::$transactionStarted) {
            return;
        }

        StaticDriver::rollBack();
        self::$transactionStarted = false;
    }

    public static function begin(): void
    {
        if (self::$transactionStarted) {
            return;
        }

        StaticDriver::beginTransaction();
        self::$transactionStarted = true;
    }

    #[\Override]
    public function bootstrap(Configuration $configuration, Facade $facade, ParameterCollection $parameters): void
    {
        $facade->registerSubscriber(new class() implements TestRunnerStartedSubscriber {
            public function notify(TestRunnerStartedEvent $event): void
            {
                StaticDriver::setKeepStaticConnections(true);
            }
        });

        $facade->registerSubscriber(new class() implements TestStartedSubscriber {
            public function notify(TestStartedEvent $event): void
            {
                $test = $event->test();
                if ($test->isTestMethod()) {
                    /** @var TestMethod $test */
                    if (0 === $test->metadata()->isDepends()->count()) {
                        DatabaseExtension::rollBack();
                    }
                }
                DatabaseExtension::begin();
            }
        });

        $facade->registerSubscriber(new class() implements TestRunnerFinishedSubscriber {
            public function notify(TestRunnerFinishedEvent $event): void
            {
                DatabaseExtension::rollBack();
                StaticDriver::setKeepStaticConnections(false);
            }
        });
    }
}

Do you think it could be a good idea ?

mpoiriert commented 1 month ago

@pmontoya Have you tried you extension ? Would it not trigger multiple being if the test have a depends ?

Also some test could be written like this:

MyTest extends KernelTestCase
{
  public function firstTest(): void
  {
  }

  public function secondTest(): void
  {
  }

  #[Depends('firstTest')]
  #[Depends('secondTest')]
  public fucntion thirdTest(): void
  {
  }
}

The work around is easy by adding a "useless" depends on the secondTest but I may have 2 other suggestions.

Having an attribute on the TestCase itself to tell that rollback should only happen on the end of the "test class". Having an attribute on the "test method" to prevent rollback (instead of having the useless depends that might create confusion).

The real question here is does your implementation works ?

I woud want to do something like this (I am assuming that want this feature will do something similar).

MyTest extends WebTestCase
{
  public function createUserTest(): void
  {
    static::createClient()->request('POST', '/users');
  }

  #[Depends('createUserTest')]
  public function connectUserTest(): void
  {
    static::createClient()->post('POST', '/login');
  }

  #[Depends('connectUserTest')]
  public fuction updateUserTest(): void
  {
  }
}
pmontoya commented 1 month ago

@mpoiriert I use this extension from 8 months without any problem as the transaction is rollbacked only if test haven't depends or at the end of test suite.