Unit testing complex scenarios – one approach

This is another of those ‘as much for my benefit’ as it is for the community posts.  On a sizeable project at work we’ve hit a ‘catch up on tests’ phase.  We don’t employ TDD, though obviously understand that testing is very important to the overall product (both in confidence on release, and confidence that changes to the code will break the build if functionality changes).  Our code coverage when we started this latest phase of work was terrible (somewhere around 20% functionality coverage with 924 tests) – after a couple of weeks of testing we’re up to fairly high coverage on our data access/repositories (>90%) and have significantly more tests (2,600 at last count).

We are following a UI –> Service –> Repository type pattern for our architecture which works very well for us – we’re using IoC, though perhaps only because of testability – the loose coupling benefits are obviously there.

We’re now at the point of testing our service implementations, and have significantly more to think about.  At the data access layer, external dependencies were literally only the database.  At service layer, we have other services as external dependencies, as well as repositories, so unit testing these required considerably more thought/discussion.  Thankfully I work with a very good team, so the approach we’ve taken here is very much a distillation of the outcomes from discussion with the team.

Couple of things about our testing:

Confidence is King

The reason we write unit tests is manifold, but if I were to try to sum it up, it’s confidence.

Confidence that any changes to code that alter functionality break the build.

Confidence that the code is working as expected.

Confidence that we have solidly documented (through tests) the intent of the functionality and that someone (more often than not another developer in our case) has gone through a codebase and has reviewed it enough to map the pathways through it so that they can effectively test it.

Confidence plays a huge part for us as we implement a Continuous Integration process, and the longer term aim is to move towards Continuous Delivery.  Without solid unit testing at it’s core, I’d find it difficult to maintain the confidence in the build necessary to be able to reliably deploy once, twice or more per day.

Test Functionality, Pathways and Use Cases, Not Lines of Code

100% code coverage is a lofty ideal, though I’d argue that if that is your primary goal, you’re thinking about it wrong.  We have often achieved 100% coverage, but done so via the testing of pathways through the system rather than focussing on just the lines of code.  We use business exceptions and very much take the approach that if a method can’t do what it advertises, an exception is thrown.

Something simple like ‘ValidateUserCanDeposit’ can throw the following:

/// <exception cref="PaymentMoneyLaunderingLimitException">Thrown when the user is above their money laundering limit.</exception>
/// <exception cref="PaymentPaymentMethodChangingException">Thrown when the user is attempting to change their payment method.</exception>
/// <exception cref="PaymentPaymentMethodExpiredException">Thrown when the expiry date has already passed</exception>
/// <exception cref="PaymentPaymentMethodInvalidStartDateException">Thrown when the start date hasn't yet passed</exception>
/// <exception cref="PaymentPlayerSpendLimitException">Thrown when the user is above their spend limit.</exception>
/// <exception cref="PaymentPlayerSpendLimitNotFoundException">Thrown when we are unable to retrieve a spend limit for a user.</exception>
/// <exception cref="PaymentOverSiteDepositLimitException">Thrown when the user is over the sitewide deposit limit.</exception>

and these are calculated often by calls to external dependencies (in this case there are 4 calls away to external dependencies) – the business logic for ‘ValidateUserCanDeposit’ is:

  1. Is the user over the maximum site deposit limit
  2. Validate the user has remaining spend based upon responsible gambling limits- paymentRepository.GetPlayerSpendLimit

    – paymentRepository.GetUserSpendOverTimePeriod

  3. GetPaymentMethodCurrent- paymentRepository.GetPaymentMethodCurrent

    – paymentRepository.GetCardPaymentMethodByPaymentMethodId

    – OR paymentRepository.GetPaypalPaymentMethodByPaymentMethodId

  4. if we’re changing payment method, ensure:- not over money laundering limit

So testing a pathway through this method, we can pass and fail at each of the lines listed above.  A pass is often denoted as silence (our code only gets noisy when something goes wrong), but each of those external dependencies themselves can throw potentially multiple exceptions.

We employ logging of our exceptions so again, we care that logging was called.

