Tuesday, June 3, 2014

Test Strategy in SharePoint: Part 1 – testing poor layering is not good TDD

copyright from: blog.goneopen.com

My review of SharePoint and TDD indicated that we need to use layering techniques to isolate SharePoint and not to confuse unit tests with integration tests. Let’s now dive into what is the nature of the code we write in SharePoint.
This post of one of two posts. This first one will review two existing code samples that demonstrate testing practices: SharePoint Magic 8 Ball & Unit Testing SharePoint Foundation with Microsoft Pex and Moles: Tutorial for Writing Isolated Unit Tests for SharePoint Foundation Applications. I argue against using these practices because this type of mocking encourages poor code design through exclusively using unit tests and not isolating approach integration tests. In the second post, I will cover examples that demonstrate layering and separation of unit and integration tests, including how to structure Visual Studio solutions.

What is the nature of SharePoint coding practice?

At this stage, I am not looking at “customisation” code (ie WebParts) but rather “extension” code practices where we effectively configure up the new site.
  1. programming code is often “wiring” deployment code (eg files mostly of xml) that then needs to be glued together via .net code against the SharePoint API.
  2. resulting code is problematically procedural in nature and untestable outside its deployment environment
  3. this makes its error prone and slow
  4. it also encourages developers to work in large batches, across longer than needed time frames and provide slow feedback
  5. problems tend to occur in later environments
So, now lets take a typical SharePoint project. Of course there is a range and gamut of SharePoint projects, but lets pick the average summation of them all. Thus in a typical SharePoint project, the portion where TDD is actually applicable is very small – which is the writing code part. In most, not all, SharePoint projects, we write code as small bandaids across the system, to cover that last mile – we don’t write code to build the entire platform, in fact the emphasis is to write as little code as possible, while meeting the requirements. So already, the applicability of TDD as a total percentage of the project is much smaller. Now, lets look at the code we write for SharePoint. These small bandaids that can be independent of each other, are comprised of some C#/VB.NET code, but a large portion of the code is XML files. These large portion ofXML files, especially the most complex parts, define the UI – something TDD is not good at testing anyway. Yes I know attempts have been made, but attempt != standard. And the parts that are even pure C#, we deal with an API which does not lend itself well to TDD. You can TDD SharePoint code, but it’s just much harder.
What we found when stuck to the procedural, wiring up techniques is:
  1. write, repetitive long blocks of code
  2. funky behaviours in SharePoint
  3. long periods of time sitting around watching the progress bar in the browser
  4. disagreement and confusion between (knowledgable) developers on what they should be doing and what was happening
  5. block copy inheritance (cut-and-paste) within and between visual studio solution
  6. no automated testing
Looking at that problem we have formulated the charitable position that standard SharePoint techniques allow you to write easy code – or more correctly, allow you to copy and paste others code and hack it to your needs. We decided that we would try to write maintainable code. This type of code is layered and SOLID …
Okay so where are the code samples for us to follow?
There are two main samples to look at and both try and deal with the SharePoint API. Let me cover these off first before I show where we went.

SharePoint Magic 8 Ball

SharePoint Magic 8 Ball is some sample source code from Best Practices SharePoint conference in 2009. This a nice piece of code to demonstrate both how to abstract away SharePoint and how not to worry about unit testing SharePoint.
Let me try and explain. The functionality of the code is that you ask a question of a “magic” ball and get a response – the response is actually just picking an “answer” randomly from a list. The design is a simple one in two parts: (1) the list provider for all the answers and (2) the picker (aka the Ball). The list is retrieved from SharePoint and the picker takes a list from somewhere.

The PickerTest (aka the ball)

1
2
3
4
5
6
7
8
9
10
11
12
[TestFixture]
 public class BallPickerTest
 {
     [Test]
     public void ShouldReturnAnAnswerWhenAskedQuestion()
     {
         var ball = new Ball();
         answer = ball.AskQuestion("Will it work?");
 
         Assert.IsNotNull(answer);
     }
}
And the Ball class
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
public class Ball
{
    public List<string> Answers { get; set }
     
    public Ball()
    {
        Answers = new List<string>{"yes"};
    }
 
    public string AskQuestion(string p)
    {
        Random random = new Random();
        int item = random.Next(Answers.Count);
        return Answers[item];
    }
}
That’s straightforward. So then the SharePoint class gets the persisted list.
1
2
3
4
5
6
7
8
9
10
11
12
13
public class SharePoint
{
    public List<string> GetAnswersFromList(string ListName)
    {
        List answers = new List<string>();
        SPList list = SPContext.Current.Web.Lists[ListName];
        foreach (SPListItem item in list.Items)
        {
            answers.Add(item.Title);
        }
        return answers;
    }
}
Here’s the test that shows how you can mock out SharePoint. In this case, the tester is using TypeMock. Personally, what I am going to argue is that isn’t an appropriate test. You can do it, but I wouldn’t bother. I’ll come back to how I would rather write an integration test.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using NUnit.Framework;
using TypeMock.ArrangeActAssert;
using Microsoft.SharePoint;
 
