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

Prevent multiple migrate:rollback on beforeApplicationDestroyed #168

Closed
antonkomarev opened this issue Feb 20, 2017 · 9 comments
Closed
Labels
bug Something isn't working

Comments

@antonkomarev
Copy link
Contributor

antonkomarev commented Feb 20, 2017

If loadMigrationsFrom method will be called multiple times - test bench will crash on the rollback. Because it's registering this action twice. First rollback will be successful and the second one will crash an app.

$this->beforeApplicationDestroyed(function () use ($options) {
    $this->artisan('migrate:rollback', $options);
});

Use case:

// Migrating package migrations
$this->loadMigrationsFrom([
    '--realpath' => database_path('migrations'),
]);

// Migrating unit test migrations
$this->loadMigrationsFrom([
    '--realpath' => realpath(__DIR__ . '/database/migrations'),
]);
@crynobone
Copy link
Member

If you have a solution for this, feel free to submit a PR.

@crynobone crynobone added open for contribution bug Something isn't working labels Feb 20, 2017
@antonkomarev
Copy link
Contributor Author

Tried to fix this but no luck still. Will try to find the way to solve this later this week because it's blocking the upgrade of some of my packages.

@crynobone
Copy link
Member

Could you do me a favour and try with the following changes:

    /**
     * Register a callback to be run before the application is destroyed.
     *
     * @param  callable  $callback
     *
     * @return void
     */
    protected function beforeApplicationDestroyed(callable $callback)
    {
        array_unshift($this->beforeApplicationDestroyedCallbacks, $callback);
    }

@antonkomarev
Copy link
Contributor Author

antonkomarev commented Feb 20, 2017

@crynobone It works! I haven't checked if database migrations are rolled back correctly between tests yet, but no errors with this fix.

@antonkomarev
Copy link
Contributor Author

Should I make a PR for it or you will do it?

@crynobone
Copy link
Member

That's is good news, use that first on your projects and feel free to submit a PR once you're confident with the fix.

To confirm, the following tests in the same class shouldn't have any data (created from factory) from the previous tests.

@antonkomarev
Copy link
Contributor Author

@crynobone Thanks for a such a quick resolution as always!

@crynobone
Copy link
Member

Should I make a PR for it or you will do it?

Everything works great right? can you make the PR to branch 3.1 for this

@antonkomarev
Copy link
Contributor Author

antonkomarev commented Feb 24, 2017

@crynobone Yes. It works well for my case. Why to 3.1? I thought it should be pointed to 3.4.

Edit: Ah... I see this behavior is the same in 3.1 too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants