search

Codeception / module-yii2

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

[Yii2] Transactions are not rolled back #27

Closed koxu1996 closed 2 years ago

koxu1996 commented 5 years ago

What are you trying to achieve?

Run functional tests with database cleanup for Yii2 app.

What do you get instead?

Database is not cleaned up after testing.

I checked database logs and it appears ROLLBACK doesn't work due to ALTER TABLE inside transaction. You can check this fiddle to see the same effect.

I figured out this query is result of calling unloadFixtures() in haveFixtures():

#0 /shopapp/vendor/yiisoft/yii2/db/Command.php(943): yii\db\mysql\QueryBuilder->resetSequence('`user`', 1)                                                                                                                                                                                                     
Codeception/Codeception#1 /shopapp/vendor/yiisoft/yii2/db/QueryBuilder.php(1056): yii\db\Command->resetSequence('user', 1)                                                                                                                                                                                                                          
Codeception/Codeception#2 /shopapp/vendor/yiisoft/yii2/db/Command.php(961): yii\db\QueryBuilder->executeResetSequence('user', 1)                                                    
Codeception/Codeception#3 /shopapp/vendor/yiisoft/yii2/test/ActiveFixture.php(128): yii\db\Command->executeResetSequence('user', 1)                                                                                                                                                                                                                 
Codeception/Codeception#4 /shopapp/vendor/yiisoft/yii2/test/ActiveFixture.php(115): yii\test\ActiveFixture->resetTable()                                                                                                                                                                                                                              
Codeception/Codeception#5 /shopapp/vendor/yiisoft/yii2/test/FixtureTrait.php(121): yii\test\ActiveFixture->unload()                                                                                                                                                                                                                                   
Codeception/Codeception#6 /shopapp/vendor/codeception/base/src/Codeception/Module/Yii2.php(461): Codeception\Lib\Connector\Yii2\FixturesStore->unloadFixtures()                       
Codeception/Codeception#7 [internal function]: Codeception\Module\Yii2->haveFixtures(Array)                                                                                                                                                                                                                                                           
Codeception/Codeception#8 /shopapp/vendor/codeception/base/src/Codeception/Step.php(264): call_user_func_array(Array, Array)                                                                                                                                                                                                                          
Codeception/Codeception#9 /shopapp/vendor/codeception/base/src/Codeception/Scenario.php(72): Codeception\Step->run(Object(Codeception\Lib\ModuleContainer))                                                                                                                                                                                           
Codeception/Codeception#10 /shopapp/tests/_support/_generated/FunctionalTesterActions.php(481): Codeception\Scenario->runStep(Object(Codeception\Step\Action))                                                                                                                                                                                        
Codeception/Codeception#11 /shopapp/tests/functional/UserCest.php(11): FunctionalTester->haveFixtures(Array)                                                                                                                                                                                                                                   
Codeception/Codeception#12 [internal function]: UserPunchCest->_before(Object(FunctionalTester))
...

I wonder if calling this function is necessary... so as temporary workaround I commented out one line https://github.com/Codeception/Codeception/blob/3.0/src/Codeception/Module/Yii2.php#L458 and now database is cleared after testing.

I hope you can investigate and solve this problem.

Runtime details

class_name: FunctionalTester
modules:
    enabled:
        - Filesystem
        - Yii2
        - Asserts
Naktibalda commented 5 years ago

Why does unloadFixtures call ALTER TABLE?

koxu1996 commented 5 years ago

@Naktibalda Look at the call stack: unloadFixtures() leads to yii\db\mysql\QueryBuilder->resetSequence(), which alter table to reset auto_increment value - https://github.com/yiisoft/yii2/blob/master/framework/db/mysql/QueryBuilder.php#L191.

SamMousa commented 5 years ago

We could load and unload fixtures outside the transaction

koxu1996 commented 5 years ago

@SamMousa In my case haveFixtures() is executed during test, so transaction is already started.

samdark commented 5 years ago

The method won't work for MySQL because of that.

koxu1996 commented 5 years ago

Today I basically tested all possibilities of suites and fixture loading places:

