GitHubFlowVersion TeamCity MetaRunner

The other day I created a TeamCity Meta Runner that allows you to run GitHubFlowVersion against an MSBuild file. I thought I’d share it because it’s cool and demonstrates how powerful this new TeamCity feature is.

If you want to see more Meta Runners check out the GitHub repository that Jetbrains has created.

Disclaimer: this runner doesn’t expose all of the options of GitHubFlowVersion.


<?xml version="1.0" encoding="UTF-8"?>
<meta-runner name="Run Solution using GitHubVersion">
  <description>Run Solution using GitHubVersion</description>
  <settings>
    <parameters>
      <param name="mr.GitHubFlowVersion.SolutionFile" value="" spec="text description='The .sln file relative to the working directory (or any MSBuild file).' display='normal' label='Solution (.sln):' validationMode='notempty'" />
    </parameters>
    <build-runners>
      <runner id="RUNNER_14" name="" type="Ant">
        <parameters>
          <param name="build-file">
          <![CDATA[
            <project name="MetaRunner">
              <property name="mr.GitHubFlowVersion.Version" value="1.3.2" />
              <target name="downloadGitHubFlowVersion">
                <echo>Downloading and extracting latest GitHubFlowVersion.exe...</echo>
                <exec executable="%teamcity.tool.NuGet.CommandLine.DEFAULT.nupkg%\tools\NuGet.exe" dir="${teamcity.build.tempDir}" failonerror="true">
                  <arg line="install GitHubFlowVersion -Version ${mr.GitHubFlowVersion.Version} -OutputDirectory &quot;${teamcity.build.tempDir}\packages&quot;"/>
                </exec>
                <echo>Downloaded and extracted latest GitHubFlowVersion.exe.</echo>
              </target>
              <target name="run" depends="downloadGitHubFlowVersion">
                <echo>Running GitHubFlowVersion.exe...</echo>
                <exec executable="${teamcity.build.tempDir}\packages\GitHubFlowVersion.${mr.GitHubFlowVersion.Version}\tools\GitHubFlowVersion.exe" dir="${teamcity.build.workingDir}" failonerror="true">
			<arg line="/WorkingDirectory &quot;${teamcity.build.workingDir}&quot; /UpdateAssemblyInfo /ProjectFile=&quot;${mr.GitHubFlowVersion.SolutionFile}&quot;"/>
                </exec>
                <echo>Finished running GitHubFlowVersion.exe.</echo>
              </target>
            </project>
          ]]>
          </param>
          <param name="build-file-path" value="build.xml" />
          <param name="target" value="run" />
          <param name="teamcity.coverage.emma.include.source" value="true" />
          <param name="teamcity.coverage.emma.instr.parameters" value="-ix -*Test*" />
          <param name="teamcity.coverage.idea.includePatterns" value="*" />
          <param name="teamcity.step.mode" value="default" />
          <param name="use-custom-build-file" value="true" />
        </parameters>
      </runner>
    </build-runners>
    <requirements />
  </settings>
</meta-runner>

WordPress 2014 Theme Full Width

I recently decided to upgrade my theme from WordPress’ 2011 Theme to the new 2014 one. I think the typography of the 2014 one is absolutely beautiful. The problem I had with the built-in theme though is that it restricts the content to a 474px width, which might be fine for text-only posts, but when you have a lot of images and in particular source code snippets then it’s simply too squishy.

What I ended up doing to fix the problem was installing the Simple Custom CSS plugin and then adding the following CSS:

.site-content .entry-header,
.site-content .entry-content,
.site-content .entry-summary,
.site-content .entry-meta,
.page-content,
.post-navigation,
.image-navigation,
.comments-area {
	max-width: 900px!important;
}

.site-header,
.site {
    max-width: none!important;
}

Making Intent Clear / Derived Values [Automated Testing Series]

This is part of my ongoing Automated Testing blog series:

Making Intent Clear

I think one of the most important things when writing tests (apart from consistency) is that they are clear in intent. If you buy into the notion that tests form part of the documentation of your system then it’s really important, like all good documentation, that the tests are both readable and understandable.

I think there are a number of techniques that can help with this in various situations and there are three in particular that I will be covering in this sub-section of the blog series. I have already covered test naming and I think that has a big impact on clarity of intent.

Derived Values

There are a number of excellent blog posts by Mark Seemann (@ploeh) in his zero-friction TDD series that I have found useful in my ongoing research and one in particular that really resonated with me was the concept of derived values.

Consider the following code:

public static class StringExtensions
{
    public static string ReverseString(this string str)
    {
        return string.Join("", str.Reverse().ToArray());
    }
}

public class NaiveTest
{
    [Test]
    public void GivenAString_WhenInverting_ThenReversedStringWillBeReturned()
    {
        const string str = "a string";
        var result = str.ReverseString();
        Assert.That(result, Is.EqualTo("gnirts a"));
    }
}

public class DerivedValueTest
{
    [Test]
    public void GivenAString_WhenInverting_ThenReversedStringWillBeReturned()
    {
        const string str = "a string";
        var expectedResult = str.Reverse().ToArray();

        var result = str.ReverseString();

        Assert.That(result, Is.EqualTo(expectedResult));
    }
}

public class DataDrivenTest
{
    [Test]
    [TestCase("", "")]
    [TestCase("a", "a")]
    [TestCase("ab", "ba")]
    [TestCase("longer", "regnol")]
    [TestCase("a string with space", "ecaps htiw gnirts a")]
    [TestCase("num3rics&punctua10n!@$", "$@!n01autcnup&scir3mun")]
    public void GivenAString_WhenInverting_ThenReversedStringWillBeReturned(string input, string expectedResult)
    {
        var result = input.ReverseString();

        Assert.That(result, Is.EqualTo(expectedResult));
    }
}

