?

Should an IDisposable object call Dispose on the IDisposable objects that it holds?

?

From: testdrivendevelopment@... [mailto:testdrivendevelopment@...] On Behalf Of David Burstin
Sent: Sunday, February 2, 2014 7:25 PM
To: testdrivendevelopment@...

Subject: Re: [TDD] How do I unit test a Dispose method ?

?

?

Hi all. Long time listener, first time caller :)

?

I've just finished a blog post called?

?

As IDisposable is primarily about managing internal resources, I agree with Amir that the best approach is to subclass, which allows us to spy on those resources. But I would not inject the resources to spy on as only the owner of the resource is responsible for its disposal - in this case the owner would be the test, not the SUT.

?

?

?

On 24 January 2014 19:28, Donaldson, John <john.m.donaldson@...> wrote:

?

.a. Yes, you are right it's an implementation detail.
Would another way make things easier?

For example, separate the storing of the password from the Connection.
You might delegate to the PasswordStore to hold and remove passwords.
Then you just need to show that:
- the PasswordStore is wired up to the Connection
- the PasswordStore does what you want.

John D.

-----Original Message-----
From: testdrivendevelopment@... [mailto:testdrivendevelopment@...] On Behalf Of Amir Kolsky
Sent: 24 January 2014 03:07
To: testdrivendevelopment@...
Subject: RE: [TDD] How do I unit test a Dispose method ?

Here's another way of thinking about this - Whether a member is IDisposable or not is really an implementation issue (as which members comprise an object is private).
So the general question here is how do we get an object to identify all of its IDisposable private methods and invoke Dispose on them in its Dispose.
There are several ways I can think of doing this, all of which are sort of terrible, but then, we're dealing with a true implementation specific problem.

The underlying technique in all cases is going to be the injection of two more members which are IDisposable mocks.

We can try to .emit code at runtime that will add these mocks to the UUT, but - ugh!
Partial classes could work, but then we'd have to mark the original as partial, so - ugh!
The easiest is to subclass the original class and have the (mock) subclass have two (mock) members whose Dispose() must be called. The implementation would, naturally, have to use reflection to find all members and Dispose() of them properly, but that's what we want to see anyway.

The price to pay is that the class cannot be sealed, but that is really not a problem as only API classes should be sealed anyway and if you class serves both as an API and it actually does anything, shame on you :)

.a.

From: testdrivendevelopment@...testdrivendevelopment@...> [mailto:testdrivendevelopment@...] On Behalf Of Roy Osherove
Sent: Thursday, January 23, 2014 4:18 AM
To: testdrivendevelopment
Subject: Re: [TDD] How do I unit test a Dispose method ?

Isn't there already an IDisposable interface? which means you can easily create an IDisposable MOCK object and check if dispose was called on it.
all you would do it then have casted IDisposable in the class level that you would access from the dispose method.

