search

laravel-doctrine / migrations

Integration with Doctrine2's migrations package for Laravel
MIT License
75 stars 77 forks source link

[Bug] DatabaseMigrations trait rollsback only the latest migration #69

Closed hshahdoost closed 6 years ago

hshahdoost commented 6 years ago

I believe, The behavior of this trait LaravelDoctrine\Migrations\Testing\DatabaseMigrations is not correct. At the beginning of the test runDatabaseMigrations runs all of the migrations and makes the test database ready. But after the test is finished line 21 is executed which is doctrine:migrations:rollback and it only rollsback the latest migration and all of the other migrations are still in the test database.

I cannot understand why does the last migration have to be rolled back and the rest of them stay?

patrickbrouwers commented 6 years ago

That used to be expected behaviour, rollback should only rollback the last migration. (As there was no way to determine "batches") If you want to rollback all of them you can use doctrine:migrations:reset Do note that this doesn't rollback, but drops the tables.

In case the parent package has modified the behaviour, I'm happy to accept a PR that syncs the console commands.

hshahdoost commented 6 years ago

Sorry, I didn't see that, You are right

hshahdoost commented 6 years ago

@patrickbrouwers There is a difference in the implementation of rollback in LaravelDoctrine and Laravel, I believe it is a bug.

Calling migration:rollback rolls back the last batch of migrations not only one migration, so if we have executed 10 migrations at once by calling rollback all of them would be removed. but doctrine:migrations:rollback only rolls back the last executed migration (as it is using doctrine:migration:execute --down [version]).

So in the case of testing since, We run all migrations at the beginning of our test, Laravel rollback command would clean the database afterward. but Doctrine rollback command only removes the latest migration and leaves the rest of the tables in the database.

It is actually a pretty bad bug, since it won't stop the tests from being executed, It only leave the data in some tables that could make tests fail randomly.

patrickbrouwers commented 6 years ago

@hshahdoost as I said before this package is just a wrapper and we just call the parent package to handle the migrations. We follow doctrine/migrations, not how how Laravel handles migrations; as the philosophy behind them is different. I can't find any "batch" behaviour Laravel has.

I wouldn't call this a bug, as it's just a implementation choice of doctrine/migrations . If you believe it's an important missing feature, I'd suggest opening a ticket at their package, as there isn't a lot we can do over here (https://github.com/doctrine/migrations) .