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

Fixed rollback process for 2 migrations case #884

Closed

Conversation

tvolf
Copy link

@tvolf tvolf commented Jul 4, 2016

This is a very small fix for such situation when you have exactly 2 migrations in your migrations table and call 'phinx rollback' will rollback not just last one migration but both two migrations at once. I've fixed this case so it will process just one last migration in this case.

@rquadling
Copy link
Collaborator

Good call. Can you add a unit test too?

@tvolf
Copy link
Author

tvolf commented Jul 4, 2016

Hi Richard. Thanks for your comment. Unfortunately I'm not so skillable in unit tests writing. I've tried to run phpUnit to test current phinx code and got many errors and warnings. Here is how it looks at the end of testing process:

...
120) Test\Phinx\Migration\Manager\EnvironmentTest::testExecutingAChangeMigrationDown
PHPUnit_Framework_TestCase::getMock() is deprecated, use PHPUnit_Framework_TestCase::createMock() or PHPUnit_Framework_TestCase::getMockBuilder() instead

ERRORS!
Tests: 522, Assertions: 759, Errors: 72, Warnings: 120, Skipped: 105, Incomplete: 2.

Can you help me with this ? Thanks in advance.

@rquadling
Copy link
Collaborator

Make sure you've got your dependencies up-to-date.

@rquadling
Copy link
Collaborator

What version of PHP are you using?
Also, run composer show to see the version of the dependencies you have.
Then, take a look at the minimum versions in composer.json to make sure you have everything up-to-date.

@rquadling
Copy link
Collaborator

Add this to tests/Phinx/Migration/ManagerTest.php...

    public function testRollbackWithTwoMigrationsDoesNotRollbackBothMigrations()
    {
        // stub environment
        $envStub = $this->getMock('\Phinx\Migration\Manager\Environment', [], ['mockenv', []]);
        $envStub->expects($this->any())
                ->method('getVersionLog')
                ->will(
                    $this->returnValue(
                        [
                            '20120111235330' => ['version' => '20120111235330', 'migration' => '', 'breakpoint' => 0],
                            '20120116183504' => ['version' => '20120815145812', 'migration' => '', 'breakpoint' => 0],
                        ]
                    )
                );
        $envStub->expects($this->any())
                ->method('getVersions')
                ->will(
                    $this->returnValue(
                        [
                            20120111235330,
                            20120116183504,
                        ]
                    )
                );

        $this->manager->setEnvironments(['mockenv' => $envStub]);
        $this->manager->rollback('mockenv');
        rewind($this->manager->getOutput()->getStream());
        $output = stream_get_contents($this->manager->getOutput()->getStream());
        $this->assertNotContains('== 20120111235330 TestMigration: reverting', $output);
    }

Now if you toggle the = in the code, run the tests, you should see this test pass/fail.

As this is a bug, I've fixed this in the dev branch and will be releasing it today.

Thank you for your time. Always come back if you've got any problems.

@rquadling rquadling self-assigned this Jul 4, 2016
@rquadling rquadling added this to the 0.6.3 milestone Jul 4, 2016
@rquadling rquadling closed this in 018a760 Jul 4, 2016
@tvolf
Copy link
Author

tvolf commented Jul 4, 2016

Hi Richard. Thanks for your help. For unit testing running I previously ran "composer install" and for these lines in composer.json:

"require-dev": {
"phpunit/phpunit": "^4.0|^5.0"
},

it was installed last version of phpUnit 5.4.6. And this version returns all of these errors and warnings.
Do you have the same version of phpUnit locally ?

@rquadling
Copy link
Collaborator

There should be no errors (as you can see from https://travis-ci.org/robmorgan/phinx/builds/142254973). There are some PHPUnit related warnings regarding a difference in V4 to V5 ( NOTE: PHPUnit V5 is needed for PHP7).

@tvolf
Copy link
Author

tvolf commented Jul 4, 2016

Thanks once more. It seems I have the same warnings as for PHPUnit v.5.4.6 on Travis-CI screenshots. The errors I had were related to absent of MySQL database phinx_testing. I've created it and now I have just only warning messages.

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

Successfully merging this pull request may close these issues.

2 participants