A recent conversation about MSpec practices

Recently I’ve heard about more and more people checking out MSpec. A few days ago I got an email from a friend. He said he was having trouble with base class explosion while creating specs. Here is a snippet from his code:

public abstract class with_null_program
{
  protected static Program program;
}

public abstract class with_program_and_empty_args : with_null_program
{
  Establish context = () => program = new Program(new string[]{});
}

public abstract class with_list_command
{

}

[Subject("Program")]
public class when_creating : with_program_and_empty_args
{
  private Because of = () => program = new Program(new string[] {});

  It should_output_to_the_console =()=> 
   program.Out.ShouldEqual(Console.Out);
}

[Subject("Program")]
public class when_running_with_no_arguments : with_null_program
{
  private static StringBuilder outputBuilder;

  private Because of = () =>
  {
    outputBuilder = new StringBuilder();
    program = new Program(new string[] {}) {Out = new StringWriter(outputBuilder)};
    program.Run();
  };

  It should_print_usage =()=> outputBuilder.ToString().ShouldContain("USAGE");
}

[Subject("Program")]
public class when_running_without_database_args : with_list_command
{

}

And here’s my response:

Hey man, Glad to see you playing with MSpec. Definitely curious to get your feedback on the experience.

So there’s a few things I noticed about the specs. For one, you’re using the with_ pattern. I know I or Scott or someone started this… but I don’t like it now. I much more prefer to just have a base class that contains any utility/meaningless cruft my specs have. You also seem to have taken this to a bit of an extreme. Would you make a regular base class just to create a single instance variable and set it to null in the constructor? Probably not. Same stuff applies here. There’s no value in creating base classes unless they provide value. There’s no naming or understanding benefit. As a matter of fact, they *hinder* understanding more than anything. This is why I try to only put utter crap in base classes, and when it’s important crap, I put it in a method and name it something descriptive and call it in the context. If I were to rewrite these specs I *may* have a single base class called ProgramSpecs. It would probably just have the static program field, but be a place holder in case i needed any utility methods. Naming specs with the with_foo_bar implies that that name is important, which it isn’t. It’s not included in the report for a reason. Your context description should be fully encapsulated by the name of the context class.

Another thing I’m seeing… and I’m not sure if this is just circumstantial, but do you know that you can have more than one context in a class hierarchy? In other words, something like outputBuilder = new StringBuilder() would go in the subclasses establish context. They’re run in order from basiest to subbiest. The Because is *only* a way to highlight a line and say "this is the catalyst that makes the observations possible… everything else up to this point is also important, but this is the real meat". Because is actually *part* of the context. The Context is the arrange and the act (if there is an act). Here’s an example of multiple context clauses (though i have no idea why i didn’t use because here, heh): http://github.com/aaronjensen/compete/blob/master/Source/Compete.Specs/Model/Game/AggregateResultSpecs.cs

As an aside, I feel the Program class is a bit awkward. Personally, it’d make more sense if the StringBuilder was a constructor parameter and the args were parameters to the run method. That’s just my style though–i’m not saying it’s better. It would definitely make testing this less awkward. Actually, look at your context. "when running with no arguments". Your code isn’t doing what you said it would do, at least not in the simplest fashion. I think I’ll change my position to "strongly prefer" the arguments to be passed to the Run method :) More often than not, when you’re feeling pain while testing it’s because your API can be improved.

You may, if you haven’t already, want to take a look at the mspec console runner specs: http://github.com/machine/machine.specifications/blob/master/Source/Machine.Specifications.ConsoleRunner.Specs/ProgramSpecs.cs

They’re quite similar to the domain you were writing specs for and may help. Yes, I used with_runner there, they’re old ;)

Hope that helps a bit, let me know if you have any other questions or anything.

Thanks, Aaron