Testing Framework and Naming Tests

NUnit is our tool of choice for writing unit tests – syntax is expressive, and it generally allows for very readable tests.  I’m a big fan of the test explaining the authors intent – being able to read and understand unit tests is a skill for sure, though once you’ve read a unit test, having it actually do what the author intended it to is another validator.

With that in mind, we tend to take the approach ‘MethodUnderTest_TestState_ExpectedOutcome’ approach.  A few examples of our unit test names:

  • GetByUserId_ValidUserId_UserInCache_GetVolatileFieldsReturnsValidData_ReturnsValidUserObject
  • GetPlaymateAtPointInTime_GivenCorrectUserAndDate_ValidPlaymateShouldExist
  • GetCompetitionEntriesByCompetitionId_NoEntries_ShouldReturnEmptyCollection
  • GetTransactionByUserIdAndTransactionId_DbException_ShouldThrowCoreSystemSqlException

Knowing what the author intended is half the battle when coming to a test 3months from now because it’s failing after some business logic update.

Mocking Framework

We use Moq as a mocking framework, and I’m a big fan of the power it brings to testing – yup, there are quite a number of steps to jump through to effectively setup and verify your tests, though again, these add confidence to the final result.

One note about mocking in general, and any number of people have written on this in far more eloquent terms than I.  Never just mock enough data to pass the test.

If we have a repository method called ‘GetTransactionsByUserAndDate’, ensure that your mocked transactions list also includes transactions from other users as well as transactions for the same user outside of the dates specified – getting a positive result when that is the only data that exists is one thing, getting a positive result when you have a diverse mocked data set with things that should not be returned again adds confidence that the code is doing specifically what it should be.

Verifying vs. Asserting

We try very much to maintain a single assert per test (and only break that when we feel it necessary) – it keeps the mindset on testing a very small area of functionality, and makes the test more malleable/less brittle.

Verifying on the other hand (a construct supported by Moq and other framekworks) is something that we are more prolific with.

For example, if ‘paymentRepository.GetPlayerSpendLimit’ above throws an exception, I want to verify that ‘paymentRepository.GetUserSpendOverTimePeriod’ is not called – I also want to verify that we logged that exception.

The Assert from all of that is that the correct exception is thrown from the main method, but the verifies that are called as part of that test add confidence.

In our [TestTearDown] method we tend to place our ‘mock.Verify()’ methods to ensure that we verify those things that are able to be after each test.

Enough waffle – where’s the code?

That one method above ‘ValidateUserCanDeposit’ has ended up with 26 tests – each one models a pathway through that method.  There is only one success path through that method – every other test demonstrates error paths.  So for example:

[Test]
public void ValidateUserCanDeposit_GetPaymentMethodCurrent_ThrowsPaymentMethodNotFoundException_UserBalanceUnderMoneyLaunderingLimit_ShouldReturnPaymentMethod()
{
	var user = GetValidTestableUser(ValidUserId);
	user.Balance = 1m;

	// remaining spend
	paymentRepository.Setup( x => x.GetPlayerSpendLimit(ValidUserId)).Returns( new PlayerSpendLimitDto { Limit = 50000, Type = 'w' }).Verifiable();
	paymentRepository.Setup( x => x.GetUserSpendOverTimePeriod(ValidUserId, It.IsAny(), It.IsAny())).Returns( 0 ).Verifiable();

	// current payment method
	paymentRepository.Setup( x => x.GetPaymentMethodCurrent(ValidUserId))
						.Throws( new PaymentMethodNotFoundException("") ).Verifiable();

	IPaymentMethod paymentMethod = paymentService.ValidateUserCanDeposit(user, PaymentProviderType.Card);

	Assert.That(paymentMethod.CanUpdatePaymentMethod, Is.True);
	paymentRepository.Verify( x => x.GetCardPaymentMethodByPaymentMethodId(ValidCardPaymentMethodId), Times.Never());
	LogVerifier.VerifyLogExceptionCalled(logger, Times.Once());
}

