-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Test and fix for duplicate foreign key entries #2667
Conversation
@Ocramius don't know why the tests have failed exactly btw, but it does not seem to relate to code change errors based on what I can find in travis logs |
0873a65
to
fb51207
Compare
/** | ||
* @group DBAL-2576 | ||
*/ | ||
public function testNotDuplicateDropForeignKeySql() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@websmurf as far as I see, this test passes without the code changes introduced in this pull request and doesn't even hit the changed code. Is it intended?
|
||
// Assert that there are no duplicates in the results | ||
$this->assertSame([ | ||
'ALTER TABLE documents DROP FOREIGN KEY documents_ibfk_1', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that the set of expected statements is different depending on the platform, I believe this feature should be tested in a functional way (statements are executed on a real DB) rather on the SQL level. This way, every DB platform will prove the correctness of the generated statements.
$documents->setPrimaryKey([ 'id' ]); | ||
|
||
// Added foreign key | ||
$diff->addedForeignKeys['FK_a788692ffa0c224'] = $documents->getForeignKey('FK_a788692ffa0c224'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not what real DBAL usage looks like. The comparator should be used to produce the diff of two given schemas, the diff shouldn't be built in parallel with the schema(s).
In relation to issue #2576 I've created a test that covers this scenario