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

UserTimeZoneService needs an interface #16613

Closed
deanmarcussen opened this issue Aug 25, 2024 · 5 comments · Fixed by #16614
Closed

UserTimeZoneService needs an interface #16613

deanmarcussen opened this issue Aug 25, 2024 · 5 comments · Fixed by #16614
Milestone

Comments

@deanmarcussen
Copy link
Member

annoying to mock

UserTimeZoneService

@sebastienros
Copy link
Member

What exactly do you need to mock in your tests?
I would have assumed that only ITimeZoneSelector would be useful to change, because only UserTimeZoneDisplayDriver is using UserTimeZoneService.

@deanmarcussen
Copy link
Member Author

and yet we extend orchard core way past what is in the orchard core repository :)

which means we use UserTimeZoneService in many more places than just the UserTimeZoneDisplayDriver

Is that a good thing or not? I don't know, but its easier to use the orchard core services that are available, than copying hte code into our repository

@sebastienros
Copy link
Member

Does Moq support mocking this class? Methods are not overridable so probably no possible?

Adding an interface to a public service would mean that we'll add interface to any public service when someone asks for it. We should have made this type internal if we thought it would not be used externally (and mocked). If mocking works, even if that's not easy, I would not add an interface.

Another point of view is that we may actually want it to be public if someone wants to create a feature enhances/uses this feature. In that case we could add the interface.

@sebastienros sebastienros added this to the 2.x milestone Aug 29, 2024
Copy link
Contributor

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

@hishamco
Copy link
Member

I will try to write a unit test mocking this service

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

Successfully merging a pull request may close this issue.

4 participants