Does "only mock types you own" imply "only inject types you own" ?

[Blake Boesinger]

Does injecting a type you don't own, for example an XmlSlurper, indicate a design problem?

Should you just wrap it in a type that more closely fits your domain and if so how do you justify the extra level of abstraction?

Also, when should you break the "only mock types you own" rule?

The code in question looks something like the following.

I'm struggling to justify wrapping the xmlSlurper.

class NameChangedToProtectTheInnocentScheduleLoader implements ScheduleLoader {

    def endpoint
    def calendar
    def xmlSlurper

	@Override
    public Schedule load() {
        def yesterday = calendar.yesterday()
        def url = endpoint + yesterday +'.xml'
        def xml = xmlSlurper.parse(url)

		//do stuff with the XML
    }



The test looks something like this

def "should retrieve yesterday's XML schedule from name changed to protect the innocent"() {
		given:
            def calendar = Mock(Calendar)
            calendar.yesterday() >> "06-02-2011"

			def xmlSlurper = Mock(XmlSlurper)
			def path = Mock(GPathResult, constructorArgs: [null, null, null, null])
			xmlSlurper.parse("http://example.org/06-02-2011.xml") >> path
			path.getProperty("ContentStoreID") >> ["test"]

			def loader = new NameChangedToProtectTheInnocentScheduleLoader(calendar: calendar, endpoint: TEST_ENDPOINT, xmlSlurper: xmlSlurper)
		when:
			def schedule = loader.load()
		then:
			assert schedule?.productions[0]?.id == new FileFriendlyId("test")
	}



All the XmlSlurper bits feel wrong.
To fix it do I just wrap it in my own class?
If so, how do I test that? Leave it to the acceptance tests?
It seems tricky to test with an integration test.

[Blake Boesinger]

By 'inject' I mean inject as a dependency through a constructor or a setter method.

[Ben Biddington]

Did you write the test first? Was the XmlSlurper "pulled into existence"?

[Blake Boesinger]

No, the code was given to us from someone who did a spike with no DI or tests. We started by writing an integration test to check we really could talk to the real webservice, then we made the endpoint, calendar and xmlSlurper dependencies to make it into a unit test. Maybe we should have thrown away the spike  :)

[Steve Freeman]

+1

[Steve Freeman]

first reaction, might be worth packaging up the construction of the url into a collaborator. If you also package up the handling of the XML, then maybe you could just write an integration test for the use of the slurper. Just different options...

S

