8000 [DoctrineBridge] fix tests with Doctrine ORM 3.4+ on PHP < 8.4 by xabbuh · Pull Request #60088 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

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

New issue 8000

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

Merged
merged 1 commit into from
Mar 31, 2025

Conversation

xabbuh
Copy link
Member
@xabbuh xabbuh commented Mar 30, 2025
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
Copy link
Member Author
xabbuh commented Mar 31, 2025

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.

Copy link
Contributor
@Spomky Spomky left a comment

Choose a reason for hiding this comment

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

Minor changes


public function findByCustom()
{
return self::$result;
Copy link
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
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
Member

Thank you @xabbuh.

@nicolas-grekas nicolas-grekas merged commit aa0b1ba into symfony:6.4 Mar 31, 2025
5 of 11 checks passed
@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
0