That may seem like a complex test, but I’ve got the following from it:

  • The author’s intent from the method signature:upon calling ValidateUserCanDeposit

    a call within that to GetPaymentMethodCurrent has thrown a PaymentMethodNotFoundException

    at that point, the users balance is below the money laundering limit for the site

    so the user should get a return that indicates that they can update their payment method

  • That those methods that I expect to be hit *are* hit (using moq’s .Verifiable())
  • That those methods that should not be called aren’t (towards the end of the test, Times.Never() verifies
  • That we have logged the exception once and only once

Now that this test (plus the other 25) are in place, if a developer is stupid enough to bury an exception or remove a logging statement that the build will fail.

Is this a good approach to testing?

I guess this is where the question opens up to you guys reading this.  Is this a good approach to testing?  The tests don’t feel brittle.  They feel like they’re focussing on one area at a time.  They feel like they are sure about what is going on in the underlying code.

Overkill?

Ways to improve them?

How do you unit test in this sort of situation? What software do you use? What problems have you faced?  Keen to get as much information as possible and hopefully help inform each other.

I’d love to get feedback on this.  It feels like it’s working well for us, but that doesn’t necessarily mean it’s right/good.

Unity 2.0 – Just in the nick of time

We’re just re-architecting our site currently and a big part of that is test first (I daren’t call it test driven!) development.  It’s an MVC2 project, with distinct business and data access layers. I’m loving MVC2, model binding, and the power I’m getting from making my domain models richer.

Obviously one of the biggies in unit testing is the use of Inversion of Control via an IoC container (older list) so that you can reliably pass around interfaces rather than implementations, the aim being to improve the testability of it all, and allowing mocking up of dependencies in order to test just certain areas of the functionality.

I’d investigated a few IoC containers:

  • CastleWindsor – I just personally found this too much like hard work in terms of the thinking side of things – I didn’t like the syntax, which must be an utterly personal thing as there’s not a big difference between that an Unity.
  • StructureMap – this one felt more like voodoo, though worked well – was definitely a good second choice.
  • Unity (v1.2) – I liked the config on this one, and I liked the way of specifying which constructor to use in a multi-constructor dependency.  What I didn’t realise was that it didn’t out of the box seem to support the ‘PerWebRequest’ type lifestyle on dependencies (though I managed to roll my own without too much difficulty).

I’d hasten to add the above are my findings, and may not be fully representative of someone with a clue playing with this!  I may easily have missed elements in the initial plays.

Unity 2.0 (which is available as part of Enterprise Library 5) comes with a ‘PerResolveLifetimeManager’ out of the box now, which does exactly what it says on the tin – during a running instance (in this case, a web request) the first time it resolves it’ll new up an instance for the app, and during that instance it’ll use that one, until the next instance (page load).

So we have:

container.RegisterType<iwebsessionmanager , WebSessionManager>(new PerResolveLifetimeManager(), new InjectionConstructor(typeof(IUserService), typeof(ISessionManager)));

instead of the default TransientLifetimeManager.

There are other lifetime managers available with Unity 2.0 (HierarchicalifetimeManager being the other new one), and obviously you still have ExternallyControlledLifetimeManager should you need it.

All in all, very happy that we went down the Unity route when we did, and those updates in 2.0 have proven to come just when we needed them.

p.s. I’ll add links to the correct MSDN sections as soon as they’re available – the documentation only seems available for 1.2 at present.

Unit Testing with DataAnnotations outside of MVC

This past week has seen us start on a big project at work to re-architect the site into .net and MVC2.  Naturally we have our models in a separate project, and we have two separate test projects (Unit and Integration) setup to use NUnit.

As it’s early days for us, and our first “real” MVC project I thought I’d write this up, a) as an aid to learning for me, but b) to try to gain feedback from the community on what they do with regards validation on their models.

I can see a few different ways we could have done this (annotate the ViewModels we’ll use on the front end, build in logic into our setters to validate, etc. etc.) but we’re now going down a route that so far feels ok.  That said, we’re focussing solidly on the modelling of our business logic at present, so haven’t yet brought the model “out to play” as it were.

