7
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ What assertion & test running framework are you using? I don't know if I need to know or not yet, but I would hate to be like "yeah, everything looks great!" and miss a really obvious error or quirk that spoils the whole thing while I assume everything just does what it looks like it does. \$\endgroup\$ – sqykly Jan 19 '14 at 6:59
  • \$\begingroup\$ The two test() expectations are reversed from their descriptions. \$\endgroup\$ – David Harkness Jan 19 '14 at 8:25
  • \$\begingroup\$ link Mocha \$\endgroup\$ – user34636 Jan 19 '14 at 8:36
5
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ I haven't written the tester class as yet, but does this correctly reflect your response? pastebin.com/ADB9meHV \$\endgroup\$ – user34636 Jan 24 '14 at 0:21
  • \$\begingroup\$ Perfect so far! \$\endgroup\$ – sqykly Jan 24 '14 at 2:22
2
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Say, for example, I had another Rule I wanted to test these BDD tests on (not just 'r'). How should I go about repeating these without repeating code? I think a loop would work, but it's probably not the most elegant solution. \$\endgroup\$ – user34636 Jan 19 '14 at 10:01
  • \$\begingroup\$ The problem with getting tricky in tests is that test code is untested. Each test needs to be obviously correct from a cursory visual inspection. That being said, I am not against some basic DRY techniques such as a function containing the above specs: function it_should_treat_null_as_error(rule) \$\endgroup\$ – David Harkness Jan 19 '14 at 19:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy