Mark Needham

I’ve been spending quite a bit of time setting up NCover and then integrating it into Team City.

I’ve read some posts which cover parts of this process but nothing which covers the end to end process so hopefully my experience can help to fill that void.

Step 1

Download NCover 1.5.8, NCover Explorer 1.4.0.7, NCover Explorer Extras 1.4.0.5 from Kiwidude’s website and the NCover website .

Step 2

Put the following into your Nant build file:

1
2

	<loadtasks assembly="..\lib\NCoverExplorer.Extras\NCoverExplorer.NAntTasks.dll"/>
   	<exec program="regsvr32" workingdir="..\lib\NCover-1.5.8" commandline="/s coverlib.dll"/>

I put this right at the top of the build but I expect it doesn’t matter where it goes as long as it’s called at some stage before NCover and NCover Explorer are called.

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
29
30
31
32

<macrodef name="cover.tests">
	<attributes>
		<attribute name="in.assemblies" />
	</attributes>
	<sequential>	
		<ncover
		    program="..\lib\NCover-1.5.8\NCover.Console.exe"
		    commandLineExe="..\lib\nunit-2.4\nunit-console.exe"
		    commandLineArgs="${build.dir}\UnitTests\UnitTests.dll"
		    coverageFile="${report.dir}\Unit.Test.Coverage.xml"
			assemblyList="${in.assemblies}"
		  />	
 
	  <ncoverexplorer
		program="..\lib\NCoverExplorer\NCoverExplorer.Console.exe"
		projectName="Project"
		reportType="ModuleClassSummary" 
		outputDir="${report.dir}"
		xmlReportName="TestCoverage.xml"
		htmlReportName="TestCoverage.html"
		showExcluded="True"
		satisfactoryCoverage="80" >
		<fileset>
		  <include name="${report.dir}\Unit.Test.Coverage.xml" />
		</fileset>
		<exclusions>
		  <exclusion type="Assembly" pattern="*.Tests" />
		  <exclusion type="Namespace" pattern="*.Tests*" />
		</exclusions>
	  </ncoverexplorer>			  		  
	</sequential>
</macrodef>

This macro can then be called as follows:

1
2
3

<target name="cover.unit.tests"	
	<cover.tests in.assemblies="Project1;Project1" />
</target>

N.B. The projects passed in as the ‘in.assemblies’ argument should be semi colon separated.

Step 3

The next step is to setup the artifacts for your project. From the Team City admin panel navigate to the project configuration settings and select artifacts.

Add the following to the ‘Artifact paths’:

1

TestCoverage.html

It should now show up as a viewable artifact from the project listing page.

Step 4

To get the coverage report to show up on a tab on the build summary page we need to edit the main-config.xml file

The location of this file can be found by browsing to ‘Administration > Server Configuration’ from the Team City admin panel

Add the following line after the other ‘report-tab’ entries in this file:

Potential Problems

I encountered some problems in getting this up and running. They were as follows:

NCover: Profiled process terminated. Profiler connection not established

After some Googling I found this post which explains how to solve the problem.

To summarise this problem occurs when trying to run NCover without Administrative privileges. The coverlib.dll shipped with NCover needs to be registered. This can be done two ways:

1) Put the following code into your build file right at the top

1

<exec program="regsvr32" workingdir="\path\to\ncover" commandline="/s coverlib.dll"/>

2) Run the same command from the command line

1

C:\path\to\NCover-1.5.8>regsvr32 CoverLib.dll

NCover – Requested value ‘/r’ was not found

This error occurred when I was using version 1.0.1 of NCover and to cut a long story short, you need to upgrade to get rid of the problem.

More details are on this post.

The information here has been accumulated from my experiences, this post on NCover integration and the official documentation.

When writing build scripts it’s very easy for it to descend into complete Xml hell when you’re using a tool like nant.

I wondered previously whether it was possible to TDD build files and while this is difficult given the dependency model most build tools follow. That doesn’t mean we can’t apply other good design principles from the coding world however.

