Skip to content

Conversation

@puzzledpolymath
Copy link
Contributor

See #228

@puzzledpolymath puzzledpolymath requested review from a team as code owners July 9, 2025 14:16
@github-actions github-actions bot added the type: test Test label Jul 9, 2025
@roxblnfk roxblnfk force-pushed the bug/boolean-unsigned branch from c8e89d9 to 0e77852 Compare July 10, 2025 06:11
@github-actions github-actions bot added the MySQL label Jul 10, 2025
@codecov
Copy link

codecov bot commented Jul 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.41%. Comparing base (7ad2df1) to head (0e77852).
Report is 4 commits behind head on 2.x.

Additional details and impacted files
@@            Coverage Diff            @@
##                2.x     #229   +/-   ##
=========================================
  Coverage     95.41%   95.41%           
  Complexity     1891     1891           
=========================================
  Files           131      131           
  Lines          5273     5273           
=========================================
  Hits           5031     5031           
  Misses          242      242           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@roxblnfk
Copy link
Member

Anyway it's good to have such test case 👍

@roxblnfk roxblnfk merged commit 02808e1 into 2.x Jul 10, 2025
30 of 31 checks passed
@roxblnfk roxblnfk deleted the bug/boolean-unsigned branch July 10, 2025 06:20
@puzzledpolymath
Copy link
Contributor Author

@roxblnfk Seeing your comment in the updated test case:

// In case of zerofill=false and unsigned=true the size value might be resolved to 4
$this->assertTrue(\in_array($qux->getSize(), [1, 4], true));

Does this mean the issue still technically exists? Do we also need to account for this when comparing?
See /src/Driver/MySQL/Schema/MySQLColumn.php

    public function compare(AbstractColumn $initial): bool
    {
        $result = parent::compare($initial);

        if ($this->type === 'varchar' || $this->type === 'varbinary') {
            return $result && $this->size === $initial->size;
        }

        if (! $result && $this->userType === 'boolean' && $this->size === 1 && $initial->size === 4) {
            return true; // Handle false positives?
        }

        return $result;
    }

@roxblnfk
Copy link
Member

roxblnfk commented Jul 10, 2025

I'm still thinking about this. Maybe we should keep track of when the size attribute is not defined by introspection, so we can exclude it from the comparison.

getSize() will still return 4 but will be ignored during comparison.

@puzzledpolymath
Copy link
Contributor Author

puzzledpolymath commented Jul 10, 2025

@roxblnfk Your proposal sounds good, I'm unsure what's involved or how to implement what your suggesting. Happy to help out where I can. There's this quick and dirty PR with added test cases #230

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants