What I hate about BDD

Disclaimer: this is not a TDD vs. BDD post – now that we’ve got that out of the way let’s discuss the thing I hate most about BDD…

I’ve recently started using BDD (again). My tests are still “unit tests” – they do not call a database nor any other external dependency, and since even when writing unit tests I aim to test functionality (mini-requirements) most of them didn’t change too much.
For the subject of this experiment I’ve chosen BDDfy which was a perfect fit for my needs.
Some BDD frameworks uses two “kinds” of files – text file(s) where the scenarios and stories are written (in plain text) and code file(s) that implement the functionality that correspond to steps in those scenarios.
BDDfy on the other hand uses code file which is also used to generate the specification – so less work for me.
By using BDDfy I can transform the following unit test:

[TestMethod]
public void RejectCall_UserRecieveCallAndRejectIt_SendReport()
{
    var fakeMessageEngine = A.Fake();
    var phone = new Phone(fakeMessageEngine);
    var notificationService = new NotificationService(phone);

    var message = new NewCallMessage("otherUserId");
    fakeMessageEngine.OnMessageArrived += Raise.With(new MessageArrivedEventArgs(message)).Now;

    phone.AnswerCall(Answer.Reject);

    var result = notificationService.GetLastReport();

    Assert.AreEqual(ReportType.CallEnded, result.ReportType);
}

To something like this:

[TestClass]
public class NotificationBddTests
{
    IMessageEngine _fakeMessageEngine;
    Phone _phone;
    NotificationService _notificationService;

    public NotificationBddTests()
    {
        _fakeMessageEngine = A.Fake();
        _phone = new Phone(_fakeMessageEngine);
        _notificationService = new NotificationService(_phone);
    }

    [TestMethod]
    public void RejectCall_UserRecieveCallAndRejectIt_SendReport()
    {
        new NotificationBddTests()
            .Given(s => s.RecievedCallFromUser())
            .When(s => s.CallRejected())
            .Then(s => s.EndCallReportAdded())
            .BDDfy();
    }

    private void RecievedCallFromUser()
    {
        var message = new NewCallMessage("otherUserId");
        _fakeMessageEngine.OnMessageArrived += Raise.With(new MessageArrivedEventArgs(message)).Now;
    }

    private void CallRejected()
    {
        _phone.AnswerCall(Answer.Reject);
    }

    private void EndCallReportAdded()
    {
        var result = _notificationService.GetLastReport();

        Assert.AreEqual(ReportType.CallEnded, result.ReportType);
    }
}

And it’s an improvement – the test is more readable and well organized.

But there’s a hidden “price” for writing this kind of tests – one that can cause brittle tests and stability issues in the near future – can you spot the problem?

It’s all about shared state

I have a confession to make: most BDD code samples on the internet cause me to cringe. They all have the same inherent flaw. It’s all about “how we test the real requirements” and “create living specification” but no one ever talks about the fact that the test above could become a huge maintainability issue!

The problem could be shown easily with the following example:

[TestClass]
public class ShoppingCartTests
{
    private readonly ShoppingCart _cart = new ShoppingCart()

    [TestMethod]
    public void EmptyCartTest()
    {
        new ShoppingCartTests()
            .Given(s => s.ShoppingCartIsEmpty())
            .Then(s => s.TotalPriceEquals(0))
            .BDDfy();
    }

    [TestMethod]
    public void AddItemTest()
    {
        var newProduct = new Product(id: "prd-1", price: 100
        new ShoppingCartTests()
            .Given(s => s.ShoppingCartIsEmpty())
            .When(s => s.AddProductToCart(newProduct))
            .Then(s => s.TotalPriceEquals(newProduct.Price))
            .BDDfy();
    }

    private void ShoppingCartIsEmpty() { }

    private void TotalPriceEquals(int expected)
    {
        Assert.AreEqual(expected, _cart.TotalPrice);
    }

    private void AddProductToCart(Product product)
    {
        _cart.Add(product);
    }
}

Do you see the field that is being used by all of the tests.

By using the same instance in all of the tests we accidently cause the test to pass (or fail) depending on the order in which they were run. Obviously it’s an easy fix (in this case), Unfortunately such a simple issue can become a huge problem, the more tests written the more initialization logic required and before you know it it’s the middle of the night and you’re using trying to figure out how could a test that used to work fails some of the times – for no apparent reason.

This is not a BDD problem – I can easily write (and written) the same code without the aid of a BDD framework. However it does seem as if all BDD framework push you towards this issue.

The Solution

Luckily it’s easy to avoid this issue completely – make sure that tests do not share the same state (I said it was easy).

I’ve created a new class called NotificationTestBuilder (because naming things is the hardest thing in programming) and shoved all of the test’s “state” inside it.

And now I can write the same BDD test using single state per test:

public class NotificationTestBuilder
{
    IMessageEngine _fakeMessageEngine;
    Phone _phone;
    NotificationService _notificationService;

    public NotificationTestBuilder()
    {
        _fakeMessageEngine = A.Fake();
        _phone = new Phone(_fakeMessageEngine);
        _notificationService = new NotificationService(_phone);
    }

    public void RecievedCallFromUser()
    {
        var message = new NewCallMessage("otherUserId");
        _fakeMessageEngine.OnMessageArrived += Raise.With(new MessageArrivedEventArgs(message)).Now;
    }

    public void CallRejected()
    {
        _phone.AnswerCall(Answer.Reject);
    }

    public void EndCallReportAdded()
    {
        var result = _notificationService.GetLastReport();

        Assert.AreEqual(ReportType.CallEnded, result.ReportType);
    }
}

[TestClass]
public class NotificationBddTests
{
    [TestMethod]
    public void RejectCall_UserRecieveCallAndRejectIt_SendReport()
    {
        var builder = new NotificationTestBuilder();
        new NotificationBddTests()
            .Given(s => builder.RecievedCallFromUser())
            .When(s => builder.CallRejected())
            .Then(s => builder.EndCallReportAdded())
            .BDDfy();
    }
}

And if I needed to do some “cleaning” after the test has finished running – I can implement IDisposable and put the whole test inside a using block.

And so by creating the state in each test we make sure that they could not effect each other.

Happy coding…