public Connection(SecureString password) { IDisposable _myDisposablePassword;
_myPassword = password.Copy(); // creates a clone of the password which needs to be disposed _myDisposablePassword = _myPassword; } public void Dispose() {
_myDisposablePassword.Dispose(); // releases any resources it holds } On Thu, Jan 23, 2014 at 12:06 PM, Gishu Pillai <gishu.pillai@...gishu.pillai@...>> wrote:

public Connection(SecureString password) {
_myPassword = password.Copy(); // creates a clone of the password which needs to be disposed } public void Dispose() {
_myPassword.Dispose(); // releases any resources it holds }

SecureString is a .Net framework type. I could wrap it in an adapter to ease testing...but doesn't solve the general problem. Wrapping all member disposable type with interfaces is going to be tedious.
It seems this is a better fit for Static code analysis.
Gishu

--
Thanks,

Roy Osherove

- @RoyOsherove<>
- Read my new book Notes to a Software Team Leader<>
- My blog for team leaders:
- +47-96-90-22-15<tel:%2B47-96-90-22-15> (Oslo Time)

[Non-text portions of this message have been removed]

------------------------------------

Yahoo Groups Links

?

?

.a. Yes, you are right it's an implementation detail.
Would another way make things easier?

For example, separate the storing of the password from the Connection.
You might delegate to the PasswordStore to hold and remove passwords.
Then you just need to show that:
- the PasswordStore is wired up to the Connection
- the PasswordStore does what you want.

John D.

-----Original Message-----
From: testdrivendevelopment@... [mailto:testdrivendevelopment@...] On Behalf Of Amir Kolsky
Sent: 24 January 2014 03:07
To: testdrivendevelopment@...
Subject: RE: [TDD] How do I unit test a Dispose method ?

Here's another way of thinking about this - Whether a member is IDisposable or not is really an implementation issue (as which members comprise an object is private).
So the general question here is how do we get an object to identify all of its IDisposable private methods and invoke Dispose on them in its Dispose.
There are several ways I can think of doing this, all of which are sort of terrible, but then, we're dealing with a true implementation specific problem.

The underlying technique in all cases is going to be the injection of two more members which are IDisposable mocks.

We can try to .emit code at runtime that will add these mocks to the UUT, but - ugh!
Partial classes could work, but then we'd have to mark the original as partial, so - ugh!
The easiest is to subclass the original class and have the (mock) subclass have two (mock) members whose Dispose() must be called. The implementation would, naturally, have to use reflection to find all members and Dispose() of them properly, but that's what we want to see anyway.

The price to pay is that the class cannot be sealed, but that is really not a problem as only API classes should be sealed anyway and if you class serves both as an API and it actually does anything, shame on you :)

.a.

From: testdrivendevelopment@...testdrivendevelopment@...> [mailto:testdrivendevelopment@...] On Behalf Of Roy Osherove
Sent: Thursday, January 23, 2014 4:18 AM
To: testdrivendevelopment
Subject: Re: [TDD] How do I unit test a Dispose method ?

Isn't there already an IDisposable interface? which means you can easily create an IDisposable MOCK object and check if dispose was called on it.
all you would do it then have casted IDisposable in the class level that you would access from the dispose method.

public Connection(SecureString password) { IDisposable _myDisposablePassword;
_myPassword = password.Copy(); // creates a clone of the password which needs to be disposed _myDisposablePassword = _myPassword; } public void Dispose() {
_myDisposablePassword.Dispose(); // releases any resources it holds } On Thu, Jan 23, 2014 at 12:06 PM, Gishu Pillai <gishu.pillai@...gishu.pillai@...>> wrote:

public Connection(SecureString password) {
_myPassword = password.Copy(); // creates a clone of the password which needs to be disposed } public void Dispose() {
_myPassword.Dispose(); // releases any resources it holds }

SecureString is a .Net framework type. I could wrap it in an adapter to ease testing...but doesn't solve the general problem. Wrapping all member disposable type with interfaces is going to be tedious.
It seems this is a better fit for Static code analysis.
Gishu

--
Thanks,

Roy Osherove

- @RoyOsherove<>
- Read my new book Notes to a Software Team Leader<>
- My blog for team leaders:
- +47-96-90-22-15 (Oslo Time)

[Non-text portions of this message have been removed]

------------------------------------

Yahoo Groups Links

?

public Connection(SecureString password)
{

? _myPassword = password.Copy(); // creates a clone of the password which needs to be disposed

}

public void Dispose()
{

? _myPassword.Dispose(); // releases any resources it holds

}

SecureString is a .Net framework type. I could wrap it in an adapter to ease testing...but doesn't solve the general problem. Wrapping all member disposable type with interfaces is going to be tedious.

It seems this is a better fit for Static code analysis.

Gishu

?

The question is a bit .Net specific.
Assuming the simple case, where I have a type which wraps a internal IDisposable member.

class Connection
{

?? private SecureString password;

}

SecureString is Disposable. That mandates that Connection should have a Dispose method, which should dispose its members.

Options:
* To know whether the internal member has been disposed, I'd have to expose it (so that the unit test can see it). Doesn't feel right to me.

* The other hack would be to promote it from private to internal + use the InternalVisibleTo path to access it.

Does anyone have something better ?

Gishu

?

On 17 January 2014 10:25, Angel Java Lopez <ajlopez2000@...> wrote:

Interesting thread!

Regarding

def exists(item_id):
? ? xdep.open() ?# opens some remote resource

? ? item = xdep.get_item(item_id)
? ? xdep.close()
? ? if item:
? ? ? ? return??== item_id
? ? else:
? ? ? ? return false

I usually write a xdep alternative implementation. I don't call it a mock, because is an implementation of the same service, but local. And I use it even in the final application (for first iteration, for demo purpose). You can build it using TDD, and then, inject it to the exists() test. It has no additional method like "expected" nor other mockist methods.

It makes sense to me, but if we forget Python's module name of unittest.mock uses word 'mock',

then the only difference at low level of how xdep double is implemented:

- you would write by hand an alternative implementation of xdep

- I use Mock class to automate things

# mock will generate implementation for me

self.xdep = mock.Mock()

# I ask the object of Mock class to 'implement get_item for me so it returns 1'

self.xdep.get_item.return_value = 1 ?

but net result (and effect) is the same, I believe.

The details how the 'magic' happens, let someone write by hand or let something generate it, are IMO not important for my question

which Roy has nicely captured in?

"I would use a STUB for a get_something if not faking it meant an integration test"

How the get_something is faked, that is a different kind of issue.

Best regards,

--
Mateusz ??oskot,

?

P.S. in the first sentence of that second paragraph I should have said you stub the method and assert something about the SUT, only. In other words:

For a query:

Arrange: stub the query and pass the stub to the object under test

Act: call the method on the object under test that will cause the stubbed method to be queried

Assert: assert something about the object under test that should only be true if the stubbed method returns correctly

For a command:

Arrange: create a mock and pass it to the object under test *

Act: call the method on the object under test that will cause the method on the mock to be called

Assert: verify that the mock was called correctly which should only be true if the object under test did what we expected

* depending on the framework you may also have to specify how the mock is expected to be called before you call it.?

On Wed, Jan 15, 2014 at 3:20 PM, Adam Sroka <adam.sroka@...> wrote:

Are you familiar with Command Query Separation (CQS)? I've always found that the most coherent and useful way to explain why you would choose a mock or a stub. The most interesting thing about a method is either what it returns or, in the case where you don't care what it returns, whether it was called with the correct semantics, not both.?

So, if the SUT cares about the value that a method returns then you stub that method, only. If the SUT doesn't care about the value a method returns then you use a mock and verify the call has the expected semantics, only.?

Does that help??

On Tue, Jan 14, 2014 at 6:37 AM, Mateusz ?oskot <mateusz@...> wrote:

?

Hi,

I'd like to ask for ideas of best practices in case of developing and testing
a wrapper for a Python legacy module.
Especially, about applying the single assert / single mock rule
which I have been learning about from numerous books/videos/blogs.

I have Python xdep module which exposes API in form of bunch of free functions.
I'm creating new Python module foo which uses xdep (the idea is to provide
simplified, Pythonic interface, but that is not directly relevant to
my question here).

I'm trying to keep the code as simple & generic as possible, so my question is
not about Python but unit testing and mocking.

# xdep.py
def open():
...
def get_item(item_id):
...
def close():
...

# foo.py
import xdep

def exists(item_id):
xdep.open()
item = xdep.get_item(item_id)
xdep.close()
if item:
return == item_id
else:
return false

Here is my module to unit test behaviuour of the foo:
- I create and plug a stub for the legacy module xdep
- I mock functions of xdep

# test_foo.py
import unittest
import unittest.mock as mock

class ItemTest(unittest.TestCase):
def setUp(self):

# create stub for external dependency of foo
self.xdep_stub = mock.Mock()
patcher = mock.patch.dict('sys.modules', {'xdep': self.xdep_stub})
self.addCleanup(patcher.stop)
patcher.start()

# import foo does import xdep, as patched above
import foo
self.foo = foo

def test_exists_ExistingItemId_ReturnTrue(self):
foo = self.foo
# Arrange
### Data
item_id = 1
### Mock
foo.xdep.get_item.return_value = 1
# Act
item_exists = foo.item.exists(item_id)
# Assert
self.assertTrue(item_exists)

I have a single assertion and I use single mock which mocks xdep.get_item
function, so it seems a valid yet clear unit test.

Now, having the single assert/single mock rule in mind, I have some questions,
about the test_exists_ExistingItemId_ReturnTrue in particular:

1) How asserting on foo.item.exists return value is different from
asserting how xdep.get_item mock was called?

I could have replaced
self.assertTrue(item_exists)
with
foo.xdep.get_item.assert_called_once_with(1)

2) What about having both assertions, would that break the single
assertion rule?

