Codeception / module-yii2

Codeception module for Yii2 framework
MIT License
16 stars 36 forks source link

Tests fail when database is used at application boostrap #98

Open warkadiuszz opened 7 months ago

warkadiuszz commented 7 months ago

Hello,

after adding a function to the boostrap of my Yii2 application, a lot of my tests stopped working. For all of them it was always one of two reasons:

  1. TooManyConnections (just as the other old issues that happened in the past in that module, eg. here and here).
  2. Data in the database not being cleared after each test (similar to this)

All tests started working again immediately after removing the function in boostrap. Imporantly, the additional code in the boostrap uses database. For example (config main.php):

'bootstrap' => [
  'something' => function () {
    Model::find()->all();
  }
],

Explanation / debugging / working fix

What I was able to find out is that both ConnectionWatcher and TransactionForcer classes work by hooking to Connection::EVENT_AFTER_OPEN event. When some code in boostrap uses database, before those two watchers are set up, the EVENT_AFTER_OPEN happens before them, so they never trigger.

The order of the code in Module/Yii2::_before (here) is:

1: $this->client->startApp($this->yiiLogger);

2: $this->connectionWatcher = new Yii2Connector\ConnectionWatcher();
3: $this->connectionWatcher->start();

// ...

4: $this->startTransactions();

Application boostrap happens at step (1) - startApp. If database is used at that point, Connection::EVENT_AFTER_OPEN also happens. Then the handlers created by (2), (3) and (4) never work again(?), because they are waiting for that event to happen.

If instead the startApp is called last:

2: $this->connectionWatcher = new Yii2Connector\ConnectionWatcher();
3: $this->connectionWatcher->start();

// ...

4: $this->startTransactions();
1: $this->client->startApp($this->yiiLogger);

Then both ConnectionWatcher and TransactionForcer (startTransactions()) bind to Connection::EVENT_AFTER_OPEN before bootstrap so they work correctly.


Steps to reproduce

  1. In application config add function that calls DB:

    'bootstrap' => [
    'something' => function () {
    SomeModel::find()->all();
    }
    ],
  2. Create a test with 2 cases that relies on data being cleaned between them:

    
    <?php

namespace app\tests\unit;

use Codeception\Test\Unit;

class SomeTest extends Unit { public function testCaseOne(): void { // Ensure no records are present in DB
$this->assertEquals(0, SomeModel::find()->count());

// Create record
$m = new SomeModel();
$m->save();

$this->assertEquals(1, SomeModel::find()->count());
}

public function testCaseTwo(): void
{
// Ensure no records are present in DB
// This will fail if boostrap function using database is present
$this->assertEquals(0, SomeModel::find()->count());
}

}



---

With all that being said, I'm not sure if using database during application bootstrap is even legal. Offical Yii2 documentation does not seem to prohibit that though, at least I was not able to find such information. Also everything works.
Another thing is – I'm not sure if the order of those functions calls can be changed at all and if it won't be a breaking change for anyone else. It certainly works for me and, in my case, it has been broken since [this](https://github.com/Codeception/Codeception/pull/4894) rework from years ago. For now I keep that change in private fork but if the maintainers would be interested, I'll be happy to open a PR. 
samdark commented 6 months ago

I'm not sure if using database during application bootstrap is even legal

As you can see, that usage is tricky.

Another thing is – I'm not sure if the order of those functions calls can be changed at all and if it won't be a breaking change for anyone else.

Likely it would break something. @SamMousa since you did that rework, do you remember order change reasoning?

SamMousa commented 6 months ago

I don't see an issue directly with changing the order. When do we remove class level event listeners? If we don't do that before starting the app in startApp it should be fine