and I got strange results:

  1. If you create some instance inside test and another model fixtures were loaded in _before/inside-test, then for every suite (acc/fun/uni), rollback will delete fixtures only. Let's consider following code:

    class Fun2Test
{
public function _before() {
    return ['cars' => CarFixture::className()];
}
public function ensureThatWorks(\FunctionalTester $I)
{
    $fixtures = $I->grabFixtures();
    $model = new Color();
    $I->assertTrue($model->save());
    $I->amOnPage(Url::toRoute('/site/cars-count'));
    $expected_count = count($fixtures);
    $I->see($expected_count);
}
}

    After running above test, you will get success, but Color model still persists in database.

  2. If you create some instance inside test and the same mode were loaded in _fixtures, then for acceptance suite, you will get failure. Let's consider following code:

    class Acc1Test
{
public function _fixtures() {
    return ['cars' => CarFixture::className()];
}

public function ensureThatWorks(AcceptanceTester $I)
{
    $fixtures = $I->grabFixtures();
    $model = new Car();
    $I->assertTrue($model->save());
    $I->amOnPage(Url::toRoute('/site/cars-count'));
    $expected_count = count($fixtures)+1;
    $I->see($expected_count);
}
}

    Test is failing with message: Failed asserting that 9 matches expected '8'.

If you want I can prepare ready-to-test package with above models and tests in some time, but I am very busy now.

SamMousa commented 5 years ago

There is no generic fix that will work here. Some DBMSes will commit transactions when DDL queries are executed. If you run DDL in your tests then it cannot be solved. For this specific case we could alter Yii behavior to not reset sequences..

Or we create some kind of queue for DDL queries that should be executed after rollback (this would be a big undertaking IMO)

koxu1996 commented 5 years ago

The easiest way would be to forbid loading fixtures when transaction is started.

SamMousa commented 5 years ago

Other DBMSes allow DDL inside transaction; for example postgres. (AFAIK)

koxu1996 commented 5 years ago

I think I found answer for my 2nd "strange" result - \Yii::$app in acceptance tests is different instance than tested application. So following code is failing:

$model = new Car();
$I->assertTrue($model->save());
$I->amOnPage(Url::toRoute('/site/cars-count'));
$I->see("Cars: 1");

I believe this is expected, am I right?

koxu1996 commented 5 years ago

And for below code, there is no transaction started, so nothing is rolled back:

class CarController extends Controller
    public function actionTest()
    {
        $model = new Car();
        $model->save();
        return $this->renderContent('OK');
    }
...
class IssueCest
{
    public function ensureEverythingWorks(AcceptanceTester $I)
    {
        $I->amOnPage(Url::toRoute((['/car/test'])));
        $I->see('OK');
    }

Conclusion: clean-up does not work for acceptance tests, no matter if you use _fixture() and which dbms you use.

SamMousa commented 5 years ago

In acceptance tests transactions make no sense. Fixtures could be cleaned up though.

SamMousa commented 5 years ago

Your OP was about functional tests though

mikehaertl commented 5 years ago

I have a suggestion that is kind of related. I was expecting that the flow for running a unit test class is like this:

  1. Load fixtures defined in _fixtures()
  2. Start Transaction
  3. Run first test
  4. Rollback transaction (== Reset fixtures to initial state)
  5. Start Transaction
  6. Run second test
  7. Rollback transaction ... and so on etc.
  8. Unload fixtures.

So I expected fixtures to be loaded only once for a test class.

But instead I see that fixtures are reloaded for each single test inside the class. I was expecting that the main reason for transactions was to speed up the loading/unloading of fixtures. If this is not the case I wonder if such a feature could be added. Shouldn't this also solve the problem in this issue?

SamMousa commented 5 years ago

You could manually load fixtures in the setup / teardown functions.

I personally don't use fixtures at all; I have found it's less work to just create helper functions that create any records I need when I need them. Fixtures imo make sense when they save you a trip to the DB, but not if you just load them into the DB anyway. (I might be wrong here, consider me consciously incompetent)

mikehaertl commented 5 years ago

@SamMousa Thanks, I may give that a try - but it's kind of a hack and I'd prefer if I could just use _fixtures(). So you confirm that what I see is the expected behavior? What's the idea behind transactions at all then?

@samdark Maybe you can also help clarify? Would my suggestion above make sense?

koxu1996 commented 5 years ago

@SamMousa imo fixtures are more handy than helpers for many-to-many relationship.

SamMousa commented 2 years ago

I'm closing this due to its age. If it is still relevant in current versions feel free to create a new issue.