2

I have code that follows this pattern.

Class Factory
    build:
       returns: Add|Remove|Stub
       takes: property1, property2, property3

Class Add
     execute:  does something
     takes: property2, propery3

Class Remove
     execute: does something
     takes: property2

Class Null
      execute: does nothing
      takes: nothing

In addition to verifying that the Factory:build produces the correct object I want to test that the 'property' values are passed through correctly.

It seems like the best place to do this is in the factory unit test but the assert would look like this.

this->assertEquals( property1, returnedObject->get... );

This smells bad to me because the test is actually testing the returnedObject and not the Factory itself. However, since the factory is generating those objects, it feels correct to test that they are generated properly.

Edit/Clarification: By calling assert on the returned object, the unit test is testing the behavior of that object and not Factory. Failures in that object could cause the Factory unit-test to fail even though the Factory is passing the correct data into it's output.
Thus changes to the Add/Remove/Null objects could cause the Factory to fail before the executable objects are even tested.

What's the correct approach?

3

If I got you right, your concerns are caused by the fact you have to call a "getter" method of the returned object for the assertion, so

  1. a failing test could be caused by an error inside the "getter", and not by an issue in the factory,

  2. a non-failing test could be a "false negative", because an error in the getter could mask an issue inside the factory.

This is both possible, in theory, but unlikely to become a problem, because:

  • in case 1, the failing test points to a real issue which can be fixed once. In the 99% case of a simple "getter" returning the value of a private member variable, the root cause of problem will be easy to spot, and it is unlikely the getter will ever be touched again.

  • case 2 is very unlikely when you provide at least 2 tests for the factory where each one returns an object with a different result for the "getter". Note the value returned by the "getter" is the property of that object from an outside view, if the getter delivers what's expected, there is no error.

The only case which is not covered by this is a situation where the internal "getter" implementation is quite so complex that it is worth to separate the unit tests for that getter from the unit tests for the factory (so in case of a failing test, it becomes easier to spot the root cause). For this it is usually sufficient to write unit tests for the "getter" alone, to make it "bulletproof" enough so you can trust it's usage inside the factory unit test.

  • I'm also considering the unlikely event that one of the returned objects gets changed. That could lead to tests failing in the Factory object should it get called before the tests of the returned object. Or is that desired as it would indicate unintended behavior of your change? – serverSentinel Apr 2 at 15:18
  • @serverSentinel: changed by calling the "getter"? – Doc Brown Apr 2 at 16:54
  • My apologies. I meant if the class, not object, gets changed. This test isn't vulnerable to changes to just the factory, it's vulnerable to changes to the action class as well. – serverSentinel Apr 2 at 21:28
  • @serverSentinel: yes, it is (which is not a bad thing, quite the opposite). The only thing you have to decide is, if in case the test fails, is it a problem to find the root cause, because the factory as well as the object are becoming so complex that it makes sense to have individual unit tests for both. Maybe the object's code is "simple" at the beginning, so you can start with a test for the factory first. Later, the object becomes more complex - then you decide to add individual tests for that object as well. – Doc Brown Apr 3 at 4:58
3

In my opinion, you should be testing the responsibilities of the Factory. One of the responsibilities is to create the right class, another is to set the properties correctly.

I see no way to examine the Factory to determine the state of the product, so you'll have to look at the product.

Analogy: If I wanted to see if a car manufacturer is working properly, I'd definitely look at the cars...

Hope this helps...

  • Thanks, for your answer. I think your analogy breaks because the factory just produces the cars but a defect in the engineering of the car doesn't mean the factory did it's job incorrectly. It could be either the Factory or the Car if you only have a car to look at. – serverSentinel Apr 2 at 15:34
0

You're correct: writing a test on the FooFactory unit shouldn't verify behavior of the Foo objects. The test should purely test that the FooFactory creates objects, passing the inputs in an expected way. Thus, we shouldn't be asserting anything on a real object that the FooFactory is creating.

One solution to this is using Dependency Injection. In a non-test environment, you give the factory the real classes (or constructors) when you instantiate the factory. When you're in the test environment, you give the factory mock constructors which look like a real object. But the mocks don't have the real logic backing them (so a non-public-API change to the Foo class won't change the FooFactory's tests), and allow you to spy on specific parts without regards to the real implementation (e.g. if an internal to Foo should be set on instantiation, but is in a private field with no getter). Thus, you can keep your FooFactory tests from testing the behavior of a Foo - since the FooFactory doesn't ever use a real Foo, it only instantiates MockFoo objects, which only have a constructor (that you tested gets called with the parameters you expect).

Your mileage may vary depending on language and project, however - in some projects, using dependency injection is trivial, others require importing a library, others just won't support it.

  • You're suggesting I create the Add, Remove and Null objects and pass them into the factory object at runtime? Factory becomes more of a lookup then and not an actual builder. – serverSentinel Apr 2 at 21:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.