Am I implementing BDD correctly?

Question

I'm writing a utility that validates fields. I decided to try my hand at Behaviour Driven Development (BDD). The validator utilises "rules" to determine if field is valid.

Three different types of rules exist:

1. RegexRule A field conforms to specific pattern(s)
2. RequireRule A value is present
3. EqualityRule A field's value equals another's value

A rule is tested against a value. If the value does not conform to the rule, a flag is set which marks the rule as erroneous. Calling getError() on an erroneous rule returns a string that details the error. Furthermore, the rule is reset (no longer erroneous).

Is this the correct manner of implementing BDD on this class? I'd greatly appreciate advice on how I could improve on this.

describe('RequireRule', function()  {
    var nameErr = 'The name field is required';

    beforeEach(function()  {
        this.r = new RequireRule('name', nameErr);
    });

    describe('test()', function()  {
        it('should return false when value provided is null or undefined', function()  {
            expect(this.r.test('asd')).to.equal(true);
        });
        it('should return true when value provided is not null or undefined', function()  {
            expect(this.r.test(null)).to.equal(false);
        });
    });

    describe('isErroneous()', function()  {
        it('should become erroneous when value tested is null or undefined', function()  {
            r.test(null);
            expect(this.r.isErroneous()).to.equal(true);
        });
        it('should not be erroneous when value tested is not null or undefined', function()  {
            r.test('asd');
            expect(this.r.isErroneous()).to.equal(false);
        });
    });

    describe('getError()', function()  {
        it('should return null when the test() succeeded', function()  {
            r.test('asd');
            expect(this.r.getError()).to.equal(null);
        });
        it('should return the error string when the test() failed, and set erroneous to false', function()  {
            r.test(null);
            expect(this.r.getError()).to.equal(nameErr);
            expect(this.r.isErroneous()).to.equal(false);
        });
    });
});

The test framework is Mocha.

Answer 1

This is roughly what BDD should look like. You're definitely on the right track in that you have a specific set of postconditions and expected results for every "public" method. However, the tests for these postconditions are incomplete in some respects that I think will probably come up as edge conditions and/or browser quirks (assuming your system is going to run in a browser). In addition, you're testing behaviors in this suite that are shared between your subclasses - even if they don't actually share a prototype, they do have a common interface and common expectations.

For incompleteness, I'm referring to 'should return false when value provided is null or undefined' when the corresponding test only tests null. This should be two tests, one for null and another for undefined. In addition, you should define and test the function's behavior for other values that are commonly mistaken for null and undefined, like 0, false, and ''. The empty string is of particular importance because it's unclear from your tests whether an empty string is acceptable as long as it's specified - the source of your data might determine whether that's applicable. I would go with two or three tests per native type: one that is acceptable, one that is not (if both are applicable), and one that is falsy (if the falsy value for the type is not already covered). If you really want to go all out, define behavior for the parameters '0', 'false', 'undefined', and NaN.

This sounds like a pain to do - especially when you see that you need to define said behavior for all of the other functions whose preconditions are the same. The root of the problem here is that your preconditions are not very DRY. Each function has two test cases: null and 'asd' as inputs to test. Since null and 'asd' are not really the only two inputs you can expect to get, your tests shouldn't be coupled to those two inputs. Further, when you go to write your tests for your other rule subclasses, you will be unable to re-use your isErroneous and getError tests because they have specifically named null as erroneous and 'asd' as acceptable.

I'm not a mocha expert, so I don't know how helpful or cooperative it will be, but you can refactor your suites to test states rather than classes, and it will be far DRYer. From what I can tell, the rule subclasses all have the following features:

• A (theoretically unbounded) set of inputs to the test method that will:
  • return false
  • leave the object in an erroneous state
• A (theoretically unbounded) set of inputs to the test method that will:
  • return true
  • leave the object in a validated state
• An initial state before test is ever called, whose behavior remains unspecified.

Each of your subclasses has a different set of erroneous and valid inputs, but you can easily define the behavior of those three states independently:

• A rule object in an erroneous state should:
  • return true from isErroneous()
  • return non-null from getError()
  • (become | not become) valid when a valid input is provided to test
• A rule object in a validated state should:
  • return false from isErroneous()
  • return null from getError()
  • (become | not become) erroneous when an invalid input is provided to test
• A rule object in its initial state should:
  • probably have defined behavior for getError and isErroneous, too.
  • become valid when a valid input is provided to test
  • become erroneous when an invalid input is provided to test

Now, your tests for all of your classes are reduced to the task of generating the input test cases and expected states they will generate. For the RequiredRule, that could be easily expressed in two arrays; you might want something different for your more complicated rules, since the criteria are presumably individualized per object for those subclasses. I might go with something like a generator.

The concept of a class is helpful for BDD when you have specific class invariants, but with javascript you are by no means limited to a one-to-one correspondence between a class (constructor/prototype) and its defined behaviors. Like anything else in software design, you can tell your unit test is wrong as soon as you type the same thing 2 or 3 times. Usually, you can fix the repetition by making the object of your suite (parameter to describe) something more applicable than the name of the constructor.

Answer 2

Let me say up front that I'm far more familiar with unit testing than behavior testing, but this looks an awful like you wrote the tests after the code. When I think of "behavior", I don't think of neatly mapping it to method names. Instead I would write use cases that may require multiple method calls.

Note: I don't recall seeing nested describe blocks before, but I assume whatever testing framework (Jasmine?) handles this neatly and that beforeEach is called with every it call.

The following rewrite tests only the error (null value provided) path to make it easier to follow. The main difference is that each test now checks a single aspect of the overall behavior. You still need to decide how to group the tests for each use case--either with separate top-level describe blocks or nesting as you did above.

describe('RequireRule', function()  {
    var nameErr = 'The name field is required';

    beforeEach(function()  {
        this.r = new RequireRule('name', nameErr);
    });

    it('should return false to denote an error', function() {
        expect(this.r.test(null)).to.equal(false);
    });

    it('should set erroneous to true to denote an error', function() {
        this.r.test(null);
        expect(this.r.isErroneous()).to.equal(true);
    });

    it('should return the message when erroneous', function()  {
        this.r.test(null);
        expect(this.r.getError()).to.equal(nameErr);
    });

    it('should clear the message after reading the message', function()  {
        this.r.test(null);
        this.r.getError();
        expect(this.r.getError()).to.equal(null);
    });

    it('should set erroneous to false after reading the message', function()  {
        this.r.test(null);
        this.r.getError();
        expect(this.r.isErroneous()).to.equal(false);
    });
});

I find that I naturally tend toward structuring my unit tests like this when I write them first using TDD. I imagine BDD would have the same effect. This results in tests that apply to the class as a whole rather than picking apart each method.