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

Update the tests so they pass again following the release of phinx 0.4.6 #123

Merged
merged 3 commits into from
Sep 30, 2015

Conversation

HavokInspiration
Copy link
Member

This should not be merged right now

This phinx release and this PR shed some light on some migrations tests that were not done. Now that they are all running, I spotted a bug in phinx implementation of the mechanism to drop foreign key constraints with SQLite. I submitted a PR on phinx repo : cakephp/phinx#641

The test suite will be failing for SQLite until the change I will propose will be merged and a new phinx release is


This phinx release introduced changes in the naming scheme the migration files / the migration classes should follow, breaking the tests.

This commit also removes the update() call after the dropForeignKey() calls : it was not necessary.

@HavokInspiration HavokInspiration added this to the 1.3.1 milestone Sep 12, 2015
This phinx release introduced changes in the naming scheme the migration files / the migration classes should follow, breaking the tests.

 This commit also removes the ``update()`` call after the ``dropForeignKey()`` calls : it was not necessary.
@HavokInspiration
Copy link
Member Author

I pushed a new commit that contains a few things.

First of all, since nothing has moved on phinx's side, I decided to implement a temporary fix to fix the cause of the tests failures with SQLite. I'll remove it once (and if) the PR on phinx repo is merged.

While doing that, I noticed a defect : the Migrations class was not using the CakeAdapter but the Adapter from phinx which prevented the bugfix to fix anything. So I fixed this along the way.

Tests should be passing again now.

…ry fix to the SQLite adapter to prevent it from dropping tables when using dropForeignKey

Also change implementation on code tested using the Migrations class (which used to use the Phinx adapter and not the Cake one, leading to unexpected results when the Cake one is used)
@HavokInspiration
Copy link
Member Author

PR on phinx has been merged. We just need a release now for the tests to pass. I updated the code to remove the temp fix I added.

@HavokInspiration
Copy link
Member Author

I pushed a commit where I change the dependency for phinx to dev-master. This way we have the merged changes from phinx and we can move on with this PR, #125 (after a rebase and a travis check), #62 (I might add some tests for this), target the min requirement for cake to 3.1 and start working on #128.

What do you think ?
This can be reverted when a new release of phinx is done.

@lorenzo
Copy link
Member

lorenzo commented Sep 30, 2015

I think that plan makes sense @HavokInspiration

lorenzo added a commit that referenced this pull request Sep 30, 2015
Update the tests so they pass again following the release of phinx 0.4.6
@lorenzo lorenzo merged commit eacaa3c into cakephp:master Sep 30, 2015
@HavokInspiration HavokInspiration deleted the tests-phinx-0-4-6 branch November 30, 2015 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants