8000 [HttpFoundation] Use `@final` to make `UrlHelper` mockable by HypeMC · Pull Request #49600 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpFoundation] Use @final to make UrlHelper mockable #49600

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[HttpFoundation] Use @final to make UrlHelper mockable #49600

wants to merge 1 commit into from

Conversation

HypeMC
Copy link
Member
@HypeMC HypeMC commented Mar 5, 2023
Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

Similar to #29955 the UrlHelper is a useful class, but mocking it is a PITA. Using the @final annotation should prevent adding an extra maintenance burden while at the same time allowing it to be mocked in tests.

@carsonbot carsonbot added this to the 6.3 milestone Mar 5, 2023
@ro0NL
Copy link
Contributor
ro0NL commented Mar 5, 2023

i'd prefer passing a RequestStack, rather than mocking this derived logic

@HypeMC HypeMC force-pushed the remove-final-from-urlhelper branch from 59a9e3b to d2b546c Compare March 15, 2023 02:22
@derrabus
Copy link
Member

Indeed. I would advise against mocking this class. Instead, you can test against the real thing and provide it with a request object that fits your scenario.

@HypeMC
Copy link
Member Author
HypeMC commented Mar 16, 2023

Thanks for the feedback guys. Honestly, passing the RequestStack is the part that annoys me and since Symfony usually practices not making classes final by default I thought I'd give it a shot to see what others have to say on the issue. But if you think it's not worth it I'm ok with closing this one.

@HypeMC HypeMC deleted the remove-final-from-urlhelper branch March 16, 2023 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Feature HttpFoundation Status: Needs Review
Projects
None yet
Milestone
6.3
Development

Successfully merging this pull request may close these issues.
5 participants
@HypeMC @ro0NL @derrabus @nicolas-grekas @carsonbot
0