Testing classes

I'm just getting into testing and wanted to see if someone can tell me if I'm writing tests correctly. I'm using C#, NUnit, & Should.

This is the class I'm testing:

```
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Reflection.Emit;
Using VM.Models.Common;

namespace Vm.Models.CustomerNS
{
public class Customer
{
// ReSharper disable once InconsistentNaming
public int ID { get; set; }
public List Locations { get; set; }
public List Users { get; set; }
public List BuyerNumbers { get; set; }
public Dictionary VehicleList { get; set; }
public List Contacts { get; set; }
public string Name { get; set; }

public Customer()
{
Locations = new List();
Users = new List();
BuyerNumbers = new List();
Contacts = new List();
VehicleList = new Dictionary();
}

public void AddLocation(AddressType addtype, string addressName, string streetAddress, string city, string state, string zip, string country)
{
var addressId = 0;
var lastOrDefault = Locations.LastOrDefault();
if (lastOrDefault != null) addressId = lastOrDefault.AddressId + 1;
Address newAddress = new Address
{
AddressId = addressId,
AddressName = addressName,
AddressType = addtype,
City = city,
State = state
};
if (streetAddress != null) newAddress.StreetAddress = streetAddress;
if (zip != null) newAddress.Zip = zip;
Locations.Add(newAddress);
}

public void RemoveLocation(int addressId)
{
Debug.Assert(Locations != null, "Locations != null");
var addressid = Locations.Where(x => x.AddressId == addressId);
var enumerable = addressid as IList ?? addres

My first observations is: use injection!

for example: Instead of passing inAddressType addtype, string addressName, string streetAddress, string city, string state, string zip, string country into AddLocation, expect an Address instance. This simplifies the code greatly (a method should preferably have max 2 parameters), and in the future if you need functionality in the Address class, you will be able to pull an interface and mock the required return from the functionality.

I'm not sure where the ShouldNotBeNull, ShouldNotBeEqual, and ShouldBeEqual methods are coming from. While I understand what you are trying to do with them, I find them distracting from reading the tests. Maybe its just my brain trained to be looking for Assert for the checking part, I don't know, but on initial read through, I found it difficult to follow.

For your test when changing things, I always add an Assert on the base case.

private void TestSomething()
{
     var sut = new List
                         {
                             "one",
                         }

     Assert.That(sut.Count, Is.EqalTo(1));

     sut.Add("Two");

     Assert.That(sut.Count, Is.EqualTo(2));
}

I always name my class being tested sut (System Under Test). This allows anybody reading it to easily identify which class you are actually testing.

I would also rename your test methods. Having Test in the name is a little redundant, seems how this is a test fixture. I like having my method names reflect what is being tested and what the expected result is:

[Test]
public void CallingAddLocationWithValidDataShouldAddLocationToCustomer()
{
    // Code goes here
}

GenerateMultipleLocations does not have to return the Customer instance because it is being passed in as a ref automatically. Any changes made in the method will be reflected in the class in the original method.

As far as testing goes, this is much better than the mess I created when I first started. Keep working at it, eventually it will be very easy to see and write tests.

StackExchange Code Review Q#39336, answer score: 4