This is a fairly contrived example, but it helps illustrate a few things:

  • The NaiveTest is hard to infer understanding at a glance – you can eventually reason an understanding about the relationship between the input and the output because of the name of the test in combination with common sense, but it’s not easy and thus I think it’s not a great test (it’s still clear AAA so it’s certainly not awful).
  • The DerivedValueTest is what Mark was describing – this is much better because the relationship between the input and result is very clear in the first two lines of the test and you immediately know a) what is being tested and b) how it should work.
    • Of note is that the implementation is the same as the real implementation – this could be a problem if the developer decides to simply copy the implementation into the test or vice versa
      • Interestingly, by writing the test using proper TDD it wouldn’t matter as much that the implementation was similar to the implementation because in writing the test you would see it fail in the “Red” step and at that time verify that the string being asserted in the test output was in fact the correct reverse string
      • The fact you are then relying on the developer verifying that the result being asserted was correct at the time of writing the test reminds me somewhat of the notion of approval tests (which I find myself using a fair bit to perform complex assertions that can’t easily be expressed in code, but can be easily reasoned “by eye”)
      • It occurs to me that if you were only testing a subset of some complex functionality that you would only need to include a subset of the implementation for the test
      • If you have a team that isn’t disciplined in writing their tests in a TDD fashion (or at least verifying the test is definitely correct) then this approach might make it easier to introduce incorrect tests that are a copy of the implementation and don’t actually test anything (hopefully code review would pick this up though)
    • Where possible you could try and include an alternate implementation of the code under test in your test (with a focus on the implementation being readable and understandable), but even in this case I still think the “Red” step mentioned above is important to make sure you didn’t have an error in your alternative implementation
  • The DataDrivenTest in my opinion is the better test in this case, not just because it provides better code coverage by trying multiple values (since this could easily have been done for the derived value test as well), but also because:
    • The relationship between input and output is made clear by their proximity and the fact that there are simple examples as well as more complex ones (the simple ones help the viewer immediately grok the relationship)
      • I feel that the “proximity” part is the most important bit here (assuming that you can grok the relationship)
      • I think the proximity in the DerivedValueTest is an important factor as well to help with immediate understanding
    • I suspect the edge cases in the above example could go into their own test so that the test name can more clearly reflect the edge case being tested
    • This approach won’t work for all situations – sometimes the logic being tested is complex enough that having the input and expected result side-by-side still won’t allow the reader to glean understanding about the relationship and it’s important to how show the expected result is derived
    • Be pragmatic – use the right approach in the right situation – derived values is sometimes useful and sometimes showing a series of {input -> expected result} is clearer – I’d say the main thing to be wary of is tests that simply have a value in the final assert and it’s not clear how that value was derived

There is a slight variation to the DataDrivenTest above that I sometimes come across that is also worth mentioning – complex example generation. It’s a strategy to avoid the situation described above where showing the derived value involves duplicating the implementation logic in situations where it’s really complex to work out that logic, but easy(ier) to come up with an example of the logic in action. I often find myself this technique for date logic – writing the date logic as part of the test never gives me a lot of confidence since it’s so darn complex to figure out (I hate programming dates/time logic). In these situations I like to pull up a calendar and pick some candidate examples for the logic I’m trying to implement.

A couple of examples are shown below, pulled from a codebase I work on (with some tweaks to generalise the second test so it’s non-identifying):

    // From http://www.timestampgenerator.com/1352031606/#result
    [TestCase("2012-11-04 12:20:06", 1352031606)]
    [TestCase("2012-11-03 23:59:59", 1351987199)]
    [TestCase("2012-02-29 13:00:01", 1330520401)]
    public void GivenDate_WhenConvertingToUnixTimestamp_ItShouldBeCorrect(string inputDate, int expectedTimestamp)
    {
        var date = DateTime.Parse(inputDate);

        var timestamp = date.ToUnixTimestamp();

        Assert.That(timestamp, Is.EqualTo(expectedTimestamp));
    }

    /* unix $ cal 8 2004
     *      August 2004
       Su Mo Tu We Th Fr Sa
        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
     */
    [Test]
    // Day before date during weekend
    [TestCase("2004-08-09", "2004-08-06", true)]
    // Day before date during week
    [TestCase("2004-08-10", "2004-08-09", true)]
    // Consecutive business days
    [TestCase("2004-08-09", "2004-08-09", true)]
    [TestCase("2004-08-09", "2004-08-10", true)]
    [TestCase("2004-08-09", "2004-08-11", false)]
    [TestCase("2004-08-09", "2004-08-12", false)]
    // Include Weekend
    [TestCase("2004-08-12", "2004-08-13", true)]
    [TestCase("2004-08-12", "2004-08-14", true)]
    [TestCase("2004-08-12", "2004-08-15", true)]
    [TestCase("2004-08-12", "2004-08-16", false)]
    // Start on Weekend
    [TestCase("2004-08-13", "2004-08-16", true)]
    [TestCase("2004-08-13", "2004-08-17", false)]
    public void WhenValidatingConnectionDate_ThenThereShouldBeAnErrorOnlyIfTheDateIsLessThan2BusinessDaysAway(string now, string date, bool expectError)
    {
        _model.ConnectionDate = DateTime.Parse(date);
        var dateTimeProvider = DateTimeProviderFactory.Create(DateTime.Parse(now));
        var modelState = new ModelStateDictionary();

        _model.Validate(modelState, dateTimeProvider);

        if (expectError)
            Assert.That(modelState[modelStateKey].Errors, Has.Count.GreaterThan(0));
        else
            Assert.That(modelState.ContainsKey(modelStateKey), Is.False);
    }

Props to my colleague Toby Moore for coming up with the idea of using the Unix cal command to generate calendars for pasting in comments above the examples).

In these examples, there is a cognitive load to figure out the relationship between input and expected result, but I don’t think there is a silver bullet in these cases – the test name, multiple examples and the comments above the tests (I think) help anyone maintaining the tests to figure out what is going on. Either way there would be a cognitive load to get your head around the logic since it’s really complex and in this case it’s about trying to minimise that load.

IDDD Course notes

Last month I completed Vaughn Vernon‘s 3-day advanced IDDD Workshop. Here are some notes I’ve since written up about the main points I got out of it after re-reviewing the course material. Some notes from a colleague who attended the same course have also been published.