This entry was posted in mspec. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://codebetter.com/members/aaronjensen/default.aspx aaronjensen

    Luke,

    The original post is explicitly about why I don’t like the with_ pattern. Your context should convey what you are actually testing. The fact that you named your with class in the example “with_some_context” implies that the with_ pattern itself has prevented some from understanding that the context is actually the name of the class. Not its base class. The context is the “Given” and the “When” if you want to compare it to GWT.

  • Luke

    Why don’t you like the with_ pattern anymore? :(
    Although it is something small, and it doesn’t make a big difference if we don’t use it, I think it looks nice to have the spec’s “header” (class name and parent class) in English form, and helps to understand what is it that you’re actually testing.

    public class when_doing_something : with_some_context
    {

    }

  • http://codebetter.com/members/aaronjensen/default.aspx aaronjensen

    This doesn’t strike me as the best way to test this. Take a step back and look at it on a higher level. The *ordering* is irrelevant. The observable behavior is relevant.

    In your example, I’m assuming you’re wanting to spec something like this:

    When an order is placed, it should be recorded

    Also

    When an order is placed for a second time, it should not be recorded

    The way to test this is to simple and does not require you to assert on code ordering. In the first spec, you would stub the GetOrder method to return null. You would then assert that the InsertOrder method was invoked with an order matching the order you care about.

    In the second spec, you would stub the GetOrder method to return an order. You would then assert that the InsertOrder method was *not* called.

    No mention of ordering here. The ordering is implicit. You define your ins and observe/assert on your outs.

    It is rare that you actually need to assert on ordering, more often than not I’d say if you take a step back you can find an alternate solution that is more elegant.

  • Kevin

    “Kevin, correct me if I’m wrong, but it doesn’t sound like you want to verify that *specs* are run in a certain order, but rather that your system under test does things in a certain order. Is that correct?”

    Yes, that’s correct. I said it the wrong way. I want to make sure that methods of my underlying system are executed in a certain order, and not the specs themselves.
    The brief example I posted is just the first thing that came to my mind, so it may be misleading.

    A simplistic but (hopefully) better example would be:

    Say I am testing the OrderService.InsertOrder() method which in turn uses a mocked “OrderDAO”. And I want to ensure that before calling the OrderDAO.InsertOrder(), the service invokes the OrderDAO.GetOrder() to verify that the order doesn’t exist in the DB already.
    So I want to set my expectations in a way that the OrderDAO.Get() is executed BEFORE the OrderDAO.Insert() and not the other way round.

    Is it more clear now? Am I missing the point here?

  • http://codebetter.com/members/aaronjensen/default.aspx aaronjensen

    Luke,

    I’m not a fan of cucumber. I think the regular expression based scenario expression leads to short stopping the analysis. It is also easily abused.

    Example:

    Given username “bob”
    and password “foo”
    when logging in
    then it should fail

    Given username “sam”
    and password “foo”
    when logging in
    then it should pass

    So great, I can reuse my code to set up my parameters and call some methods. But uh oh, what happens when someone reads these? Why should the first fail? Is the password wrong? Does the username not exist? Should we rewrite it to setup the user?

    Given user named “bob” with password “asdf”
    when logging in with username “bob” and password “foo”
    then it should fail

    Sure, we could do that, but now we’re getting pretty verbose.

    Compare that to the context specification alternative…

    When logging in with a user that exists with an incorrect password, it should fail

    or even

    When logging in with an incorrect password, it should fail

    I’ve done just a little bit more analysis and figured out what the scenario is actually trying to say. I’ve used English to describe it rather than allowing the reader to try and infer what I’m actually trying to test. I’ve potentially discovered terms in my ubiquitous language that I may not have had I stuck to plug and play scenario bits.

    Cucumber and friends are quite popular at the moment though, so don’t necessarily take my word for it. Feel free to experiment and draw your own conclusions.

  • http://codebetter.com/members/aaronjensen/default.aspx aaronjensen

    Kevin, correct me if I’m wrong, but it doesn’t sound like you want to verify that *specs* are run in a certain order, but rather that your system under test does things in a certain order. Is that correct?

    If so, this is an annoying problem to solve with any testing framework and it could be an indication that you’re testing the wrong thing. Why is it that you want X to happen before Y? The example you gave seems contrived. Is that your real example? If so, why do you want to test this? Isn’t there something more meaningful you can test?

    Essentially, you’re attempting to test the encapsulated portions of your system under test when really you should be testing the *behavior* that is observable externally.

    Any way, if you really want to do this, you’d just use custom fakes or something along those lines. The testing framework has nothing to do with this problem… unless i’m misunderstanding your question… it’s late.

  • Kevin

    I want to verify that certain specs are executed in a specific order (for instance, I want to make sure that a validation is run first and that the repository is accessed after that.. or whatever)

    Can Mspec ensure that?

  • Luke

    I think you mentioned Cucumber in some previous answer. I wasn’t able to use it, but have seen some screencast about Cuke4Nuke (which is the framework ported to .NET I believe) with Watin.

    http://tinyurl.com/yfkt3yc

    What do you think about it? What do you think about the regular expression way of expressing different scenarios? I am not so sure how well it works in bigger, real-life cases.

  • http://codebetter.com/members/aaronjensen/default.aspx aaronjensen

    Duplication isn’t something you should strive to avoid. I tend to test my data access against the db itself then stub it out when testing the actual functionality.

    I also tend to not test Controllers as much since mine are so simple. I don’t necessarily recommend that, I just haven’t gotten much value in it.

    We also write a lot of code that has very clear testing boundaries… pieces that receive messages and as a result send messages (or fire domain events). Really easy to know where to test these–you just test everything and stub out the db if you have one.

  • Martin

    Ok, I see what you mean Aaron and thanks for the quick reply.

    Another question I have:

    When you plan your tests (or specs) at a higher-level (ie: when you plan to test the user stories, the business requirements) should it be at the controller level (in an MVC arch), at the service layer, both?

    Definitely, it doesn’t seem the right thing to test the stories at the data access layer for example since you sometimes don’t talk in terms of “business” at that point. What do you think?

    If you DO test at data access layer, you will definitely also have to test those stories at the service and/or controller layer too, to ensure that the specification is fulfilled across all the layers of your app, and you might end with duplicate specs.

  • http://codebetter.com/members/aaronjensen/default.aspx aaronjensen

    Hi Martin,

    Just because they’re low level requirement doesn’t mean they cannot be expressed in English. Context/Spec certainly isn’t a one-size-fits-all testing paradigm, but it fits pretty well for most things.

    With lower level things you speak in lower level terms. You want to be sure to consider your audience: http://codebetter.com/blogs/aaron.jensen/archive/2008/10/19/bdd-consider-your-audience.aspx

    In this case your audience would be other programmers. Being able to express in plain, terse English what your code is doing is valuable both in terms of initially writing/designing it and later attempting to understand something that already exists.

  • Martin

    How does MSpec (or BDD itself) adapt to low level requirements, such as testing business classes methods in isolation, verifying a DB connection is closed, etc.?

    You may want to look at this article which talks about this and made me think about it:
    http://thedotnetters.blogspot.com/2010/01/introducing-mspec.html

  • http://codebetter.com/members/aaronjensen/default.aspx aaronjensen

    Hi Ryzam,

    You actually don’t. That’s not how the framework works. “Establish context” is just a marker. The actual text description of the context is the class name. MSpec is not Cucumber or any other GWT framework, it’s a Context/Specification framework. As such, you have one context. Some C/S frameworks support nested contexts, but MSpec does not at this time. You’ll need to say everything you need to say in the class name. Hope this helps.

  • ryzam

    How can i write establish context to the reporting?

    I like to do somthing like this

    Establish given_customer_status_is_non_active = () =>

    Establish and_given_balance_is_equal_to_1000 = () =>

    I try to write establish context to reporting, but not sure how

  • http://www.corebvba.be Tom Janssens

    Hey Aaron,

    Based on your version I am currently implementing a new BDD framework, but with an even cleaner layout then yours.

    Please let me know what you think about it, since your framework has been my initial inspiration :
    http://www.corebvba.be/blog/post/A-new-BDD-framework-in-Net-NetSpec.aspx

  • http://codebetter.com/members/aaronjensen/default.aspx aaronjensen

    Paco,

    I don’t see any positives with this approach other than saving typing. The negatives are numerous though, it makes scanning more difficult, you have to skip around the file in order to understand a test. It also can (and likely would) lead people to being lazy about their subjects and not thinking about them thoroughly. Most would just set their subject once on the base class and not think about it again.

    Scott,

    Yes, I agree. Some were starting to use or assume that the with_ classes were part of the context and therefore should be in the report or were allowed to be necessary for understanding. This is what I want to discourage. I, of course, have no qualms with this naming convention if used responsibly.

    BJ,

    Totally agree. Alex is a smart guy :)

  • BJ Dweck

    I’ve been using mSpec for only a few weeks now and struggled with this point a little, until I read the following advise from Alexander Groß:

    “…I mainly use “With” base classes as a container and perform only general initialization there, like setting up mocked dependencies. However, strive to not stub behavior in the “Withs”.”

    http://stackoverflow.com/questions/1184297/bdd-and-mspec-am-i-approaching-this-right

    Now my base classes play the role of a container and look like this:

    public class With_controller_container
    {
    protected static ISeasonsService _seasonsService;
    protected static IPrintService _printService;
    protected static IDesignService _designService;
    protected static IProductCategoryService _productCategoryService;

    protected static PrintController _printController;
    protected static AjaxController _ajaxController;
    protected static DesignController _designController;
    protected static AdminController _adminController;

    Establish context = () =>
    {
    _seasonsService = MockRepository.GenerateStub();
    _printService = MockRepository.GenerateStub();
    _designService = MockRepository.GenerateStub();
    _productCategoryService = MockRepository.GenerateStub();

    _printController = new PrintController(_printService);
    _ajaxController = new AjaxController(_printService, _seasonsService);
    _designController = new DesignController(_designService);
    _adminController = new AdminController(_productCategoryService, _seasonsService);
    };
    }

    and things are really working out well so far…

  • http://codebetter.com/members/ScottBellware/default.aspx ScottBellware

    The “with_” stuff is just a naming style. It’s aesthetic. It’s not the impetus for creating base context classes. As Aaron said, base context classes are there to remove noise. In C#, noise is inevitable – it’s just a noisy programming language, filled with compiler instructions and ceremony. When I create a base context, I may name it using the “with_” convention, or some other convention, but the drive to create a base context has nothing to do with trying to create English language constructs using C#’s syntax for class extension.

  • Paco

    Why don’t you declare the Subject attribute on the with_runner class in your example? Because it’s not a feature of MSpec of course. I would like to have subjects and sub-subjects with inheritance from baseclasses that appear in the report output.

    [Subject(“base subject”) public class Baseclass { }

    [Subject(“sub subject”)public class When_something : Baseclass { }

    Report:

    Base subject specs:
    - sub subject specs:
    + When something
    * should_do_this
    * should_do_that
    + When something else
    * should_do_that