Using Mocks in Tests

Question

I recently started using layouts (using Java mockito) in my tests. Needless to say, they simplified the installation part of the tests and together with Dependency Injection, I would say that he made the code even more reliable.

However, I found myself disconnecting during testing from implementation, and not from the specification. I ended up creating expectations, which I would say is not part of the tests. In more technical terms, I will test the interaction between the SUT (test class) and its coauthors, and such a dependency is not part of the class contract or interface!

Note that you have the following: When working with XML node, suppose you have an attributeWithDefault() method that returns the value of the node attribute, if available, otherwise it will return the default value!

I would set the test as follows:

 Element e = mock(Element.class); when(e.getAttribute("attribute")).thenReturn("what"); when(e.getAttribute("other")).thenReturn(null); assertEquals(attributeWithDefault(e, "attribute", "default"), "what"); assertEquals(attributeWithDefault(e, "other", "default"), "default"); 

Well, I not only verified that attributeWithDefault() adheres to the specification, but I also tested the implementation since I needed to use Element.getAttribute() instead of Element.getAttributeNode().getValue() or Element.getAttributes().getNamedItem().getNodeValue() etc.

I assume that I'm going to do it wrong, so all the tips on how I can improve my use of layouts and best practices will be appreciated.

EDIT: What's wrong with the test

I made the assumption above that the test is bad style, here is my rationale.

• The specification does not indicate which method is being called. The library client does not have to care about how the attribute is retrieved, for example, if this is done correctly. The developer must be fluent in order to access any of the alternative approaches, in any way he considers necessary (in terms of productivity, consistency, etc.). This is an Element specification that ensures that all of these approaches return identical values.

• It makes no sense to refactor Element into a single method interface using getElement() (in fact, Go is pretty good). For ease of use, the method client should be able to use the standard Element in the standard library. Interfaces and new classes are just stupid, IMHO, because it makes the client code ugly, and it's not worth it.

• Assuming that the specification remains as it is and the test remains as it is, the new developer may decide to reorganize the code to use a different approach to using the state and cause the test to fail! Well, an untrue test is true.

• Having a collaborator’s state in several formats is fairly common. The specification and test should not depend on which specific approach is used; only implementation should be!

Answer 1

This is a common problem when testing the layout, and a common mantra to get away from this:

Only the layout of the types you own .

Here, if you want to mock collaboration with an XML parser (it’s not necessary, to be honest, how a small test XML file should work fine in a single context), then this XML parser should be behind an interface or a class that you own will handle messy Information about which method you use for the third-party API. The main thing is that it has a method that receives an attribute from an element. The layout of this method. This separates implementation from design. The actual implementation will have a real unit test, which actually verifies that you are getting a successful element from the real object.

Mocks may be a good way to save some template customization code (acting essentially like Stubs), but this is not their primary goal in terms of driving design. Mocks tests behavior (as opposed to state) and not Stubs .

I should add that when using Mocks as stubs, they look like your code. Any stub should make assumptions about how you are going to call it, which are tied to your implementation. This is normal. Where this is a problem is if you are poorly managing your design.

Answer 2

When developing unit tests, you will always effectively test your implementation, and not some abstract specification. Or it can be argued that you will check the “technical specification”, which is a technical specification supplemented by technical details. There is nothing wrong. Instead of testing this:

My method will return a value if it is defined or by default.

you are testing:

My method will return a value if it is defined or by default, provided that the provided XML element will return this attribute when I call getAttribute (name).

Answer 3

The only solution that I see for you here (and I must admit that I am not familiar with the library that you use) is to create a layout element that includes all the functionality, that is, also the ability to set getAttributeNote (). getValue () and getAttributes (). getNamedItem (). getNodeValue ().

But, assuming that they are all equivalent, just check them out. When it changes, you need to check all cases.

Answer 4

I find nothing wrong with your use of layouts. What you are testing is the attributeWithDefault() method and its implementation, and not whether the Element correct or not. Thus, you mocked Element to reduce the required setup. The test ensures that the implementation of attributeWithDefault() complies with the specification, of course, there must be some specific implementation that can be run for the test.

Answer 5