Getting started with DDD

  • DDD is the formation of a ubiquitous language explicitly bound by a “bounded context” – the context is important because the same word can have different meanings
  • Hexagonal architecture / ports and adapters can be used with DDD – there are adapters from the domain to a port (either input or output – could be datasources or other systems)
  • DDD should be used in situations for complex/unknown applications and problem domains that provide a competitive advantage – it is a technique to help uncover these complexities over time in a sustainable way
    • If CRUD is more appropriate – use CRUD
    • Keep in mind that investing in the code generally pays off over the long term
  • DDD can be used with legacy code, but you need to abstract and encapsulate the parts of the legacy code you aren’t modelling
  • DDD is about bridging the gap between technical people and subject matter experts so finding the subject matter experts (they aren’t necessarily your product owner and there might be multiple ones) is important – where possible interact with them directly for best effect
  • A ubiquitous language should include the terms as well as scenarios that describe the context of those “things”
  • Good general rule: Tell, don’t ask (tell objects what you want to do, don’t ask them for data)

Domains, subdomains, bounded contexts

  • Subdomains – identify the pieces of information you need for your project to be successful
    • Where possible have a one to one mapping with a bounded context unless there is a small shared kernel
  • Bounded context contains: interfaces (service or UI), app services, database schema, domain itself
  • Example: pull out user and identity into separate domain, then you can model more explicit things that might be mapped from that context e.g. Author, Collaborator (these could even just be a value object with an id that links to the other context)
  • For each domain look at what domains it (core) is linked to and identify them as: supporting or generic

Context maps / relationships between subdomains

  • It can help to draw context maps to illustrate where the subdomains, contexts and the relationships between them are
  • Ways of joining two separate (sub)domains:
    • Partnership
    • Shared kernel
    • Customer-supplier
    • Conformist
  • When communicating with another domain (via OHS, web service, messaging):
    • ACL
    • Published language
    • Event subscription

Architecture

  • Ports and adapters, {infra -> UI -> application -> domain}, CQRS/ES, event-driven are some of the architectural options
    • Ports don’t have to model the domain e.g. UserInRole REST service even though there is no UserInRole domain object)
    • Adapters can map to/from domain representation e.g. UserInRoleAdapter
    • Ports and adapters allow you to focus on the domain, delay infrastructure concerns and do separate testing
  • Domain services shouldn’t control security (where it’s a cross-cutting concern as opposed to something being modelled) or transactions

Entities / Value objects

  • Use entities to model things that you care about individuality
    • Equality via id
    • Avoid just doing sets – if you have two sets in a row then you are probably missing a behaviour – ask yourself if you missed a set would you be in an inconsistent state?
    • Ensure consistency by using invariants and non-default constructors
  • Use value objects where possible to model things that describe, measure, qualify or quantify the ubiquitous language – should model a conceptual whole
    • Generally immutable, set state via ctor – makes it easy to test and maintain
      • Modification methods should return a new instance
    • Equality by comparing properties
    • Discardable, replaceable and interchangeable
    • If you want you can use value objects to represent the id of a particular entity (e.g. wrap guid)
  • Generating identities
    • User provides
    • Application generates
    • Persistence store generates
    • Anther bounded context provides

Aggregates

  • Main purpose: determine transactional boundaries for consistency
  • Balance between having a large aggregate graph that gives navigational convenience, but can result in higher likelihood of transactional errors that don’t affect consistency / negative size and performance impacts vs small aggregates that might mean a lot of messing about in application code to wrangle multiple aggregates
  • Keep in mind eventual consistency – do multiple aggregates have to be kept consistent?
    • event processing / batch processing etc. – get business to specify the max time to be consistent
  • Where possible reference between aggregates by id – less memory, faster to load, better garbage collection
  • Can use double dispatch from an application service to pass instances between aggregates to perform actions
  • Domain events are non-async
    • This means you can respond to a domain event within the same transaction
    • You can have a listener that then publishes the events to a bus (and from there they can be handled async)
    • Who job is it to make the data consistent?
      • If the end user then use transaction consistency if another user or the system then eventual consistency should be fine

Domain Services

  • A domain service is a part of the domain model that is a non-transactional, lightweight stateless operation that doesn’t have a natural home in an entity
    • If you find yourself using static methods in your domain entities then it’s a good indicator that you need a domain service
    • They can be passed into models and used via double dispatch or the model can be passed into them

Domain Events

  • Domain events inform subscribers of the facts about past happenings in a bounded context
  • When domain experts use triggering works then it’s a good sign you might need a domain event:
    • When
    • If that happens
    • Inform me if
    • Notify me if
    • An occurrence of
  • You can persist events along with state (or just the events if using event sourcing)
  • Events can be published outside of the bounded context or processed asynchronously by forwarding them to a message exchange
    • Bus
    • Decoupled publishing e.g. atom feed
  • Event-driven modelling exercise – Trello is a good example
    • Good example to do with a domain expert as a first step to fleshing out a domain
    • Leads to a good model if you plan on using event sourcing
    • 1. Model the events in time order (verb, past tense)
    • 2. Model the commands that would create the events (imperative exhortation)
    • 3. Model the aggregates that would participate in the command/event (noun)

Modules (namespaces)

  • Name the modules as per the ubiquitous language (e.g. aggregates or concepts to group an aggregate)
  • Can have child namespaces e.g. concept -> aggregate
  • Don’t create modules based on type of component (e.g. include services with the entities)
  • Try not to couple between modules and where there are dependencies try and use acyclic graphs

Factories

  • Where possible construct objects inside of domain services and entities so the ubiquitous language is expressed
  • If you have all the parameters and the construction is simple it’s ok to new up an object in an app service

Acceptance Tests Structure [Automated Testing Series]

This is part of my ongoing Automated Testing blog series:

Acceptance Tests Structure

When writing high level acceptance tests (as opposed to unit tests) I will always try to use separate methods for the Given, When and Then since usually the Given and possibly When are more complex so there might need to be multiple methods for each. My favourite framework for writing tests with this structure by far is Bddfy (disclaimer: I am a core contributor of the TestStack organisation). When I say high-level acceptance tests I usually refer to automated UI / full system tests, but it’s possible they could also be complex subcutaneous tests (where subcutaneous tests fit in is part of my thinking I’m not quite sure on yet).