On 9 Jan 2013, at 21:41, Blake Boesinger wrote:
> Does injecting a type you don't own, for example an XmlSlurper, indicate a
> design problem?
> Should you just wrap it in a type that more closely fits your domain and if
> so how do you justify the extra level of abstraction?
>
> Also, when should you break the "only mock types you own" rule?
>
>
> The code in question looks something like the following.
> I'm struggling to justify wrapping the xmlSlurper.
>
> class NameChangedToProtectTheInnocentScheduleLoader implements ScheduleLoader {
>
> def endpoint
> def calendar
> def xmlSlurper
>
> @Override
> public Schedule load() {
> def yesterday = calendar.yesterday()
> def url = endpoint + yesterday +'.xml'
> def xml = xmlSlurper.parse(url)
>

> *//do stuff with the XML*

> }
>
>
>
> The test looks something like this
>
> def "should retrieve yesterday's XML schedule from name changed to protect the innocent"() {
> given:
> def calendar = Mock(Calendar)
> calendar.yesterday() >> "06-02-2011"
>
> def xmlSlurper = Mock(XmlSlurper)
> def path = Mock(GPathResult, constructorArgs: [null, null, null, null])
> xmlSlurper.parse("http://example.org/06-02-2011.xml") >> path
> path.getProperty("ContentStoreID") >> ["test"]
>
> def loader = new NameChangedToProtectTheInnocentScheduleLoader(calendar: calendar, endpoint: TEST_ENDPOINT, xmlSlurper: xmlSlurper)
> when:
> def schedule = loader.load()
> then:
> assert schedule?.productions[0]?.id == new FileFriendlyId("test")
> }
>
>
>
> All the XmlSlurper bits feel wrong.
> To fix it do I just wrap it in my own class?
> If so, how do I test that? Leave it to the acceptance tests?
> It seems tricky to test with an integration test.
>
>

Steve Freeman

Winner of the Agile Alliance Gordon Pask award 2006
Book: http://www.growing-object-oriented-software.com

+44 797 179 4105
Twitter: @sf105
Higher Order Logic Limited
Registered office. 2 Church Street, Burnham, Bucks, SL1 7HZ.
Company registered in England & Wales. Number 7522677

[Ben Biddington]

Can you describe what it does and what it knows in a few sentences? Might help tease out some peers.

[Nat Pryce]

On 9 January 2013 21:47, Blake Boesinger <bl...@boesinger.org> wrote:
> By 'inject' I mean inject as a dependency through a constructor or a setter
> method.

I can see no problems passing a reference to an object that you didn't
write to the constructor of one you did.

After all, if you write an adapter around some third-party object, you
still have to pass that third-party object to the constructor of your
adapter.

--Nat

--
http://www.natpryce.com

[Uberto Barbini]

What it bothers me in this test is that you are mocking 3 quite complex objects.
Wouldn't be better to give to
NameChangedToProtectTheInnocentScheduleLoader the result of xml
parsing instead of the XmlSlurper itself?
What I mean is having some object which determines the next schedule
time from configurations and then pass the date to the scheduleLoader.
If I understand correctly the code.

Btw XmlSlurper is the standard class to do xml parsing in Groovy, if
it's not the same the name is a bit unfortunate.

Uberto

[J. B. Rainsberger]

On Thu, Jan 10, 2013 at 5:55 AM, Uberto Barbini <ube...@ubiland.net> wrote:

What it bothers me in this test is that you are mocking 3 quite complex objects.
Wouldn't be better to give to
NameChangedToProtectTheInnocentScheduleLoader the result of xml
parsing instead of the XmlSlurper itself?

Almost always better. It's the age-old difference between the "Virtual Clock" pattern and the "Instantaneous Request" pattern.

Virtual Clock: pass in an interface to the system clock so that you can stub the time.

Instantaneous Request: give the request a timestamp so that you don't even have to stub the system clock in the first place

At some point, one line of code has to combine "now" with the rest of the request. That line of code ***might*** be Too Simple to Break.

--
J. B. (Joe) Rainsberger :: http://www.myagiletutor.com :: http://www.jbrains.ca ::
http://blog.thecodewhisperer.com
Free Your Mind to Do Great Work :: http://www.freeyourmind-dogreatwork.com

[J. B. Rainsberger]

On Wed, Jan 9, 2013 at 5:41 PM, Blake Boesinger <bl...@boesinger.org> wrote:

 

Does injecting a type you don't own, for example an XmlSlurper, indicate a design problem?

Not on its own.

 

Should you just wrap it in a type that more closely fits your domain and if so how do you justify the extra level of abstraction?

I do this often to remove duplication of irrelevant details in tests.

 

Also, when should you break the "only mock types you own" rule?

I can't imagine summarising this in a concise way. Every time I mock types I don't own, I eventually end up duplicating irrelevent details in tests, and eventually discover a missing abstraction that simplifies everything.

-- 

[James Richardson]

You can also use an offset clock, so that the business rules of the actual date can be encapsulated in another place (which can be tested independently)

class xx {

  Clock clock;

  void foo() {

   dosomethingWith(clock.today());

  }

}

then construct - new xx(new OffsetClock(systemClock, new Duration(-1, DAYS));

You can make these arbitrarily complex... an OrderDateClock might be - if its before 6 am then use yesterday's date, otherwise if its between 6 am and 9 pm then use todays, else if its after 9 pm then use tomorrows date, but the recipient of the clock never knows....

James

[George Dinwiddie]

J.B., Blake,

On 1/10/13 11:41 AM, J. B. Rainsberger wrote:
> On Wed, Jan 9, 2013 at 5:41 PM, Blake Boesinger <bl...@boesinger.org
> <mailto:bl...@boesinger.org>> wrote:
>
> Also, when should you break the "only mock types you own" rule?
>
>
> I can't imagine summarising this in a concise way. Every time I mock
> types I don't own, I eventually end up duplicating irrelevent details in
> tests, and eventually discover a missing abstraction that simplifies
> everything.

The only time I've found it beneficial to mock types I did not own was
when I was constructing a non-trivial adapter around that type. The only
instance I can recall at the moment is the JDBC wrappers I wrote, where
I wanted to verify behavior for all potential issues I could imagine,
whether or not the JDBC objects would actually exhibit these (e.g.,
throwing an exception while closing a ResultSet). See
http://idiacomputing.com/moin/JdbcPersistence for the code & tests.

- George

--
----------------------------------------------------------------------
* George Dinwiddie * http://blog.gdinwiddie.com
Software Development http://www.idiacomputing.com
Consultant and Coach http://www.agilemaryland.org
----------------------------------------------------------------------

[Paul King]

Your production code doesn't type the xmlSlurper so if you aren't using Groovy's static typing capabilities just use a surrogate interface definition. You are relying on a collaborator that has a parse() method so just create an interface:

interface MyXmlSlurper {

    def parse(String url)

}

then you can avoid creating the path result object and let duck typing take care of the rest:

def xmlSlurper = Mock(MyXmlSlurper)

xmlSlurper.parse(TEST_ENDPOINT + "06-02-2011.xml") >> [ContentStoreID: ["test"]]

Cheers, Paul.

[J. B. Rainsberger]

On Thu, Jan 10, 2013 at 2:32 PM, George Dinwiddie <li...@idiacomputing.com> wrote:

 

The only time I've found it beneficial to mock types I did not own was when I was constructing a non-trivial adapter around that type. The only instance I can recall at the moment is the JDBC wrappers I wrote, where I wanted to verify behavior for all potential issues I could imagine, whether or not the JDBC objects would actually exhibit these (e.g., throwing an exception while closing a ResultSet). See http://idiacomputing.com/moin/JdbcPersistence for the code & tests.

In this case, I spend time looking at how confidently I can mock those types accurately enough. In the JDBC case specifically, I'd probably choose to write integrated tests with HSQLDB, then squeeze duplication out of the adapters in order to create ever more types that I own.

To the general concept, a definite "Me, too".

-- 

[J. B. Rainsberger]

On Fri, Jan 11, 2013 at 5:50 AM, Paul King <pa...@asert.com.au> wrote:

Your production code doesn't type the xmlSlurper so if you aren't using Groovy's static typing capabilities just use a surrogate interface definition. You are relying on a collaborator that has a parse() method so just create an interface:

interface MyXmlSlurper {

    def parse(String url)

}

then you can avoid creating the path result object and let duck typing take care of the rest:

def xmlSlurper = Mock(MyXmlSlurper)

xmlSlurper.parse(TEST_ENDPOINT + "06-02-2011.xml") >> [ContentStoreID: ["test"]]

So in a legacy Java system, we could use Groovy to experiment with extracting interfaces for legacy classes without actually extracting the interfaces? That kicks ass. Someone should do this and write about it.

[George Dinwiddie]

J. B.,

On 1/14/13 1:58 PM, J. B. Rainsberger wrote:
> On Thu, Jan 10, 2013 at 2:32 PM, George Dinwiddie
> <li...@idiacomputing.com <mailto:li...@idiacomputing.com>> wrote:
>
> The only time I've found it beneficial to mock types I did not own
> was when I was constructing a non-trivial adapter around that type.
> The only instance I can recall at the moment is the JDBC wrappers I
> wrote, where I wanted to verify behavior for all potential issues I
> could imagine, whether or not the JDBC objects would actually
> exhibit these (e.g., throwing an exception while closing a

> ResultSet). See http://idiacomputing.com/moin/__JdbcPersistence

> <http://idiacomputing.com/moin/JdbcPersistence> for the code & tests.
>
>
> In this case, I spend time looking at how confidently I can mock those
> types accurately enough. In the JDBC case specifically, I'd probably
> choose to write integrated tests with HSQLDB, then squeeze duplication
> out of the adapters in order to create ever more types that I own.

I did write integrated HSQLDB tests. The mocks were principally to throw
exceptions in various circumstances, and also to validate that every
compontent (ResultSet, PreparedStatement, & Connection) was closed, even
in the face of multiple errors. Another test was that in the case of
multiple errors, the first one was reported rather than the last.

I did not try to mock "happy path" functionality.

>
> To the general concept, a definite "Me, too".

:-) I like to hear that.