7

This is a test design question. I have a class 'handler' that accepts a 'validator' that checks some business logic on what's passed to the handler.

I made the unit tests for the validator and now I'm writing the test for the handler. Obviously I want to make sure that the validator is called when the handler does its thing.

Should I test the cases of the validator once more or is this pointless?

EDIT 1:

I'll provide a little more insight about what I'm trying to do here, as it seems that questions has more to it than I expected.

What I'm doing is a service to register users for a website and I need to check that the user provided exactly one contact method out of three possible options (email, telephone and postal address). This validation needs to be run both when the user registers in the website and when the user needs to reset his password in case he forgot it.

The method that handles the register users goes like this.

public Headers HandlePetition(Petition petition)
{
     if (petition == null)
     { throw new ArgumentNullException(); }

     contactPointValidator.ValidateContactPoint(new ContactData
     {
          Email = petition.email,
          PostalAddress = petition.postaladdress,
          Telephone = petition.telephone,
     });

     var response;

     // Do stuff

     return response;
}
Zalomon
  • 1,200
  • 3
  • 8
  • 18
  • Possible duplicate of [How should I test the functionality of a function that uses other functions in it?](http://programmers.stackexchange.com/questions/225323/how-should-i-test-the-functionality-of-a-function-that-uses-other-functions-in-i) – gnat Apr 19 '16 at 14:56
  • 5
    Contrary to other answers here, I don't think you should be `Verify()`ing calls to internal implementation details. You've tested in the wrong order. A "unit" is not necessarily a single class, although it *can* be. Write the tests that are most valuable first. If you test your code through its public API, then you will get coverage of those internal POCO (or POJO) classes for free, while keeping your tests and code flexible. – RubberDuck Apr 19 '16 at 16:19
  • @RubberDuck It depends. Validation could be doing a lot. Throwing it together with the handler could make your unit too big. Hard to know without seeing the code, though – Ben Aaronson Apr 19 '16 at 16:27
  • 2
    +1 for "a unit is not necessarily a single class". That's also worth considering. – Fabian Schmengler Apr 19 '16 at 16:46
  • You're right @BenAaronson. It does depend. – RubberDuck Apr 19 '16 at 17:57
  • 2
    @RubberDuck Imagine a class that returns nothing. You can pass it objects that it uses. You can call it's methods. It does stuff. But it never returns anything. It never outputs anything. It never even throws an exception. Infact, it never tells you a damn thing. Yet it does useful work. It calls other objects. Is testing that those calls are made really pointless testing of "internal implementation details"? – candied_orange Apr 20 '16 at 01:52
  • 1
    @CandiedOrange I'm not saying "never `Verify()`". I'm saying that it should only be necessary on the very edges of the system. There are lots of ways to skin a cat. I shy away from testing methodologies that cause me to update my tests when internal implementations change. That doesn't mean that I never mock, it just means that I prefer not to under "normal" circumstances. – RubberDuck Apr 20 '16 at 02:03
  • @RubberDuck rather than provide a definition of "internal implementation details" you're making me wonder what "normal" is. Some people consider [tell, don't ask](http://programmers.stackexchange.com/questions/157526/explanation-on-how-tell-dont-ask-is-considered-good-oo) normal. Your point is sound, you don't want to inflexibly couple an implementation to details only because of a test. But what exactly makes them internal? I'd welcome an answer that got into this. – candied_orange Apr 20 '16 at 02:26
  • @CandiedOrange that answer would likely be an essay unto itself. As for what makes something internal, I'd say it's the center of "the onion". If you could make them `internal` scope without affecting your end points or clients, then they're internal classes that only need to be tested indirectly via the public API. Like I said, an essay unto itself... – RubberDuck Apr 20 '16 at 02:35
  • First of all, thanks a lot for all your comments and answers, they've been useful and interesting to read. I've edited my question and, hopefully, I can add a little more context. RubberDuck raises a good point but, since validation rules might change somewhere along the way and it's used in at least to different cases; it feels right to unit test the validation class itself instead of the callers, which I think that breaks the separation of concerns to me. – Zalomon Apr 20 '16 at 06:52

3 Answers3

5

It should be pointless. But you should be making sure handler calls what validator needs called.

For unit testing you should be testing in isolation. That means you'll need a stub (or mock) validator to hand to handler when you test handler. You don't check validator business logic when testing handler. You test that handler calls validator (in this case ValidatorStub) when, and with what, it should.

candied_orange
  • 102,279
  • 24
  • 197
  • 315
  • Maybe I should create a mock that always raises an exception or make sure that something it passed to it so I can make sure that a validator is properly called? – Zalomon Apr 19 '16 at 15:02
  • 1
    That's an idea, but suppose one call to handler is meant to cause two calls to validator. How do you plan to check that both calls happened? – candied_orange Apr 19 '16 at 15:09
  • Rather than expect exceptions a possibility is to just pass a data structure into the mock and let the mock write to it when called. Your test constructs a Handler(MockValidator(datastructure)). Then make your calls to handler and verify that datastructure has set what should be set. – candied_orange Apr 20 '16 at 10:19
  • In fact I cannot do that since Validator makes no changes in the structure, just raises an exception if data is not valid. – Zalomon Apr 20 '16 at 10:37
  • 1
    Huh? No MockValidator. MockValidator can do whatever it wants once called. It's a mock. The datastructure exists only for testing. It's a moot point if you reactor handler so it has no dependency on validator though. – candied_orange Apr 20 '16 at 10:39
  • Yeah, you're totally right, I was too stuck on the expected behaviour from the validator. – Zalomon Apr 20 '16 at 10:54
3

Obviously I want to make sure that the validator is called when the handler does its thing

Then mock the validator and test that it is called with the right parameters. No need to test the validator itself twice.

On a related note, unit tests have their true value in test driven development. It sounds like you are writing the tests after the code instead. Maybe integration tests are more suitable for you here. Then you would test the handler together with validators, or even a bigger unit in one test that checks if everything works together as expected.

Integration tests are more valuable for regression testing than unit tests as they are more likely to find bugs from unexpected side effects.

  • 1
    In fact I'm writing the tests prior to the code but I just stumbled into this question while doing so. My issue of how to check if the validator is called with the right parameters since this is happening inside the method that I'm testing. – Zalomon Apr 19 '16 at 15:01
  • Then only pay attention to the first paragraph of my answer. Are you familiar with the concept of mock objects? – Fabian Schmengler Apr 19 '16 at 15:05
  • You can validate calls to the faked object using fakeiteasy. A.CallTo(() => fakedObject.SomeMethod()).MustHaveHappened(Repeated.Exactly.Once); – Fran Apr 19 '16 at 15:13
  • @fschmengler as far as I know using a mock will only validate that the class is referenced somehow, am I correct? – Zalomon Apr 20 '16 at 07:09
  • No, mock objects should be able to verify exactly which methods are called with which parameters. – Fabian Schmengler Apr 20 '16 at 07:15
  • How would I do that? if you see my edited post you can see the relevant parts of what I'm trying to accomplish here, I'm not quite sure that a call to 'contactPointValidator.ValidateContactPoint' can verify much. Nevertheless I think that my problem was that I was facing the problem the wrong way: the handler shouldn't be validating any of that since it's bussiness logic; not even indirectly as it is doing it now. Both the handler and the validator should be encapsulated by an upper class that uses both. I think that this is making more sense to me. Thanks for your time. – Zalomon Apr 20 '16 at 08:21
0

I'll post my last comment to fschmengler's answer as an answer since I think that it is the most fitting way to solve my question.

I think that my problem was that I was facing the problem the wrong way: the handler shouldn't be validating any of that since it's bussiness logic; not even indirectly as it is doing it now. Both the handler and the validator should be encapsulated by an upper class that uses both. I think that this is making more sense to me.

Zalomon
  • 1,200
  • 3
  • 8
  • 18