Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Yii2] Transactions are not rolled back #27

Closed
koxu1996 opened this issue Jul 20, 2019 · 18 comments
Closed

[Yii2] Transactions are not rolled back #27

koxu1996 opened this issue Jul 20, 2019 · 18 comments

Comments

@koxu1996
Copy link

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

  • Codeception version: 3.0.2
  • PHP Version: 7.3.7
  • Operating System: Arch Linux (5.2.1-arch1-1-ARCH)
  • Installation type: Composer
  • Suite configuration:
class_name: FunctionalTester
modules:
    enabled:
        - Filesystem
        - Yii2
        - Asserts
@Naktibalda
Copy link
Member

Why does unloadFixtures call ALTER TABLE?

@koxu1996
Copy link
Author

@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
Copy link
Collaborator

We could load and unload fixtures outside the transaction

@koxu1996
Copy link
Author

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

@samdark
Copy link
Collaborator

samdark commented Jul 23, 2019

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

@koxu1996
Copy link
Author

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

  • acceptance + _fixtures
  • acceptance + _before/inside-test
  • functional + _fixtures
  • functional + _before/inside-test
  • unit + _fixtures
  • unit + _before/inside-test

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.

  1. 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
Copy link
Collaborator

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
Copy link
Author

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

@SamMousa
Copy link
Collaborator

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

@koxu1996
Copy link
Author

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
Copy link
Author

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
Copy link
Collaborator

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

@SamMousa
Copy link
Collaborator

Your OP was about functional tests though

@mikehaertl
Copy link
Contributor

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
Copy link
Collaborator

SamMousa commented Aug 2, 2019

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
Copy link
Contributor

@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
Copy link
Author

koxu1996 commented Aug 2, 2019

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

@Naktibalda Naktibalda transferred this issue from Codeception/Codeception Jan 3, 2021
@SamMousa
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants