Skip to content

[DoctrineBridge] fix tests with Doctrine ORM 3.4+ on PHP < 8.4#60088

Merged
nicolas-grekas merged 1 commit into
symfony:6.4from
xabbuh:doctrine-orm-11853
Mar 31, 2025
Merged

[DoctrineBridge] fix tests with Doctrine ORM 3.4+ on PHP < 8.4#60088
nicolas-grekas merged 1 commit into
symfony:6.4from
xabbuh:doctrine-orm-11853

Conversation

@xabbuh

@xabbuh xabbuh commented Mar 30, 2025

Copy link
Copy Markdown
Member
Q A
Branch? 6.4
Bug fix? no
New feature? no
Deprecations? no
Issues
License MIT

The solution implemented in #59887 does no longer work on PHP < 8.4 with the changes made in doctrine/orm#11853.

@carsonbot carsonbot added this to the 6.4 milestone Mar 30, 2025
@xabbuh
xabbuh force-pushed the doctrine-orm-11853 branch 3 times, most recently from 2d9df3d to 521ab70 Compare March 31, 2025 07:17
@xabbuh

xabbuh commented Mar 31, 2025

Copy link
Copy Markdown
Member Author

I have pushed another commit which rewrites the test a bit to no longer mess around with Doctrine ORM internals, but provide a custom repository instead which provides the results required by the test methods.

@Spomky Spomky left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor changes

Comment thread src/Symfony/Bridge/Doctrine/Tests/Fixtures/SingeIntIdEntityRepository.php Outdated

public function findByCustom()
{
return self::$result;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why a static property instead of a setter?
It may lead to unreproductible tests not not correctly set between test runs right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's a valid concern and @alexandre-daubois shared the same thoughts. I rewrote the code a bit to use a public instance variable instead.

@xabbuh
xabbuh force-pushed the doctrine-orm-11853 branch from 521ab70 to 9be0d0a Compare March 31, 2025 07:48
@nicolas-grekas

Copy link
Copy Markdown
Member

Thank you @xabbuh.

@nicolas-grekas
nicolas-grekas merged commit aa0b1ba into symfony:6.4 Mar 31, 2025
@xabbuh
xabbuh deleted the doctrine-orm-11853 branch March 31, 2025 08:19
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.

5 participants