I’ve said before and I still maintain that consistency is the most important aspect when it comes to keeping a software application maintainable so I think that within a particular set of tests if you are writing some that are single method Arrange, Act, Assert tests then you shouldn’t mix those tests with something like Bddfy since it’s wildly different. I feel that the techniques I described for structuring tests using test-per-class in the last post in the series is OK to mix with AAA tests though as I discussed in that post.

The above two paragraphs have led me to the following thoughts:

  • I keep my high-level acceptance tests in a separate project from my unit/integration/etc. tests since they are:
    • Inconsistently specified as discussed above
    • As seen below the way I structure the tests into namespaces / folders and the way I name the test class is very different too
    • They have a different purpose and intent – one is to check your implementation is sound / help design the technical implementation and the other is to specify/check the business requirements of the system i.e. the concept of an executable specification
  • If possible use something like Bddfy and a Specification base class (see below) that allows you to specify the implementation of your scenario
    • Yes I know about SpecFlow, but I don’t think that the maintenance overhead is worth it unless you actually have your product owner writing the specifications, but by all accounts I’ve heard (and based on my experiences) it’s tricky for them to get it right and the developers end up writing the scenarios anyway – do yourself a favour and use a framework that is built for developers and get the devs to sit with the product owner to write the test - that’s what they are good at!
    • One of the many cool things about Bddfy is its reporting functionality – out of the box it generates a HTML report, but it’s also flexible enough to allow you to define your own output; I think this fits in nicely with the idea of an executable specification
  • I’ve used the base class shown below to make it really easy to define Bddfy tests (then you simply need to specify methods according to the Bddfy conventions and it will automatically pick them up
    • If you want to reuse Given’s, When’s or Then’s then simply use inheritance (e.g. you might define an AuthenticationUserSpecification that extends Specification and provides a GivenUserIsAuthenticated method)
    • If you need to use a data-driven test then see below for an example of how to do it

Basic Specification base class

You can add any setup / teardown that is required to this for your purposes or wrap the run method code if needed (e.g. for automated UI tests I catch any exception from the this.BDDfy() call and take a screenshot).

public abstract class Specification
{
    [Test]
    public virtual void Run()
    {
        this.BDDfy();
    }
}

For a more advanced example of this base class including a generic version that identifies the SUT and provides auto-mocking (in this case the tests are unit tests rather than acceptance tests) check out the TestStack.Seleno tests.

Example of extracting common parts of scenarios

Here is an example of how you might pull out common logic into a customised specification base class. In this case it is demonstrating what it might look like to have a base class for when the scenario starts with the user not being logged in when using the TestStack.Seleno library.

public abstract class UnauthenticatedSpecification : Specification
{
    protected MyAppPage InitialPage;

    public void GivenNotLoggedIn()
    {
        InitialPage = Host.Instance
            .NavigateToInitialPage<MyAppPage>()
            .Logout();
    }
}

Example test class

Here is an example of what an actual test class might look like. In this case it is a test that ensures a user can register for an account on the site and extends the example UnauthenticatedSpecification above. There is some code missing here about how the page objects are implemented, but it’s fairly readable so you should be able to follow what’s happening:

public class ValidRegistration : UnauthenticatedSpecification
{
    private RegistrationViewModel _model;
    private MyAppPage _page;
    private Member _member;

    public void AndGivenValidSubmissionData()
    {
        _model = GetValidViewModel();
    }

    public void AndGivenUserHasntAlreadySignedUp()
    {
        // In this instance the Specification base class has helpfully exposed an NHibernate Session object,
        //  which is connected to the same database as the application being tested
        Session.Delete(Session.Query<Member>().SingleOrDefault(m => m.Email == _model.Email));
    }

    public void WhenVisitingTheJoinPageFillingInTheFormAndSubmitting()
    {
        _page = InitialPage
            .Register()
            .Complete(_model);
    }

    public void ThenTheMemberWasCreated()
    {
        _member = Session.Query<Member>()
            .Single(m => m.Email == _model.Email);
    }

    public void AndTheMembersNameIsCorrect()
    {
        _member.FirstName.ShouldBe(_model.FirstName);
        _member.LastName.ShouldBe(_model.LastName);
    }

    public void AndTheMemberCanLogIn()
    {
        _page.Login()
            .Successfully(_model.Email, _model.Password);
    }

    private RegistrationViewModel GetValidViewModel()
    {
        // This is using the NBuilder library, which automatically populates public properties with values
        // I'm overriding the email and password to make them valid for the form submission
        return Builder<RegistrationViewModel>.CreateNew()
            .With(x => x.Email = "email@domain.tld")
            .With(x => x.Password = "P@ssword!")
            .Build();
    }
}

Demonstration of data-driven test

I mentioned above that it was possible to have a data-driven test with the Specification base class. I can do this by using a hack that Jake Ginnivan showed me when we were creating acceptance tests for GitHubFlowVersion for XUnit and a technique I have found works for NUnit. I’ve put a full code example in a gist, but see below for an abridged sample.

I should note I’ve only tried these techniques with the ReSharper test runner so it’s possible they function differently with other runners. Another option would be to not inherit the base class for the situations where you need data-driven tests, but I typically find that I would then loose a bunch of useful setup/functionality when doing that; YMMV.

NUnit

You might have noticed I set the Run method in the Specification base class to virtual. If you do this then you can do the following in your test class and it will ignore the test method in the base class and use the data-driven one in this class – it does show the base test up as being ignored though – you could also assign a category to the test and not run that category:

    [Test]
    [TestCase(1)]
    [TestCase(2)]
    [TestCase(5)]
    public void RunSpecification(int someValue)
    {
        _privatePropertyTheTestMethodsUse = someValue;
        base.Run();
    }

    [Ignore]
    protected override void Run() { }

XUnit

You don’t have to have the base Run method virtual for this one (but it doesn’t hurt if it is either). Also, the test from the base class comes up as Inconclusive in the ReSharper test runner (just ignore it):

    [Theory]
    [InlineData(1)]
    [InlineData(2)]
    [InlineData(5)]
    public void RunSpecification(int someValue)
    {
        _privatePropertyTheTestMethodsUse = someValue;
        base.Run();
    }

    public new void Run() {}

Namespaces / folder structure

When structuring the acceptance test project I like to have a folder/namespace called Specifications and underneath that a folder for each feature being tested. Then there can be a file within each feature for each scenario being tested (I generally name the scenario with a short name to easily identify it).

So, for instance the valid registration test above might be found at Specifications.Registration.ValidRegistration.

General Test Structure [Automated Testing Series]

This is part of my ongoing Automated Testing blog series:

General Test Structure

Recently I found myself experimenting with demon-coder, fellow Readifarian, and all-round-nice-guy Ben Scott (o/ – that’s for Ben; he will understand) with trying to extract shared Given, When and/or Then sections of tests by using a pattern whereby the Given and possibly When of a test happens in the test setup (or constructor if using XUnit). This allows each assertion (or Then) that you want to make to be in a separate test method and avoids both the problem of multiple somewhat/completely unrelated asserts in a single test and massive duplication of test setup.

I realise there are other ways of dealing with the duplication such as abstracting common logic into private methods (this is the technique I have used in the past), but I’ve found the above solution to be much nicer / cleaner and clearer in intent. It’s also worth noting that there are frameworks that directly support testing with the Given, When, Then structure and I cover that further in a later post in this series (as well as why I don’t just use them all the time).

I’ve created a set of code samples to illustrate the different techniques at https://gist.github.com/robdmoore/8634204:

  • 01_Implementation.cs contains an example mapper class to be tested – it is mapping from some sort of measurement of a value that has been taken (say, from an API or a hardware device) with a confidence and some kind of identifier to an object that represents the identity (broken down into component parts), the measurement value and an adjusted value (based on the confidence). It’s a fairly contrived example, but hopefully it’s understandable enough that it illustrates my point.
  • 02_MultipleAsserts.cs shows a single Arrange Act Assert style test that contains multiple asserts for each of the mapped properties and has description strings for each assert so that you can tell which assert was the one that failed.
  • 03_RelatedMultipleAsserts.cs partially addresses the main problem in the first test (multiple somewhat unrelated asserts) by grouping the related asserts together – this introduces repetition in the tests though.
  • 04_AbstractedCommonCode reduces the duplication by abstracting out the common logic into private methods and provides a separate test for each assert – because of this the assertions don’t need a description (since there is one assert per test), but the test class is a lot more verbose and there is duplication both across test names and test content.
  • 05_GivenAndWhenInSetup.cs demonstrates the technique I describe above; in this case the constructor of the class (it also could have been a [SetUp] method) contains both the Arrange/Given and Act/When part of the test and each test method is both very slim (containing just the assertion) and is named with just the Then.
    • In NUnit you can use [SetUp] and in XUnit the constructor, but it’s worth noting that this means that code will run for every test (which is usually fine), however if the code you are testing takes a reasonable amount of time to run then you can use a constructor or [TestFixtureSetUp] in NUnit or a static constructor (if you need to also use the constructor (say, to perform the When or allow for slightly different behaviour across inherited classes when using inheritance) since the fixtures get injected after the constructor is called) or IUseFixture<TFixture> in XUnit.
    • It’s worth noting this class is still a lot bigger than the single AAA test , but for a situation like this I feel it’s a lot more understandable and better encapsulates the behaviour being tested; this technique also comes into it’s own when there is duplication of Given and/or When logic across multiple AAA tests (you can use inheritance to reuse or extend the Given/When code)
  • 06_ConcernForBaseClass.cs shows a variation of the above strategy that provides a common base class that can be used in tests to help make the Given and When code more explicit as well as identifying what the subject under test is.

Another technique to reduce the setup of tests is to use AutoFixture’s auto data attributes (XUnit and NUnit are supported) to inject any variables that you need into the test. I’m not going to cover that in this post at this stage because a) I haven’t used it in anger and b) I suspect it’s too complex for teams that aren’t all experienced (i.e. it’s a bit too “magic”). It is very cool though so I highly encourage you to at least evaluate it as an option.

