3
\$\begingroup\$

My approach to unit testing feels very wrong. I feel like I am missing something very basic, and I'm looking for some insight please.

Let's assume that I am testing a method (I'm using NUnit) that returns an object. It might look like this:

public void TestConnection()
{
     DBConnector dbConn = new DBConnector(connString);
     Assert.IsInstanceOf(typeof(DBConnector), dbConn);
}

Let's assume too that the test passes.

Later in my test class, I create another test on an object that has a dependency on MyObject. That test method might look like this:

public void IsCallInstanceNull()
{
     DBConnector dbConn = new DBConnector(connString);
     dbConn.EstablishConnection();
     CallInstance ci = new CallInstance(dbConn);
     Assert.IsNotNull(ci);
}

I am first marking TestConnection() "public void", ensuring a basic pass, then changing the method to private so that I can use a typed object in the second method, instead of duplicating code. So my example test class becomes:

private DBConnector CreateDBConnector()
{
     var dbConn = new DBConnector(connString);
     dbConn.EstablishConnection();
     return dbConn;
}

public void IsCallInstanceNull()
{            

     CallInstance ci = new CallInstance(CreateDBConnector());
     Assert.IsNotNull(ci);
}

But this feels completely wrong. No one will ever know that the constructor for DBConnector was ever tested (since not only did I change its visibility, I renamed the method!) so how can this be refactored so that I ensure test coverage for the private method without having to leave 2 almost identical methods in the test class?

\$\endgroup\$
3
  • 3
    \$\begingroup\$ For dependencies, you should use Mocks or Fakes. These are pseudo objects that fulfil the interfaces of your dependencies without introducing issues from however your dependency objects are coded. NSubstitute or Moq are excellent mocking / faking libraries, for example, but there are many more out there. \$\endgroup\$
    – Nick Udell
    Dec 2, 2014 at 15:07
  • 2
    \$\begingroup\$ The asserts in your tests don't seem to prove anything. They will either never fail, or fail because the test threw an exception (i.e. the assert won't run anyway). I think you need to rethink what you're actually trying to test and write the test first to prove that it fails before writing the production code. \$\endgroup\$ Dec 2, 2014 at 23:49
  • \$\begingroup\$ Both tests are totally useless, just delete them. If you are not asserting any useful behavior has occurred, the test is useless. Tip: Search for "Assert.IsInstanceOf" and "Assert.IsNotNull" in your tests, and evaluate each test if that assert is useful. \$\endgroup\$ Dec 4, 2014 at 7:52

2 Answers 2

-1
\$\begingroup\$

Okay so I'll try answer the meta-question about unit testing with dependencies. As others have pointed out, your examples aren't really unit tests, which makes it a little difficult to redo them.

First point. the aim of a unit test is to test some behaviour of the system, in an isolated manner. This is important to keep in mind as not all dependencies are the same, and their nature dictates how you'll handle them.

Some dependencies will be the source of state, some will be the source of domain logic/knowledge and others will be further system behaviour.

So lets take the following code: (Implied is the code to set the fields which hold the dependencies)

public class House {
  private TemperatureSensor thermometer;
  ...
  public bool IsLoungeTooHot() {
    return thermometer.CurrentTemperature > 20;
  }
}

It's a simple class with a single bit of behaviour checking the lounge temperature and a single dependency on a thermometer.

So thermometer is the source of the data and as such it is always faked since the test needs to be able to use multiple temperatures to ensure it works (at the very least return temp below 20 and above 20)

Now if we were to extend the code to

public class House {
  private TemperatureSensor thermometer;
  private Thermostat thermostat;
  ...
  public bool IsLoungeTooHot() {

    return thermometer.CurrentTemperature > thermostat.DesiredTemperature;
  }
}

So we now the desired temperature is no longer a constant but rather retrieved from a domain object. So the thermostat dependency is something that will almost never be faked (safe to read never but there can be edge cases). The reason why we don't want to fake this is to protect ourselves against test fragility.

As the writers of the code we could create a perfect fake of the Thermostat. However it would only be perfect at the moment we wrote the test. At some point down the line the behaviour of Thermostat could change, and if we had used a fake there is a very good chance we would not notice.

Third and final extension of the code:

public class House {
  private TemperatureSensor thermometer;
  private Thermostat thermostat;
  private DataRepository repo;
  ...
  public bool IsLoungeTooHot() {
    repo.Audit("IsLoungTooHot");
    return thermometer.CurrentTemperature > thermostat.DesiredTemperature;
  }
}

The repo dependency similar to the thermostat dependency, it is additional system behaviour. However in this specific case we would replace it with a fake because using the real database would break the isolation we need to ensure that this test doesn't affect other tests that may also be running.

A bit of a wall of text, hope it's made things a little clearer.


fake - stub/mock see https://stackoverflow.com/a/346440/5889 for a good explanation of the differences.

\$\endgroup\$
3
  • \$\begingroup\$ This is not a review of the posted code. \$\endgroup\$ Dec 4, 2014 at 7:38
  • \$\begingroup\$ @abuzittingillifirca OP asked for insight into what was wrong about his code, so excuse me for giving a more detailed answer than "everything is wrong with your code" \$\endgroup\$ Dec 4, 2014 at 9:58
  • \$\begingroup\$ Marking Graeme's as the answer. Nick's was a good direction but I appreciate the clarity provided in Graeme's answer. And for the record, I genuinely dont understand all the surly "its wrong" answers. Isn't this the CodeReview SE? I know its wrong. I'm looking for help on making it better so I dont repeat the same mistakes. \$\endgroup\$ Dec 4, 2014 at 13:34
1
\$\begingroup\$

When unit testing a class you should focus only on the functionality in that class. For all other dependencies, mocking should be used. That is to say you should create "fake" implementations of the dependencies and use them to simulate the real thing. You can then run tests against the mocks to ensure that they are being used correctly according to the functionality of the class under test.

In order to do this you will have to inject all dependencies(this is pretty much always what you should be doing), likely in your constructor.

Your test example doesn't tell us too much and doesn't test anything. Instantiating a class then validating its type is not a test. Could you provide the class that you would like to test? Then we can give concrete examples.

I wrote a tutorial for using Moq to test dependencies, it can be found here: http://stevemichael.net/how-do-i-use-moq-with-asp-net-mvc/

\$\endgroup\$
2
  • \$\begingroup\$ Thank you everyone - I modified my original question to include "real" code as it was placed on hold for usage of hypothetical code. The original question was still more about testing the dependency, as opposed to the actual unit test content, which Nick provided direction on. That being said, you are all right, I should provide a better example with better unit test content. \$\endgroup\$ Dec 3, 2014 at 13:13
  • \$\begingroup\$ If you want to provide the code you are testing we can work together to get it tested in case there are more questions :) \$\endgroup\$ Dec 3, 2014 at 13:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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