namespace BPC.Magic8Ball.Test
{
    [TestFixture]
    public class SharePointTest
    {
        [Test]
        public void ShouldReturnListOfAnswersFromSharePointList()
        {
              SPList fakeList = Isolate.Fake.Instance();
              Isolate.WhenCalled(() => SPContext.Current.Web.Lists["ListName"]).WillReturn(fakeList);
              SPListItem fakeAnswerOne = Isolate.Fake.Instance();
              Isolate.WhenCalled(() => fakeAnswerOne.Title).WillReturn("AnswerZero");
              Isolate.WhenCalled(() => fakeList.Items).WillReturnCollectionValuesOf(new List{fakeAnswerOne });                             
   
              var answers = new SharePoint().GetAnswersFromList("ListName");
 
              Assert.AreEqual("AnswerOne", answers.First());
        }
    }
}
In the code above, it nicely demonstrates that TypeMock can mock out a static class that lives somewhere else. Put differently, you don’t have use dependency injection patterns to do your mocking. I want to argue that this is a poor example because this is not a practical unit test – and I’m sure that if I wasn’t lazy I could find the test smell here already documented in xUnit Test Patterns by Gerald Mezaros. Least the naming of the classes!
The key problem is that sample project is that the real test here is that there is good decoupling between the Picker (the ball) and the answers (sharepoint list). If there is any mocking to go on, it should be that the answers (sharepoint list) is actually invoked. If this the case, then this also points to a potential code smell and that is that the dependency of the answer list (either as List or SharePoint) is documented clearly. In other words, it might want to be passed in into the constructor. Now you might even argue that the ball should have a property for the answers but rather a reference to the SharePoint (answer getter). This may seem a small point but are important because we need designs that scale – and decoupling has been proven to do so.

So, instead of this code:
1
2
3
4
5
var ball = new Ball();
var sharePoint = new SharePoint();
 
ball.Answers = sharePoint.GetAnswersFromList(ListName);
var answer = ball.AskQuestion("MyQuestion");
You are more likely to have if hand in the list:
1
2
3
var ball = new Ball(new SharePoint.GetAnswersFromList(ListName));
ball.Answers = sharePoint.GetAnswersFromList("Your answer");
var answer = ball.AskQuestion("MyQuestion");
Or if the SharePoint is a dependency:
1
2
3
var ball = new Ball(new SharePoint(ListName));
ball.Answers = sharePoint.GetAnswersFromList("Your answer");
var answer = ball.AskQuestion("MyQuestion");

What is at the heart of this sample is that it is trying to unit test everything. Instead it should split tests into unit and integration tests. I have documented elsewhere very specific definitions of unit and integration tests:
  • Unit – a test that has no dependencies (do our objects do the right thing, are they convenient to work with?)
  • Integration – a test that has only one dependency and tests one interaction (usually, does our code work against code that we can’t change?)
What would I do?
  • unit test the ball – there are a number of tests around the AskQuestion() method – does the randomness work, can I hand it a list of answers. If I am going to hand in the SharePoint class – I can then can handin a fakeSharePoint class for stubs and then mock the class to check that it actually calls the GetAnswersFromList() method.
  • integration test the SharePoint class – the integration test requires that SharePoint is up and running. This is a great integration test because it checks that you have all your ducks lined up for a SPContext call and that you have used the correct part of the API.Interestingly, the harder part of the integration tests are getting the setup context correct. Having to deal with the Setup (and Teardown) now is in fact one of the most important benefits of these types of integration tests. We need to remember that these integrationAPIs merely confirm that the API is behaving as we would expect in the context in which we are currently working. No more. No less.
In summary, this sample showing particular layering in the code but not in the testing. Layering your tests is as important. In SharePoint, we have had the most success in creating abstractions exclusively for SharePoint and testing these as integration tests. These SharePoint abstractions have also been created in a way that we can hand in mock implementations into other layers so that we can unit test the parts of the code that have logic. This is a simple design that effectively makes SharePoint just another persistence-type layer. There is a little more to it than that but that isn’t for here.
Let’s turn to Pex and Moles and see if it provides an option any better to TypeMock. I suspect not because the issue here isn’t the design of the test system – these both can intercept classes hidden and used somewhere else with my own delegates – but rather the design of our code. I’m hoping to use tools that help me write better more maintainable code – so far both of them look like the “easy code” smell.

Pex and Moles

