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…

8 thoughts on “What I hate about BDD”

  1. In your topic about : “It’s all about shared state”

    I think there's a flaw in your example. Your ShoppingCart() ->

    private readonly ShoppingCart _cart = new ShoppingCart()

    Should be instantiated before EACH TEST to make sure it's a valid, out of the box, system under test.

  2. You're right – in fact that's what I wrote under the code sample 🙂
    I didn't ant to put a complex example and was just making a point. I saw countless tests that started that “simple” but soon became very complected – with more such fields added, and different tests needs different initialization. I try to avoid the problem of shared state between tests – and this is way I prefer the trick I wrote about in the post.

  3. It might be worth mentioning that other BDD framework, recreate your classes on each new scenario.

    So I feel you may took it a little far with your title.

  4. Your mindset is not the right one for BDD.

    BDD is not about unit testing. It's about functional testing or feature testing – essentially, replacing at least some of the work manual testers do with automated tests.

    Such tests are expensive, in terms of execution time. What adds to their cost is that some require tedious and complex setup.

    Which is why manual testers never think of their tests in isolation. Similarly to unit tests using a test fixture which sets up the environment properly for the tests to be executed, manual testers think of setting up their environment in a way which makes it possible to execute several tests, not just one, with that setup, in order to reduce the manual work required.

    In order to use BDD, you have to think along the same lines. Set up your environment, implement the steps for the story, then add assertions for all acceptance criteria. Then run the same test with several sets of input, rather than adding several test methods to a single test class. Extract the setup common to more tests into base classes, and try to model as much as possible of the differences between tests via the different input sets, rather than via different test methods. Your tests will no longer depend on order of execution, be less brittle and more compact.

  5. Actually BDDfy has the same API but regardless of API I see shared state even if the scenarios are one per file – to enable code reuse but on the way you lose in the form of shared state. And since the most unit testing frameworks that runs the BDD tests do not create a new process for each class you end op with scenarios (and even stories) effecting each other.

  6. You raise some good points but I fail to see how manual testing is related – first manual testers do think about Isolation, they take great pain to make sure that each scenario is run individually and do not effect another test run, otherwise there will be quite a lot of “cannot reproduce” bug rejects in their future.
    Furthermore regardless of state of mind (and I agree with you, I'm still learning to think “the BDD way”) why not avoid issues that we know exist just because it's BDD.

    Reading your comment once again I have to say I totally agree with the lats part – when possible I'd like to reduce the number of tests by using different inputs but even then I'd like to make sure that I do not carry a state between tests and I don't want to relay on the scenario writer to remember what to “initialize” before or “clean” after each run.

  7. When I said that manual testers don't think about their tests in isolation, I meant that they think about how to create the setup part so that they can fit s many tests as possible onto the same test fixture.

    That's what we programmers do too. It also makes it easier for them to describe the steps for reproduction of bugs. I didn't at all mean coupled tests – this would just make description of tests lengthier.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s