There are a number of things to note about what I’ve been doing with this technique so far:

  • We still managed to keep the convention of having a test described by Given (where appropriate; sometimes you don’t need the Given), When and Then by ensuring that the combination of the class name and the test name cater for them
    • e.g. a class called GivenX_WhenY or just WhenY with a bunch of test methods called ThenZ or class called GivenX with a bunch of test methods called WhenY_ThenZ
  • You can reuse and extend common parts of your test logic by using inheritance e.g. define a base class with common Given and/or When and extend it for different variations of the Given and either reuse Then’s from the base class and/or define new ones for each inheriting class
    • I’ve seen some people use this technique with 5 levels deep of nested classes with one line of code being reused in some of the hierarchy; while I’m a massive fan of DRY I think that there is a point where you can go overboard and in the process lose understandability – 5 level deep nested test classes is in my opinion completely overboard
  • An interesting side-effect of this approach is that rather than having one test class per thing being tested you end up with multiple classes
    • I have always followed a convention of putting a test file in the same namespace as the class being tested (except in the Test project and post-fixing with Tests) e.g. MyApp.Commands.CreateInvoiceCommand would be tested by MyApp.Tests.Commands.CreateInvoiceCommandTests, but that doesn’t work when you have multiple test files so what I’ve taken to doing is making a folder at MyApp.Tests\Commands\CreateInvoiceCommandTests, which contains all the test classes
    • This allows me to mix and match folders (where needed) and single test classes (sometimes a single class is enough so I don’t bother creating a folder) while keeping it clear and consistent how and where to find the tests
  • Not all tests need to be written in this way – sometimes a simple, single AAA test (or a bunch of them) is enough
    • As you can see from the above code samples the AAA tests tend to be terser and quicker to write so when there isn’t much duplicated logic across tests and you only need one assertion or the assertions are one logical assertion (i.e. belong in a single test) then there is no reason to stray from single AAA tests in my opinion
    • I don’t feel that a combination of AAA tests and the common setup tests cause a consistency issue because it’s fairly easy to trace what’s happening and the common setup logic falls within the normal bounds of what you might have for AAA tests anyway
    • I find it’s easier to create data-driven tests using single method tests because you have so much more flexibility to do that using NUnit and XUnit – if you find a good way to do it with test-per-class rather than test-per-method let me know!
      • NUnit’s TestFixture attribute goes some of the way, but it’s not as powerful as something like TestCase orTestCaseSource
  • There is a base class (ConcernFor) that we were using for some of the tests that I included in the last code sample above