You are effectively checking your layout. If you want to test the attributeWithDefault() method, you must argue that e.getAttribute() is called with the expected argument and forgets about the return value. This return value only checks the setting of your mock object. (I don't know how to do this with Java mockito, I am a pure C # guy ...)

Answer 6

This depends on whether getting the attribute through a call to getAttribute () is part of the specification, or if it is an implementation detail that may change.

If Element is an interface, than stating that you should use 'getAttribute' to get an attribute is probably part of the interface. So your test is fine.

If the Element is a concrete class, but the WhithDefault attribute does not need to know how you can get this attribute, there may be an interface waiting to appear here.

 public interface AttributeProvider { // Might return null public String getAttribute(String name); } public class Element implements AttributeProvider { public String getAttribute(String name) { return getAttributeHolder().doSomethingReallyTricky().toString(); } } public class Whatever { public String attributeWithDefault(AttributeProvider p, String name, String default) { String res = p.getAtribute(name); if (res == null) { return default; } } } 

Then you should check the attributeWithDefault on the Mock AttributeProvider instead of the element.

Of course, in this situation this will probably be redundant, and your test is probably very good even when implemented (you still have to test it somewhere;)). However, this decoupling can be useful if the logic is ever complicated, either in getAttribute or in the attributeWithDefualt.

Hoping this helps.

Answer 7

It seems to me that there are three things you want to check with this method:

• It gets the attribute from the right place (Element.getAttribute ())
• If the attribute is not null, it is returned
• If the attribute is null, the string "default" is returned

You are currently checking # 2 and # 3, but not # 1. With mockito you can check # 1 by adding

 verify(e.getAttribute("attribute")); verify(e.getAttribute("other")); 

This ensures that the methods are actually called on your layout. Admittedly, this is a bit awkward at mockito. In easymock, you would do something like:

 expect(e.getAttribute("attribute")).andReturn("what"); expect(e.getAttribute("default")).andReturn(null); 

It has the same effect, but I think your test is a little easier to read.

Answer 8

If you use dependency injection, collaborators should be part of the contract. You should be able to introduce all collaborators through a constructor or public property.

Bottom line: if you have a collaborator who is new to injecting, you probably have to reorganize the code. This is a change in thinking necessary for testing / bullying / injection.

Answer 9

This is a late answer, but it takes a different point of view from others.

In principle, the OP is right in thinking that the mock test is bad, for the reasons that he outlined in the question. Those who say that the mafia is in order did not give a good reason for this, IMO.

Here is the full version of the test in two versions: one with mockery (BAD) and the other without (GOOD). (I allowed another mocking library to be used, but that doesn't change the point.)

 import javax.xml.parsers.*; import org.w3c.dom.*; import org.junit.*; import static org.junit.Assert.*; import mockit.*; public final class XmlTest { // The code under test, embedded here for convenience. public static final class XmlReader { public String attributeWithDefault( Element xmlElement, String attributeName, String defaultValue ) { String attributeValue = xmlElement.getAttribute(attributeName); return attributeValue == null || attributeValue.isEmpty() ? defaultValue : attributeValue; } } @Tested XmlReader xmlReader; // This test is bad because: // 1) it depends on HOW the method under test is implemented // (specifically, that it calls Element#getAttribute and not some other method // such as Element#getAttributeNode) - it therefore refactoring-UNSAFE; // 2) it depends on the use of a mocking API, always a complex beast which takes // time to master; // 3) use of mocking can easily end up in mock behavior that is not real, as // actually occurred here (specifically, the test records Element#getAttribute // as returning null, which it would never return according to its API // documentation - instead, an empty string would be returned). @Test public void readAttributeWithDefault_BAD_version(@Mocked final Element e) { new Expectations() {{ e.getAttribute("attribute"); result = "what"; // This is a bug in the test (and in the CUT), since Element#getAttribute // never returns null for real. e.getAttribute("other"); result = null; }}; String actualValue = xmlReader.attributeWithDefault(e, "attribute", "default"); String defaultValue = xmlReader.attributeWithDefault(e, "other", "default"); assertEquals(actualValue, "what"); assertEquals(defaultValue, "default"); } // This test is better because: // 1) it does not depend on how the method under test is implemented, being // refactoring-SAFE; // 2) it does not require mastery of a mocking API and its inevitable intricacies; // 3) it depends only on reusable test code which is fully under the control of the // developer(s). @Test public void readAttributeWithDefault_GOOD_version() { Element e = getXmlElementWithAttribute("what"); String actualValue = xmlReader.attributeWithDefault(e, "attribute", "default"); String defaultValue = xmlReader.attributeWithDefault(e, "other", "default"); assertEquals(actualValue, "what"); assertEquals(defaultValue, "default"); } // Creates a suitable XML document, or reads one from an XML file/string; // either way, in practice this code would be reused in several tests. Element getXmlElementWithAttribute(String attributeValue) { DocumentBuilder dom; try { dom = DocumentBuilderFactory.newInstance().newDocumentBuilder(); } catch (ParserConfigurationException e) { throw new RuntimeException(e); } Element e = dom.newDocument().createElement("tag"); e.setAttribute("attribute", attributeValue); return e; } }