self.assertTrue(item_exists)
foo.xdep.get_item.assert_called_once_with(1)

I'm not concern about breaking the rule as technical convention, but as:
am I testing multiple things here?

3) AFAIU, it's natural that unit tests are coupled with implementation of
method/behaviour they are testing, foo.exists function in my case.

So, given that foo.exists function calls three functions of the legacy
xdep module
xdep.open
xdep.get_item
xdep.close
should I mock all the three function and verify how they were called?
Would that still belong to scope of test_exists_ExistingItemId_ReturnTrue test
or I better create a new dedicated test case(s), something like
test_exists_IfCalled_xdepOpenCalled
test_exists_IfCalled_xdepCloseCalled
test_exists_IfCalled_xdepGetItemCalled
and do assert_called_once_with() appropriately.

Above, I count each mock for each xdep function as a separate mock,
so "single mock per test" rule seems to suggests I should verify they are called
in separate tests, doesn't it?

Also, some of other xdep functions need to be called with multiple parameters,
and as setting up and verifying their mocks may be quite elaborate,my
gut feeling
tells me that single unit test per "legacy API call expectation" as listed above
is a good approach for readability and maintainability of tests.
Am I right?

4) What other, if any, approach to single assert/single mock rules
would you take for testing similar case as above?

Best regards,
--
Mateusz ?oskot,