laravel / ideas

Issues board used for Laravel internals discussions.
938 stars 28 forks source link

[Proposal] Add testing of migration #1074

Open olivernybroe opened 6 years ago

olivernybroe commented 6 years ago

Problem

Testing a migration is close to impossible as there is no way to specify that it should migrate only to a specific migration.

Example

An example scenario where this is needed is for example where you have stored sales in a database and the amount was stored as doubles, now you wan't to change this to BigInt (by multiplying with 100), so you make a migration for doing this, however how do you test that the migration works as expected?

Solution

Add a method in tests like this $this->migrateDatabaseTo(string $class);. (Could use a better name.) This method will migrate the database to a specific class, so we can simply specify that all migrations before and the class itself should be migrated. By using this we can have a test looking like this

        $this->migrateDatabase(\CreateSalesTable::class);
        factory(Sale::class)->create([
            'amount' => 100.33
        ]);
        $this->assertSame(100.33, Sale::all()->first()->amount);

        $this->migrateDatabase(\UpdateSalesToBigInt::class);
        $this->assertSame(10033, Sale::all()->first()->amount);

        $this->artisan('migrate:rollback', ['--rollback' => 1]);
        $this->assertSame(100.33, Sale::all()->first()->amount);
sisve commented 6 years ago

Could you describe your usual workflow for migrations?

All my migrations has already executed in my dev environment. Running php artisan migrate will only execute that one new migration that I am currently writing, and php artisan migrate:rollback will roll it back. Repeating it over and over again, for fun, will change the field to bigint and back in this case. You could just watch the table structure in your favorite database tool to verify that it works properly.

olivernybroe commented 6 years ago

@sisve I am talking about automated test. When using automated tests everything gets migrated as all migrations are new migrations. Therefor rollback will rollback the whole database.

ColinLaws commented 6 years ago

Hm, the only issue I ever have with migrations when testing changes is when I encounter an error that doesn't allow for the migrations to be rolled back by the artisan commands. I guess you could just write a test that spins up a volatile db, and then run the migrations and report back if it's successful or not and just delete the test database once it's done. Honestly you don't really need a new Laravel built-in feature to do that, you could write it pretty easily (but I still like the idea).

olivernybroe commented 6 years ago

@ColinLaws The problem is with your solution you can't really test a scenario like the one I mentioned. In the scenario I mentioned the migration is changing all rows in the database first (eg. by multiply with 100), and then changing the format of the row to ints instead of doubles. However this gives us a problem, because we need some objects in the database when the first migration is done, before we are able to test if the query which changes all the existing doubles to int in our second migration actually works correctly.

ColinLaws commented 6 years ago

Couldn't you just write a test that copies the database if you send it a connection string, or make it use the default one?

Then you could just copy the DB, run a given migration with some kind of structured data specifying the migrations to run, and see if an error is returned?

I don't think this is something Laravel really needs, this really just sounds like you need to write some test code. You shouldn't really be changing major design like that at production anyways.

deleugpn commented 6 years ago

I'm on the fence on this, not really in favour, but also haven't read anything I'm against. Just want to leave my suggestion for the problem.

If I had to do this, I think I would write 2 "unit" tests that doesn't prove anything by themselves, but might prove when executed together. 1st test you swap the Queue for fake on setup before the migrations get executed. Then, you can run a test that just asserts a job is pushed to the Queue after all migrations. Then you write another test that just test the job (multiply the numbers). Inside the migration you dispatch a job and then change the table structure.

epixian commented 4 years ago

all these workarounds for how to do the task seem unnecessarily complicated. a migration is certainly a change condition, and we want to verify that a change happened successfully (especially if there is production data at stake).

being able to migrate up to a certain point (typically being the current state of the production database), save that state as a reference, run the migration(s), and compare the result to the reference could be very useful.

https://stackoverflow.com/q/48931556/4468423