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

4.next: add PHPUnit 11 support #736

Merged
merged 8 commits into from
Aug 11, 2024
Merged

4.next: add PHPUnit 11 support #736

merged 8 commits into from
Aug 11, 2024

Conversation

LordSimal
Copy link
Contributor

This is a huge PR, so i tried to split things up via the commits:

Part 1: Fix PHPUnit Deprecations and convert mock classes to anonymous classes
Part 2: Fix small Migration specific deprecations
Part 3: Transform DataProviders and remove all Covers annotations
Part 4: Fix Depends annotations

I also noticed, that our CS fixer doesn't care / automatically fix annotations if they are written like this so this is something for our CS-Fixer

#[DataProvider('defaultsCastAsExpressions')] public function testDefaultsCastAsExpressionsForCertainTypes

@LordSimal LordSimal added this to the 4.x (CakePHP 5) milestone Aug 9, 2024
use Migrations\Db\Table\Index;
use Migrations\Db\Table\Table;

trait DefaultPdoAdapterTrait
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a few places where we need a custom PDO instance which only returns specific values in specific cases. So I just added this trait to more easily make it possible to "mock" those instances without having to re-define all those methods.

@LordSimal
Copy link
Contributor Author

LordSimal commented Aug 9, 2024

The tests failing for MySQL indicates, that the changes in the composer.json are causing this...
My guess would be something in core 5.next

Current 4.next of this repo is running fine: https://github.com/cakephp/migrations/actions/runs/10326114754

@LordSimal
Copy link
Contributor Author

LordSimal commented Aug 10, 2024

I found the Problem: inside our Migrations\Util\TableFinder::getTableNames we check only the loaded plugins, no matter if the table class is present or not.

I remember there was something related to how plugins are loaded inside tests so the easiest fix was to load that test plugin inside that specific mysql test.

And I guess this doesn't fail for the other DBMS's because we don't have the same test for the other versions.

Comment on lines +23 to +25
$this->adapter = new class (['foo' => 'bar', 'version_order' => Config::VERSION_ORDER_CREATION_TIME]) extends PdoAdapter {
use DefaultPdoAdapterTrait;
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 This will be easier to maintain than phpunit mocks have been.

@markstory markstory merged commit 9440f8d into 4.next Aug 11, 2024
16 checks passed
@markstory markstory deleted the 4.next-phpunit11 branch August 11, 2024 16:08
@markstory
Copy link
Member

I found the Problem: inside our Migrations\Util\TableFinder::getTableNames we check only the loaded plugins, no matter if the table class is present or not.

This has caused problems in the past as well. But scanning plugins that aren't loaded isn't something we do generally. I think the correct solution is to update the tests as you've done.

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