phpunit icon indicating copy to clipboard operation
phpunit copied to clipboard

Reliable API for freeing resources used by the test

Open morozov opened this issue 3 years ago • 1 comments

Take this test for example:

<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Tests;

use Exception;
use PHPUnit\Framework\TestCase;

abstract class IntegrationTest extends TestCase
{
    protected $connection;

    /**
     * @before
     */
    protected function connect(): void
    {
        // connect to the database
        $this->connection = ...;
    }

    /**
     * @after
     */
    protected function disconnect(): void
    {
        // disconnect from the database
        $this->connection = null;
    }
}

class TearDownFailureTest extends IntegrationTest
{
    protected function tearDown(): void
    {
        throw new Exception();
    }

    public function testSkipped(): void
    {
        self::markTestSkipped();
    }
}

The code above does the following:

  1. The base class defines the @before and @after methods that will prepare and clean up the test database and the connection.
  2. The specific test cases may define their own setup and teardown procedures.

There is a problem in the TearDownFailureTest that it may get skipped but it doesn't check (and probably can't) if it was skipped, it attempts to perform the teardown logic and throws the exception that gets suppressed by the test runner. Because of that, the disconnect() method in the parent class doesn't get called and the test instance remains connected to the database leaking resources.

It wouldn't be a problem if the test object got destroyed after the run but it doesn't (there are a few reported issues about that, e.g. https://github.com/sebastianbergmann/phpunit/issues/3039).

If the teardown methods are meant for the cleanup, should they be called independently on the results of each other? Currently, an exception stops the chain: https://github.com/sebastianbergmann/phpunit/blob/8053bc6441311c5b8db213cac56f87633b8c14d6/src/Framework/TestCase.php#L919-L924

Currently, the desired logic could be implemented in a TestListener class which has access to the Test object but it's deprecated: https://github.com/sebastianbergmann/phpunit/blob/5a9574812e6f77e376e1e391c1577425523db829/src/Framework/TestListener.php#L37

Neither the deprecated TestHook API, nor the event-based API seem to provide access to the Test instance for reliable cleanup.

morozov avatar Oct 13 '21 01:10 morozov

duplicate of https://github.com/sebastianbergmann/phpunit/issues/4705

mvorisek avatar Dec 12 '21 18:12 mvorisek