-
Notifications
You must be signed in to change notification settings - Fork 492
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
Make DeviceClient
mockable
#110
Comments
Personally, I use own wrapper of |
Regardless of the language, the described problem is a code smell in your application's design. See chapter 8 of Growing Object-Oriented Software, Guided by Tests for reasons to "Only Mock Types That You Own". Larger SDKs (e.g. .NET Framework) are not providing public interfaces unless those interfaces (a) will never change and (b) must be implemented by application code to achieve certain behaviors (e.g. IDisposable). @yfakariya's answer is the correct one: create your own wrapper (facade or adapter). I would also recommend a factory method/class together with the wrapper. That way:
Here are a few online resources: |
@CIPop it is controversial if "Only Mock Types That You Own" is a Code smell, see Stack Overflow: Should you only mock types you own? I have have a azure function which is currently written in Node, but since Node Function have no Claims Support (Azure/azure-functions-nodejs-worker#183) I considered, to move this code to c#. But with adding extra wrapper would blow this little function (39 loc and two method calls to the iothub library) disproportionately. So please reconsider and let the user of the Library decide if they want to mock types they not own. |
It seems to me that creating a wrapper for the sole purpose of unit testing is more of a code smell than mocking an object you don't own. |
Service samples use best practices and dispose
Service samples use best practices and dispose
Could we please have
DeviceClient
implement an interface calledIDeviceClient
so it's mockable for unit testing purposes in downstream projects? Thanks!The text was updated successfully, but these errors were encountered: