-
-
Notifications
You must be signed in to change notification settings - Fork 31
Boolean comparison #230
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
Boolean comparison #230
Conversation
|
I see you commented out: // if (! $result && $this->userType === 'boolean' && $this->size === 1 && $initial->size === 4) {
// return true; // Ignore size differences for boolean columns when matching defaults
// }I was able to get tests parsing without the code above, however within an application context, using migrations etc changes are continually detected unless a preventative measure like above exists. What's weird is that the following assertions are valid, E.g. the size is different between the column and schema object. $this->assertSame(1, $column->getSize());
$this->assertSame(4, $schema->column('column')->getSize());So with the assertion below, you expect it to return false (without the measures implemented in this PR)?? $this->assertTrue($schema->column('column')->compare($column));[EDIT] $this->assertTrue($column->compare($schema->column('column'))); |
|
Testing locally against this repository, the comparison works fine and I guess that's since public const EXCLUDE_FROM_COMPARE = ['size', 'timezone', 'userType', 'attributes', 'first', 'after'];So perhaps the issue now lies within another repository. |
…-comparison # Conflicts: # tests/Database/Functional/Driver/MySQL/Schema/BooleanColumnTest.php
| public function testBooleanWithProblematicValues(): void | ||
| { | ||
| $schema = $this->schema('table'); | ||
|
|
||
| $column = $schema->boolean('target') | ||
| ->defaultValue(false) | ||
| ->nullable(false) | ||
| ->unsigned(true) | ||
| ->comment('Target comment'); | ||
|
|
||
| $schema->save(); | ||
|
|
||
| $this->assertTrue($schema->exists()); | ||
|
|
||
| $schema = $this->schema('table'); | ||
| $target = $schema->column('target'); | ||
|
|
||
| $this->assertSame(1, $column->getSize()); | ||
| $this->assertSame(4, $target->getSize()); | ||
| $this->assertFalse($column->isNullable()); | ||
| $this->assertFalse($target->isNullable()); | ||
| $this->assertTrue($column->isUnsigned()); | ||
| $this->assertTrue($target->isUnsigned()); | ||
|
|
||
| $object = new \ReflectionObject($target); | ||
| $property = $object->getProperty('defaultValue'); | ||
| $property->setAccessible(true); | ||
| $defaultValue = $property->getValue($target); | ||
|
|
||
| $this->assertSame(false, $column->getDefaultValue()); | ||
| $this->assertSame(0, $target->getDefaultValue()); | ||
| $this->assertSame('0', $defaultValue); | ||
| $this->assertTrue($column->compare($target)); | ||
| } |
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.
@roxblnfk It took a while! But here is the test case which confirms the issue, under very specific conditions and attributes defined. Having a comment somehow contributes to the problem.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 2.x #230 +/- ##
=========================================
Coverage 95.41% 95.41%
- Complexity 1891 1893 +2
=========================================
Files 131 131
Lines 5273 5279 +6
=========================================
+ Hits 5031 5037 +6
Misses 242 242 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hopefully there's not too much push-back or changes required, so this can make it's way into the 2.14.0 release |
Made conditional inline
|
Please don't merge PRs. We follow conventional commits and need to adhere to certain rules when merging. |


Quick fix for ignoring size differences for boolean columns, when matching defaults.
Without this or a similar fix, migrations will continually find unnecessary changes in the schema.