Hopefully the above gives a wee bit of insight into where we are with it.

We’ve decided to plump for the MetaData model approach to keep the main objects slightly cleaner – an example for us would be:

namespace MyCompany.Models.Entities
{
	/// 
	/// 
	/// 
	[MetadataType(typeof(MyCompanyUserMetaData))]
	public class MyCompanyUser
	{
		public int UserId { get; set; }

		public string Username { get; private set; }
		...

		public void SetUsername(string newUsername)
		{
			if (Username != null)
				throw new ArgumentException("You cannot update your username once set");

			//TODO: where do we ensure that a username doesn't already exist?
			Username = newUsername;
		}
	}
}

and then in a separate class:

namespace MyCompany.Models.Entities
{
	public class MyCompanyUserMetaData
	{
		[Required(ErrorMessage="Your password must be between 6 and 20 characters.")]
		[StringMinimumLength(6, ErrorMessage="Your password must be at least 6.")]
		public string Password { get; set; }

		[Required(ErrorMessage="Your username must be between 6 and 20 characters.")]
		[StringLength(20, MinimumLength=6, ErrorMessage="Your username must be between 6 and 20 characters.")]
		[MyCompanyUserUsernameDoesNotStartWithCM(ErrorMessage="You cannot use the prefix 'CM-' as part of your username")]
		[CaseInsensitiveRegularExpression(@"^[\w\-!_.]{1}[\w\-!_.\s]{4,18}[\w\-!_.]{1}$", ErrorMessage = "Your username must be between 6 and 20 characters and can only contain letters, numbers and - ! _ . punctuation characters")]
		public string Username {get;set;}
	}
}

With all of this in place you’re all well and good for the MVC world, though unit testing just doesn’t care about your Annotations so your simple unit tests:

		
[Test]
public void SetUsername_UsernameTooShort_ShouldThrowExceptionAndNotSetUsername()
{
	// Arrange
	testUser = new MyCompanyUser();
			
	// Act

	// Assert
	Assert.Throws(() => testUser.SetUsername("12345")); // length = 5
	Assert.That(testUser.Username, Is.Null, "Invalid Username: Username is not null");
}

won’t give you the expected results as the logic of that is based upon the DataAnnotation.

What was our solution?

After much reading around (there didn’t seem to be an awful lot out there covering this) we took a two step approach.  First was to allow SetUsername to validate against the DataAnnotations like so:

		
public void SetUsername(string newUsername)
{
	if (Username != null)
		throw new ArgumentException("You cannot update your username once set");

	Validator.ValidateProperty(newUsername, new ValidationContext(this, null, null) { MemberName = "Username" });

	//TODO: where do we ensure that a username doesn't already exist?
	Username = newUsername;
}

Validator is well documented and there are a few examples out there of people doing this within their setters.  Essentially validating the input for a particular MemberName (Username in this case).

The second step was necessary because of the approach we’d taken with the MetaData class above, and it was a mapping in the TestFixtureSetup within our unit tests:

TypeDescriptor.AddProviderTransparent(new AssociatedMetadataTypeTypeDescriptionProvider(typeof(MyCompanyUser), typeof(MyCompanyUserMetaData)), typeof(MyCompanyUser));

This line (though I’ve yet to look down at the source code level) would appear to just be a standard mapping for the class to tell it where to find the metadata/annotations.

After putting those two things in place, the unit tests successfully validate against the annotations as well as any coded business logic, so jobs a good un!

Was it the right solution?

This is where I ask you, the person daft enough to suffer this blog post!  I have no idea if there is a better way to do this or how this will pan out as we propagate up to the MVC level – will I be causing us headaches taking this approach, will it simply not work because of overlap between the way MVC model binder validates versus what we’ve done down at the domain level?

It’s still early days for the project, and the above feels like a nice way to validate down at a business domain level, but how it pans out as we propagate wider and start letting other projects consume, hydrate and update the models… well that’s anyone’s guess!

Comments very much welcome on this one folks 🙂