Test Naming [Automated Testing Series]

For the last 6 months I’ve been thinking and reading a lot about how best to write automated tests for applications – including data generation, structure, naming, etc. This blog series is a foray into my current thinking (which will probably change over time; for instance I’m looking back at tests I wrote 6 months ago that I thought were awesome and now thinking that I hate them :P). I’m inviting you to join me on this journey by publishing this series and I encourage thoughtful comments to provoke further thinking and discussion.

Test Naming

I have always been a fan of the Arrange Act Assert structure within a test and started by naming my test classes like <ThingBeingTested>Should with each test method named like A_statement_indicating_what_the_thing_being_tested_should_do. You can see examples of this type of approach in some of my older repositories such as TestStack.FluentMVCTesting.

More recently I’ve started naming my tests with a Given, When, Then description (be it an acceptance test or a unit test). I like the Given, When, Then method naming because:

  • Generally the Given will map directly to the Arrange, The When will map directly to the Act and the Then will map directly to the Assert – this provides a way of quickly cross checking that a test does what the name suggests it should by comparing the name to the implementation
    • I find this really useful when reviewing pull requests to quickly understand that the author of a test didn’t make a mistake and structured their test correctly
    • e.g. it’s probably wrong if there is a Given in the name, but no clear Arrange section
  • It requires that you put more thought into the test name, which forces you to start by thinking about what scenario you are trying to test rather than “phoning in” an arbitrary test name and diving into the implementation and focussing on how you are testing it
    • My gut feel along with anecdotal evidence from the last few projects I’ve worked on – with developers of varying skill levels – suggests this leads to better written, more understandable tests
    • Side note: I really like the idea discussed by Grzegorz Gałęzowski in his TDD e-book about the order in which you should write a TDD test (from the section “Start From The End”) – he suggests you start with the Then section and then work backwards from there
      • I can’t honestly say I usually do this though, but rather just that it sounds like a good idea; generally I know exactly what I want to write when writing a test (yes, it’s possible I’m just naive and/or arrogant ;))

To clarify, I still use Arrange, Act, Assert in my tests. I could write a whole blog post on how I structure the AAA sections in my tests, but luckily I don’t have to because Mark Seemann has already written a really good post that mimics my thoughts exactly.

Windows Azure Whitepapers

My latest whitepaper for my employer, Readify, has been published. That makes two whitepapers I’ve written now.

  • Cloud and Windows Azure Introduction
    Cloud Computing is an often talked about and often misunderstood phenomenon. While many people disagree about what it is, most people agree that it is a revolutionary shift in the way computing resources are consumed and delivered and will have a big impact on the future. This paper consists of two parts; firstly, it discusses what cloud computing is, what benefits cloud computing can deliver to your business and what things to look out for when making a decision about cloud computing. The second part of the paper gives an overview of the Microsoft Windows Azure cloud computing platform and outlines the benefits that it can deliver to your business.
  • Azure Web Apps: Developing and Deploying Web Applications in Windows Azure
    When deploying web applications in Windows Azure there are a number of options available to you. It’s not always immediately clear from the documentation which of the options are the right ones to use in any given situation.This whitepaper will explain the three main options that are available in Windows Azure to deploy web applications and will cover the similarities and differences between them in order to make the decision of which to use easier. It will also cover the different deployment strategies available to you after you have made the decision.

Announcing AzureWebFarm.OctopusDeploy

I’m very proud to announce the public 1.0 release of a new project that I’ve been working on with Matt Davies over the last couple of weeks – AzureWebFarm.OctopusDeploy.

This project allows you to easily create an infinitely-scalable farm of IIS 8 / Windows Server 2012 web servers using Windows Azure Web Roles that are deployed to by an OctopusDeploy server.

If you haven’t used OctopusDeploy before then head over to the homepage and check it out now because it’s AMAZING.

The installation instructions are on the AzureWebFarm.OctopusDeploy homepage (including a screencast!), but in short it amounts to:

  1. Configure a standard Web Role project in Visual Studio
  2. Install-Package AzureWebFarm.OctopusDeploy
  3. Configure 4 cloud service variables - OctopusServerOctopusApiKeyTentacleEnvironment and TentacleRole
  4. Deploy to Azure and watch the magic happen!

We also have a really cool logo that a friend of ours, Aoife Doyle, drew and graciously let us use!AzureWebFarm.OctopusDeploy logo

 

It’s been a hell of a lot of fun developing this project as it’s not only been very technically challenging, but the end result is just plain cool! In particular the install.ps1 file for the NuGet package was very fun to write and results in a seamless installation experience!

Also, a big thanks to Nicholas Blumhardt, who gave me some assistance for a few difficulties I had with Octopus and implemented a new feature I needed really quickly!

Using Pull Requests for commercial/private/proprietary development

Developers who are familiar with open source will likely be aware of pull requests, which were created and popularised by GitHub as a way of providing some automation, visibility and social interaction around merging software changes. It replaces the old school notion of emailing patch files to each other as well as providing more visibility and interaction over pushing to the same branch (aka mainline development).

This is a post I’ve been meaning to write for a while. As a Senior Consultant for Readify I find myself spending a lot of time mentoring teams and as a part of that I’m constantly paying attention to trends I notice and picking up / experimenting with techniques and processes that I can introduce to teams to get positive outcomes (improved efficiency, quality, maintainability, collaboration etc.). In my experiences so far I’ve found that the single most effective change I have introduced to a software team that has improved the quality of software output is to introduce pull requests. It also has a positive effect on other things like improved collaboration and collective code ownership.

I can’t claim to have invented this idea – I got the idea from listening to the experiences that my fellow consultants Graeme Foster and Jake Ginnivan had and there are certainly examples of companies using it, not the least of which is GitHub itself.

What are pull requests?