Encapsulation is one of the key principles of OOP and it can be applied in build files too. Stephen Chu talks about this in his post on Pragmatic Nant Scripting where he recommends having 3 different levels of targets to help create this encapsulation.

I’ve been trying to follow this advice with our build scripts and today Bernardo made the suggestion of using macros in an English readable way. He calls it OO Scripting – it’s effectively a DSL inside a DSL if you like.

I was having problems with the ncover nant task – the following error message was being thrown every time I called it:

could not find ncover folder in your path in NCoverExplorer.NAntTasks.NCoverUtilities

I managed to find the source code for that class and had a look at it but I couldn’t figure out what was going wrong without debugging through it. The strange thing was that it worked fine from the command line which suggested to me that I was getting something simple wrong.

I created a cover.tests macro to encapsulate the details of how I was executing the coverage.

The plan was to get it working using an exec call to the ncover executable and then phase the ncover nant task back in when I’d figured out what I was doing wrong.

This is what I started out with:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17

<macrodef name="cover.tests">
        <attributes>
                <attribute name="in.assemblies" />
        </attributes>
       <sequential>
                <copy file="\path\to\Coverage.xsl" tofile="${report.dir}\Coverage.xsl" />
 
                <exec program="..\lib\NCover-1.5.8\NCover.Console.exe">
                        <arg value="..\lib\nunit-2.4\nunit-console.exe" />
                        <arg value="${build.dir}\UnitTests\UnitTests.dll" />
                        <arg value="//a" />
                        <arg value="${in.assemblies}" />
                        <arg value="//x" />
                        <arg value="${report.dir}\Unit.Test.Coverage.xml" />
                </exec>
        </sequential>
</macrodef>

//a is the assemblies to include in the report

//x is the name of the report xml file which will be created

The full list is here.

The macro was called like this:

1
2
3

<target name="coverage">
   <cover.tests in.assemblies="Project1;Project2" />
</target>

I substituted the ncover task back in with the same parameters as above and low and behold it worked!

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16

<macrodef name="cover.tests">
        <attributes>
                <attribute name="in.assemblies" />
        </attributes>
        <sequential>
                <copy file="\path\to\Coverage.xsl" tofile="${report.dir}\Coverage.xsl" />
        
                <ncover
                    program="..\lib\NCover-1.5.8\NCover.Console.exe"
                    commandLineExe="..\lib\nunit-2.4\nunit-console.exe"
                    commandLineArgs="${build.dir}\UnitTests\UnitTests.dll"
                    coverageFile="${report.dir}\Unit.Test.Coverage.xml"
                    assemblyList="${in.assemblies}"
                  />        
        </sequential>
</macrodef>

I’m not sure exactly what the problem parameter was but encapsulating this part of the build gave me the option of working that out in a way that impacted very little of the rest of the build file.

*Update*
Fixed the first example to include the opening as pointed out by Vikram in the comments. Thanks again Vikram for pointing that out!

I’ve been having trouble trying to work out how to build our projects in msbuild in release mode without creating the customary pdb files that seem to be created by default.

I tried calling msbuild.exe with the ‘Release’ configuration:

'C:\WINDOWS\Microsoft.NET\Framework\v3.5\MSBuild.Exe ( Proj.csproj /p:OutputPath=\output\path\ 	/p:Configuration=Release)'

To no avail. It still created the pdb file. Next I tried setting the ‘DebugSymbols’ property to false:

'C:\WINDOWS\Microsoft.NET\Framework\v3.5\MSBuild.Exe ( Proj.csproj /p:OutputPath=\output\path\ 	/p:Configuration=Release /p:DebugSymbols=false)'

Still it created the file. Finally I found this post which suggested that you actually needed to make the change in the Proj.csproj file itself.

I changed this part of the file so that DebugType is now ‘none’. It had a value of ‘pdbonly’ when I opened the file.

  none
  true
  bin\Release\
  TRACE
  prompt
  4

The pdb is no longer created.

*Update*
This can also be done by passing /p:DebugType=none as a command line argument as Tim points out in the comments.

Sometimes the simplest things can provide the greatest value to project teams. We often look for a technical solution to problems where something simpler would achieve the same aim.

