What I hate about BDD

8 comments

Monday, September 29, 2014

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<IMessageEngine>();
    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<IMessageEngine>();
        _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<IMessageEngine>();
        _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…

Visual Studio 2014 CTP - first impressions

No comments

Tuesday, September 09, 2014

I’ve installed the latest VS2014 CTP3 on my aging I3 home machine – because why use a VM when I can always re-install windows.
And so far I’m impressed! this is a low end I3/4GB machine and it loaded fast and works without any noticeable performance issues – I might be mistaken but it seems the performance is so much better then previous versions.
I’ve stated by creating a new project and when I’ve tried to rename Class1 to the less generic MyClass I was surprised to see a new dialog opening up – it’s no Resharper but looks like VS refactoring is heading in the right direction:
Rename
 image
Also – preview changes – just in case you want to check what exactly a “trivial” rename has in store for you:
Rename2
Just to make sure I’ve checked the “Refactor” menu (what do you mean – what refactor menu???) and it has the same options we has in VS2013 but I did find two new options available – Quick fixes (whatever that is) and Organize Usings which I miss whenever I’m forced to use “plain vanilla” VS.
image
It’s not much but I can only speculate that with Roslyn inside we’ll see a many more Refactorings options (and inspections) real fast real soon - both by Microsoft and the “Community”.
After tinkering with the IDE for a while I was ready to try some cool C# 6 features.
I’ve added a Primary Constructor since I wanted to try the new Read-Only Properties feature when VS politely informed me that the feature is only available in “experimental” language version.
experimental
By the way – notice how much information I got just by hovering over the code – and it got to me as fast as my cursor moved.
After a quick trip to Google and back I found that in order to use the new C# 6 features all I need to do is update the project file (.csproj).
And so I’ve opened the project file using XML editor and added the required magic:
image
And the error went away –  and I got to compile this:
image
How cool is this – I have a Primary Constructor (after the class) and I can use the values passed to initialize fields and properties – read-only  properties!

So far it seems that the old(er) VS2013 on my machine was not affected and it’s still working just like before the installation – but I haven't done extensive development on this machine (because it’s slow) and so I won’t install it on my main machine (yet).

But so far so good…
Related Posts Plugin for WordPress, Blogger...