Essentially, a pull request allows a developer to make some commits, push those commits to a branch that they have access to and create a request to a repository that they don’t necessarily have access (but also could) to have their commit(s) merged in. Depending on the software that you are using, a pull request will generally be represented as a diff, a list of the commits and a dialogue of comments both against the pull request itself and against individual lines of code in the diff.

Why are pull requests useful for open source?

Pull Requests are awesome for open source because they allow random third parties to submit code changes easily and effectively to projects that they don’t usually have commit access to and for project maintainers to have a conversation with the contributor to get the changes in a state where they can be merged.

This lowers the barrier of entry to both maintainers and contributors, thus making it easier for everyone involved :).

Why are pull requests useful for commercial development?

On first thought you might think that commercial development is significantly different from open source since the people submitting code changes are on the same team and there will be existing code review processes in place to deal with code quality.

Based on my experiences and other people I know there are definitely a range of advantages to using pull requests for commercial development (in no particular order):

  • If you have enough developers and a big enough project/product then you might actually have a similar setup to an open source project in that the product might have a “core team” that look after the product and maintaining standards and consistency and other developers in the company can then submit pull requests to be reviewed by the core team
  • Pull requests give a level playing field to the whole team – it encourages more junior or shy developers to “safely” review and comment on commits by more senior developers since it takes away the intimidation of doing it in person
  • It provides a platform for new developers to get up-skilled quicker by providing:
    • Easy to digest, focussed examples of how to implement certain features that they can browse easily without having to ask how to find them
    • Confidence to raise pull requests without the stress of needing to know if the code is OK
    • Line-by-line comments to help them identify what they need to change to get their code “up to scratch”
    • This has a positive effect on the new developer themselves as well as the team since the learning is founded in practical application (which in my view is always the most efficient way to learn) and the person potentially needs less time from the team to get up to speed (thus having a smaller “burden”)
  • In much the same way as it helps with up-skilling it helps with learning for more junior developers
  • It improves the safety of making changes to the codebase by ensuring that changes are looked at by at least one person before being merged into mainline
  • It improves the consistency/maintainability and quality of the codebase by ensuring all changes are reviewed by at least one person
  • It makes it more likely that changes that would normally pass code review because they were minor enough that it didn’t warrant rejecting a code review / going back and fixing what was already there are more likely to get fixed before being merged in
  • There is tooling out there (e.g. TeamCity’s pull request integration) that can integrate with pull requests to provide assurances that the pull request is OK to merge (and potentially automatically merge it for you)
  • The ability to have threaded comments on individual lines of code makes it really easy to have a contextual conversation about parts of the code and arrive at better designs because of it
  • Despite what code review processes you might have in place I suspect that most of the time it’s more likely code will get reviewed using pull requests because in my experience code reviews don’t always happen regardless of what development processes exist, whereas a pull request would always be reviewed before merging
  • Pull requests are pull-based rather than push-based i.e. the reviewer can review the code when they are not in the middle of something so there is less context-switching overhead (note: important/blocking pull requests might be a reason to interrupt someone still, but in my experience those pull requests are not the majority)
  • If you are working with developers across multiple timezones then the above point means that code reviews are easier to perform since they are pull-based
  • You can use pull requests to raise work-in-progress for early feedback (reducing feedback cycles :)) or for putting up the result of spikes for team comment
  • If approached in the right way then it helps teams with collective code ownership because there is a lot of back-and-forth collaboration over the code and everyone in the team is free to review any pull request they are interested in

As you can see – it’s a pretty convincing list of points and this summarises pretty well why pull requests are the most effective change I’ve made to a development team that has affected code quality.

So, nothings perfect; what are the downsides/gotchas?

There are only a few I’ve noticed so far:

  • If there is a lot of churn on the codebase from a lot of disparate teams/developers with potentially conflicting, high-priority tasks and there is no “core team” looking after the project then there is a tendency for developers to:
    • Work with other developers in their team to quickly merge the pull request without much inspection
    • Worse still – developers might just merge their own pull requests or bypass them completely and push directly to the branch
    • Acknowledge the review comments, but claim that their work is too high priority to not merge straight away and never get around to making the changes
  • If there is someone in the team that is particularly picky then it’s possible that team members might try and avoid that person reviewing their pull requests by asking someone else to review it
  • If there are a lot of changes to be made based on review comments it can sometimes get tricky to keep track of which ones have and haven’t been applied and/or the pull request could drag on for a long time while the developer completes the changes – there is a balance here between merging in the potentially important work in the pull request and spending time fixing potentially minor problems
  • If pull requests are too big they are harder to review and there will be less review comments as a result and things might get missed (this would be a problem without pull requests, but pull requests simply highlight the problem)
  • The diff UI on GitHub and Bitbucket frankly suck compared to local diffing tools. Stash’s pull request diff UI is actually really good though; this does make it a bit more difficult for bigger PRs (but then it’s still easy to pull and review the changes locally so you can get around this limitation)
  • Comments on controversial code changes (or sometimes not so controversial changes) can sometimes get a bit out of hand with more comments than lines of code changed in the PR – at this point it’s clear that the back-and-forth commenting has taken the place of what should be an in-person (or if you are remote video) conversation next to a whiteboard
  • You end up with a lot of local and remote branches that clutter things up, but there are techniques to deal with it

These behaviours are generally unhealthy and are likely to be a problem if you don’t have pull requests anyway (and in fact, pull requests simply serve to visualise the problems!) and should be addressed as part of the general continuous improvement that your team should be undergoing (e.g. raise the problems in retrospectives or to management if serious enough). Hopefully, you are in a situation with a self-organising team that will generally do the right things and get on board with the change :).

Pull requests and continuous delivery

I’m extremelly passionate about Continuous Delivery and as such I have been in favour of a mainline development approach over feature branches for quite a while. Thus, on first hearing about using pull requests I was initially wary because I mistakenly thought it was a manifestation of feature branches. What I’ve since learnt is it definitely doesn’t have to be, and in my opinion shouldn’t be, about creating feature branches.

Instead, the same mindset that you bring to mainline development should be taken to pull requests and the same techniques to enable merging of partially complete features in a way that keeps the code both constantly production-ready and continuously integrated should be used. The way I see it developers need to have a mindset of looking at a pull request as being “a set of changes I want to merge to master” rather than “a complete feature ready to be integrated”. This helps keep pull requests small, easy to review and ensures all developers are regularly integrating all changes against the mainline.