The Information Wall is as its name may suggest a place where you can put information that people in the team need to know but which they have not (yet) committed to memory.

Examples of things that you could put on an information wall could be:

• Wiki url
• SVN repository url
• Remote machine names & credentials
• Build server url
• Useful phone numbers
• Anything else that’s needed!

Although it may seem to add very little value I think it’s a really good tool for making information accessible to people. It also means you avoid the valueless conversations where team members keep interrupting each other to ask for these details.

Having the wall as close to the development team area as possible is ideal.

I’ve been trying to integrate NCover into our build and probably making life harder for myself than it needs to be.

The title refers to the error message that I was getting when trying to run the ncover nant task on version 1.0.1 of NCover earlier today.

1
2
3
4
5
6
7
8

[ncover] Starting 'C:\Program Files\NCover\ncover-console.exe 
(//r "\long\path\to\tmp392.tmp.ncoversettings" )' in 'C:\my-project\trunk\src'
[ncover] Unhandled Exception: System.ArgumentException: Requested value '/r' was not found.
[ncover]    at System.Enum.Parse(Type enumType, String value, Boolean ignoreCase)
[ncover]    at NCover.Utilities.Arguments.ParseArgument(String arg, 
CommandLineArgument& key, String& value) in C:\to
ols\eclipse3M6\workspace\ncover\src\NCover\Utilities\Arguments.cs:line 192
...

After some inspired Googling my colleague managed to work out that the problem was that you can’t pass a settings file path which has spaces in to the ncover executable, hence the error message. It’s the same problem in handling spaces that I mentioned in an earlier post on msbuild.

The advice on the forum was to upgrade to one of the more recent versions where the bug has been fixed. Downloads of the free version of NCover (it becomes paid for at version 2.0) are available here.

Projects often reach the interesting point where the prototyping and development phases intersect and there are some interesting decisions to make.

From a development point of view the biggest decision is what to do with the code that has been developed.

When developing prototypes the focus tends to be on getting something to work quick and dirty. Not a lot of time is put into considering edge cases or error conditions or any of the other niceties that are needed for software to be usable in an enterprise environment.

There are generally three choices with regards to what to do with the code:

Throw the code away

This would be the favoured choice all things considered as it means that the team can get the dual benefit of writing good quality production code TDD style while also taking on the lessons learnt during prototyping.

If this approach is to actually happen then expectations of the client need to have been set extremely well. It needs to be made clear to them during the prototyping phase that what’s being written is throwaway code and is not production quality.

It is very easy for the client to see the output of prototyping and believe that functionality has been implemented and can now be rolled out.

From their point of view what they would see (from a UI level for example) from code written as a prototype or as production quality code may be no different, and they may think that you are just wasting their money by writing the same thing twice unless this is well handled.

Keep the code and retrospectively refactor it

This option is the middle ground between the two extremes. We keep the prototyping code but add some tests and refactor the code in retrospect.

The code produced probably won’t be as clean as if it had been written with a TDD approach, but it will allow the project to be delivered more quickly in the short term at least.

Edge cases probably won’t be investigated as thoroughly as if the code is written from scratch with complete analysis, but more cases will be considered than if it was not tested at all.

Keep the code as it is

The final option is to keep the code written during prototyping and add edge cases and error conditions around it.

This is the most risky approach because although we have working prototype code we lose the set of regression tests that we would have if it had been developed in a test first approach.

Our mind set will now also be more inclined to think that a piece of functionality is complete and we may end up missing important edge cases which only rear their ugly head when the code is in production.

In a perfect world every prototyping session would result in the code being thrown away and then rewritten using the lessons learned in the initial attempt.

As it is not, it does make me wonder whether it would be best to sacrifice some of the speed in the prototyping phase to write better code and remove some of the pain that will otherwise be felt later on.

When pair programming there are obviously times when you have different opinions about how things should be done.

One of these is the way that we should return from methods. There seem to be two approaches when it comes to this:

Exit as quickly as possible

The goal with this approach is as the title suggests, to get out of the method at the earliest possible moment.

The Guard Block is the best example of this. It is generally used at the start of methods to stop further execution of the method if the parameters are invalid for example:

1
2
3
4
5

public void DoSomething(SomeObject someObject)
{
   if (someObject == null) return;
   // Do some other stuff
}

When used in this way it is very similar to what would be called a pre condition in the world of Design by Contract. It can also be used in methods which return a result:

1
2
3
4
5

public SomeObject GetSomeObject()
{
   if(ThisConditionHappens()) return new SomeObject(thisParameter);
   return new SomeObject(thatParameter);
}

In this example there are only two execution paths so returining early is fine. When there start to become a lot of different branches, however, the idea of returning in each place becomes counter productive and makes the code harder to read.

If that becomes the case, however, there’s probably greater things to worry about with regards to the method than how best to return the result!

Return everything at the end

This approach is fairly self explanatory too. If my somewhat contrived example was written to return everything at the end it would look like this:

1
2
3
4
5
6
7
8
9
10
11
12
13

public SomeObject GetSomeObject()
{
    SomeObject someObject = null;
    if(ThisConditionHappens())
    {
        someObject = new SomeObject(thisParameter);
    }
    else
    {
        someObject = new SomeObject(thatParameter);
    }
    return someObject;
}

This idea starts to come into its own when there are more possible paths of execution, although if this becomes the case it might be better to use the collecting parameter idiom to solve the problem.

Just in case it hasn’t come across, I am a big advocate of the idea of returning early from methods as it means I don’t have to understand the whole internals of a method if I only care about one branch.

If a method gets into such a state that returning early becomes unreadable this would be a clear sign to me that the method is doing too much and should be refactored into smaller chunks.

I’ve been playing around with C#’s Xml libraries today and in particular the XmlWriter class.

I wanted to use it to create an Xml document so I called the XmlWriter.Create() method. One of the overloads for this methods takes in a StringBuilder which I initially thought the XmlWriter used to create the Xml document.

In fact it actually writes the Xml Document into this StringBuilder. This is actually possible to deduct from the documentation provided on the Create method but I only glanced at the type needed initially and misunderstood how it worked.

Now clearly one response to that could be ‘well just read the documentation more closely’ but wouldn’t it be better if the method was actually XmlWriter.CreateInto(StringBuilder output) for example?

I suppose I could write my own extension method to do this but my initial thought was to name the StringBuilder so that I knew what it was doing:

1
2

var xmlCollectingBuilder = new StringBuilder();
var xmlWriter = XmlWriter.Create(xmlCollectingBuilder);

That seems more clear to me and I can see at a glance what the code is doing but something about it feels wrong. I am explicitly referencing the collecting parameter pattern in the code to make it easier for me to understand what’s going on.

Talking through the problem with a colleague we wondered whether it is actually necessary to explicitly reference the pattern being used in code or whether it can be inferred. In the case of collecting parameter, the following code would be an example where this theory makes sense:

1
2
3
4

private void CollectSomeStuff(IList stuff)
{
   stuff.Add("some value");
}

It’s obvious in this example that ‘stuff’ is being used to collect some data so I don’t need to add that information anywhere else – it would be redundant.

However with other patterns it seems that it’s considered good practice to explicitly state their use. The strategy and visitor patterns are two examples of these. It’s almost like there are implicit rules for when we should or should not explicitly state which pattern we are using in our code.

At the end of the day I think the key to writing code is expressing things in a way that other people who read it can understand. We should do whatever we need to do to make that a possibility.

I mentioned in an earlier post my dislike of the passing of null around code, and since then there have been a couple of posts on the subject on the ThoughtWorks blogs.

I had always thought that was a silver bullet for the way that we can handle null objects in our code but it seems from reading other people’s opinions and from my own experience that this is not the case (surprise, surprise!). Several ideas of how to handle nulls came out in these posts and the comments posted on them, and it seems to me that there are several strategies for handling nulls in code.

Return null

The most common way of handling cases where there is no object to return, the code just returns null instead.

The problem with this approach is that the client now has to handle two different types of results. One if an object is returned and another if nothing is returned. This either results in code like this being scattered throughout the code base:

1
2
3
4
5

Car car = carRepository.RetrieveCar(carId)
if(car != null)
{
   car.Drive();
}

Or the client doesn’t bother to handle the null and we end up with a Null Pointer/Reference exception at some stage. Neither of these solution is particularly desirable.

Scala actually has an interesting way of getting around this problem by allowing you to define an interface which informs the client that there is the potential for nothing to be returned, therefore effectively designing a contract which allows the client to deal with it appropriately. The Option[T] idea is explained about half way down the page on this post.

This can also be done in C# to an extent by making use of the nullable operator. In C# it is only useful when you want to make it clear to the client that they may get a null value instead of a primitive.

Overall, this is the simplest solution and probably also the easiest to understand. It just doesn’t result in the cleanest code.

Throw Exception

The idea here is that if there is no object to return, the code throws a custom exception which describes the reason that no object was found.

1
2
3
4
5
6
7
8
9

public Car RetrieveCar(Guid carId)
{
   Car car = FindCarInDatabaseBy(carId);
   if(car == null)
   {
      throw new CarNotFoundException();
   }
   return car;
}

This is certainly more descriptive in telling you why nothing has been returned, but as with ‘Return null’ at some stage the alternate result from the method call needs to be handled. If this is being done with Java’s checked exceptions then it would either need to be handled by the method which calls RetrieveCar or bubbled up through the car. There are a variety of considerations for why you would would choose each way but that discussion is for another post.

As well as this, in theory you could end up with a method returning different Exceptions depending on the reason for the failure to return an object.

I’m not a big fan of handling state via exceptions as I think that exceptions should only be used where something exceptional has happened and I don’t think that failing to find an object can be considered exceptional in most cases.

Null Object Pattern

The null object pattern is the most helpful of the null handling strategies as far as the client is concerned. The client will now know that the object it has been returned is a null object, it will just see an object of the type requested. The devil is in the detail:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20

public class Car
{
   public virtual void Drive()
   {
      // Do some driving
   }

   public static Car NULL
   {
      get { return new NullCar(); }
   }

   class NullCar : Car
   {
      public override void Drive()
      {
         throw new NotImplementedException();
      }
   }
}

1
2
3
4
5
6
7
8
9

public Car RetrieveCar(Guid carId)
{
   Car car = FindCarInDatabaseBy(carId);
   if(car == null)
   {
      return Car.NULL;
   }
   return car;
}

This pattern effectively delays the need to handle the unexpected behaviour exhibited by the RetrieveCar method. Depending on the implementation of the NullCar we might decide to throw a NotImplementedException if a method on it is ever called. Or we can just override every method to do nothing which just hides the problem as far as I’m concerned.

These are the main ways I have come across for handling nulls. I’m sure there are others so if you know of any better ways please let me know.

The way we have setup the build on our continuous integration server, Team City is configured to create a new tag every time the functional tests past successful on that machine.

We then have a QA and Showcase build that we can run to deploy all the artifacts necessary to launch the application on that machine.

Originally I had just written the batch script to take in the tag of the build which the user could find by looking through repo-browser for the last tag created. This quickly became very tedious so I started looking for a way to get the latest tagged revision from the command line.

We thought it would be possible to get this information using svn info but it turned out that the information returned by svn info about revisions doesn’t necessarily refer to the latest created tag. We ended up using svn log and then parsing through that data. It’s a bit messy but it does the job (I name each tagged version of the code as ‘build-{TeamCity-Build-Number}):

1
2

FOR /F "Tokens=2" %%i in ('svn log /tags/path --limit=1 -v ^| find "build"') do set TMP=%%i
FOR /F "Tokens=2 delims=/" %%i in ('echo %TMP%') do SET TAG=%%i

The for loop uses a space as its default delimiter so that’s what the ‘delims=/’ is doing on the second line, the ‘Tokens=2′ allows us to get the second token after the string is split and the ‘^’ in the first command is being used to escape the pipe.