So I’ve headed off to Unit Testing SharePoint Foundation with Microsoft Pex and Moles: Tutorial for Writing Isolated Unit Tests for SharePoint Foundation Applications. It is a great document that tells us why SharePoint is not a framework built with testability baked in.
The Unit Testing Challenge. The primary goal of unit testing is to take the smallest piece of testable software in your application, isolate it from the remainder of the code, and determine whether it behaves exactly as you expect. Unit testing has proven its value, because a large percentage of defects are identified during its use.
The most common approach to isolate production code in a unit test requires you to write drivers to simulate a call into that code and create stubs to simulate the functionality of classes used by the production code. This can be tedious for developers, and might cause unit testing to be placed at a lower priority in your testing strategy.
It is especially difficult to create unit tests for SharePoint Foundation applications because:
* You cannot execute the functions of the underlying SharePoint Object Model without being connected to a live SharePoint Server.
* The SharePoint Object Model-including classes such as SPSite and SPWeb-does not allow you to inject fake service implementations, because most of the SharePoint Object Model classes are sealed types with non-public constructors.
Unit Testing for SharePoint Foundation: Pex and Moles. This tutorial introduces you to processes and concepts for testing applications created with SharePoint Foundation, using:
* The Moles framework – a testing framework that allows you to isolate .NET code by replacing any method with your own delegate, bypassing any hard-coded dependencies in the .NET code.
* Microsoft Pex – an automated testing tool that exercises all the code paths in a .NET code, identifies potential issues, and automatically generates a test suite that covers corner cases.
Microsoft Pex and the Moles framework help you overcome the difficulty and other barriers to unit testing applications for SharePoint Foundation, so that you can prioritize unit testing in your strategy to reap the benefits of greater defect detection in your development cycle.
The Pex and Moles sample code works through this example. It shows you how to mock out all the dependencies required from this quite typical piece of code. Taking a quick look we have these dependencies:
  • SPSite returning a SPWeb
  • Lists on web
  • then GetItemById on Lists returning an SPListItem as item
  • SystemUpdate on item
1
2
3
4
5
6
7
8
9
10
public void UpdateTitle(SPItemEventProperties properties)
{
  using (SPWeb web = new SPSite(properties.WebUrl).OpenWeb())
  {
    SPList list = web.Lists[properties.ListId];
    SPListItem item = list.GetItemById(properties.ListItemId);
    item["Title"] = item["ContentType"];
    item.SystemUpdate(false);
  }
}
Here’s a quick sample to give you a feeling if don’t want to read the pdf. In this code, the first thing to know is that we use a “M” prefix by convention to intercept classes. In this case,MSPSite intercepts the SPSite and then returns a MSPWeb on which we override the Lists and return a MSPListCollection.
1
2
3
4
5
6
7
8
9
10
11
12
13
string url = "http://someURL";
 
MSPSite.ConstructorString = (site, _url) =>
{
  new MSPSite(site)
  {
    OpenWeb = () => new MSPWeb
    {
      Dispose = () => { },
      ListsGet = () => new MSPListCollection
 
 ...
There is no doubt that Pex and Moles is up to the job of mocking these out. Go and look at the article yourself. Others have commented on getting use to the syntax and I agree that because I am unfamiliar it is not as easy as say Moq. But it seems similar to MSpec. That’s not my gripe. My grip is that the tool helps bake badness into my design. Take the example where the sample adds validation logic into the above code, the authors seem to think that this is okay.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
public void UpdateTitle(SPItemEventProperties properties)
{
  using (SPWeb web = new SPSite(properties.WebUrl).OpenWeb())
  {
    SPList list = web.Lists[properties.ListId];
    SPListItem item = list.GetItemById(properties.ListItemId);
     
    string content = (string)item["ContentType"];
    if (content.Length < 5)
      throw new ArgumentException("too short");
    if (content.Length > 60)
      throw new ArgumentOutOfRangeException("too long");
    if (content.Contains("\r\n"))
      throw new ArgumentException("no new lines");
    item["Title"] = content;
 
    item.SystemUpdate(false);
  }
}
In the sample, described as “a more realistic example to test”, it just encourages poor separation when writing line of business applications. This is validation logic and there is no reason whatsoever to test validation alongside the need for data connections. Furthermore, there is little separation of concerns around exception throwing and handling and logging.
I’m therefore still frustrated that we are being shown easy to write code. Both Pex and Moles and TypeMock (regardless of how cool or great they are) are solutions for easy to write code. In this sample, we should see get separation of concerns. There are models, there are validators, there is the checking for validations and error handling. All these concerns can be written as unit tests and with standard libraries.
We will then also need integration tests to check that we can get list items. But these can be abstracted into other classes and importantly other layers. If we do that we will also avoid the duplication of code that we currently see with the using (SPWeb web = new SPSite(properties.WebUrl).OpenWeb()) code block. I am going to return to this in another post which is less about layering but about some specific SharePoint tactics once we have a layering strategy in place.
Posted by at

No comments:

Post a Comment

Subscribe to: Post Comments (Atom)