Introducing pull requests to the deployment pipeline adds another step that increases the confidence in your production deployments (the whole point of a deployment pipeline) as well as helping make sure that code in the mainline is always production-ready since the reviewer can (and should) review the changeset for it’s likely effect on production as well as the usual things you would review.

Side note: I love this git branching model description.

Tips

  • Keep PRs small (I think I’ve explained why above well enough)
  • Keep PRs focussed – a PR should contain one logical set of changes – it makes it easier to review and means if there is a problem with one set of changes that it doesn’t block the other set of changes
  • Delete a remote branch after it has been merged (if using Stash/Bitbucket then always tick the “close this branch after PR is merged” box)
  • Keep commits clean – having a PR isn’t an excuse to be lazy about your commits; use good commit messages, keep related logical changes together in a single commit, squash them together where relevant, use rebasing rather than merge commits and think about the history after the PR is merged – don’t make it harder for yourself or your team in the future
  • Don’t let PRs get stagnant – if you are finished with a task take a look at the PRs and merge a couple before moving on; if everyone does this then there will be a constant flow of merges/integration – if a PR is open for more than a day there is a problem
  • Where possible have one PR/branch per-person – you should be able to clean up your commits and force push to your pull request branch and it encourages the PR to be smaller
    • Be incredibly careful when doing a force push that you are not pushing to master
    • If you can turn off the ability to force push to master (e.g. Bitbucket allows this) then do it – better safe than sorry
  • For those situations where you do need to work closely with someone else and feel like you need a PR across multiple people there are two techniques you can use:
    • Identify and pair together on at least the blocking “integration” work, put up a PR, get it merged and then work in individual PRs after that
    • If necessary then put up a pull request and contribute to the branch of that pull request by individual pull requests (i.e. create a featurex branch with a pull request to master and then create separate featurex-subfeaturea, featurex-subfeatureb branches with pull requests to featurex) – try and avoid this because it increases the size and length of time of the integrated pull request – also, consider rebasing the integrated pull request just before it’s pulled in
    • If you plan on submitting a pull request against someone else’s pull request branch then make sure you tell them so they don’t rebase the commits on you (can you see why I try and avoid this? :P)
  • Make sure that PR merges are done with actual merge commits (the merge button in all of the PR tools I’ve used do this for you) that reference the PR number – it makes it easier to trace them later and provides some extra information about the commits that you can look up (this is the only thing you should use merge commits for in my opinion)
  • If your PR is not urgent then leave it there for someone to merge at their leisure and move on, if it’s urgent/blocking then ask the team if someone can review/merge it straight away (from what I’ve seen this is not the common case – e.g. one or two a day out of 10-15 PRs)
  • Tag your PR title with [WIP] to mark a work-in-progress PR that you are putting up for feedback to make sure you are on the right track and to reduce feedback cycles – [WIP] indicates that it’s open for people to review, but NOT to merge
  • In those rare circumstances when a PR is ready to be merged, but isn’t production-ready (maybe it’s a code clean-up waiting for a successful production release to occur first to render the code being deleted redundant for instance) then tag the PR title with [DO NOT MERGE]
  • Unless you know what you are doing with Git never work off master locally; when starting new work switch to master, pull from origin/master, and create and switch to a new branch to describe your impending PR – it makes things a lot easier
    • I almost never do this – I generally just work on master unless editing a previously submitted PR because I find it quicker, but it does require me to constant be very careful about what branches I’m pushing and pulling from!
  • Call out the good things in the code when reviewing – reviews shouldn’t devolve into just pointing out things you think are wrong otherwise they develop negative connotations – if the PR creator did a good job on a bit of complex code then give them a pat on the back!
  • Similarly, avoid emotive phrases like “ugly”, “gross” etc. when describing code; when I’m reviewing code a technique I use to try and not sound condescending is to ask a question, e.g. “Do you think this should be named XX to make it clearer?” – make the pull request into a two-way conversation; work out the best solution collaboratively as a team
  • If you aren’t sure about something don’t be afraid to ask a question; quite often this will highlight something that the author missed or at the very least help you to learn something
  • Have fun! Use funny gifs, use funny branch names – just because we are developing serious software doesn’t mean we can’t enjoy our jobs!

Getting started

I’ve found that the following helps immensely when first getting a team started (but after a while the team is able to sustain itself easily):

  • Having at least one person experienced with Git is really helpful as inevitably people will get themselves confused when first using branches and in particular with rebasing
  • Have at least one person that knows what they are doing ensure that in the first week all PRs are reviewed and merged quickly – this will alleviate the team from needing to worry about keeping on top of merging PRs and instead will concentrate on the semantics of creating pull requests, responding to review comments and updating PRs. Once they get the hang of it then stop reviewing/merging all, but the most important PRs and the team will start to notice there is an increase in PRs and will hopefully self-organise to get them merged and get into the habit of doing that
    • It’s possible this technique might not always work, but it has when I’ve tried it and it assists the team to slowly get the hang of using pull requests rather than dropping a huge change on them all at once
    • If the team doesn’t self-organise then bring up the fact PRs aren’t getting merged in retrospectives and suggest a possible solution of reviewing open PRs directly after standup and team members volunteer to review/merge each of the open ones
  • Developers will naturally approach pull requests like feature branches even if told up front to treat it the same as what you would originally push to mainline with mainline development because of the natural human desire to get things “right” / finished
    • This will lead to enormous PRs that are constantly conflicted and a real pain in the ass for the developer
    • Let this happen and when the PR is finally merged sit down with them and ask them how they think it could be avoided in the future
    • Hopefully they will come to the realisation the PR was too big themselves and because they have made that realisation based on experience they are more likely to keep PRs small

Tools

There are various tools that can help you use pull requests. The ones I have experience with or knowledge of are:

If you are interested in semver and you want to use pull requests (or you don’t and you are using mainline development) then I encourage you to check out the GitHubFlowVersion project by my friend Jake Ginnivan.

Blog about software engineering, web development, agile, C#, ASP.NET and Windows Azure.