Learning to write DSL utilities for unit tests and am worried about extensibility

I'm trying to simplify our unit tests with hand written DSL's. So far I have DSL's that walk developers through processing a service after setting up all preconditions and the construction of an monster object that has a huge constructor and setters that conflict. The monster is one of the many preconditions of processing the service and they are constructed by hand over and over as developers write unit tests for new services in our system. So it would be worth putting considerable effort into making constructing them easier. I'm tired of watching people copy and paste the "life support system" from unit test to unit test.

I'm making separate new classes, one who's job it is to build the monster class and one that walks them through the steps to process the service. These will be used by many developers. This means once it's used widely changes will be difficult. That's why I'm worried about extendability.

Rather than a simple fluent interface builder (where every method returns a this and is always available) I've decided to use an internal DSL that returns a different nested class. This is a very powerful choice. It lets me limit the methods available at any one step to only the valid ones. It also lets me change what the return type of a method is based on the "state" previous choices put the DSL into.

Briefly, the way it works is the first method returns the first nested class that has the next valid method that could not have been called before. This limits the choices intellisense will offer making writing the DSL's calling code easier.

```
//Final build() method doesn't exist on PersonBuilder. It's on a nested class.
public class PersonBuilder {
public GotRequired doRequired() {
return new GotRequired();
}

//One of many nested classes
public class GotRequired {
public GotFirstName addFirstName(String firstName) {
mFirstName = firstName;
return new GotFirstName();
}
}

A few thoughts...

-
It's not at all obvious how the user gets started. new PersonBuilder().blahblahblah()?

A common alternative idiom is to use a public static method as the bootstrap point. PersonBuilder.newInstance(), PersonBuilder.create(), or even PersonBuilder.start()

-
I'm bothered by the presence of new in the middle of the test setup

public class GotRequired {
    public GotFirstName addFirstName(String firstName) {
        mFirstName = firstName;
        return new GotFirstName();
    }
}

addFirstName includes both logic and object construction, which in production logic would be a code-smell. It usually means one of two things; either that you should be injecting a factory to worry about the object construction, or that you should be creating the object earlier.

In this case, I think creating the object in advance is called for:

public class GotRequired {
    private final GotFirstName next;

    GotRequired(GotFirstName next) {
        this.next = next;
    }

    public GotFirstName addFirstName(String firstName) {
        mFirstName = firstName;
        return next;
    }
}

Presumably, you are doing that because you wanted to share the member objects in the PersonBuilder. Instead, make the record that everybody is editing explicit.

-
Your design has the state transitions scattered all over -- instead of being able to see the entire design in one place, you have to look at each class in order to discover what's going on.

If you build out the steps in advance, then the "create" method becomes the documentation for how the state machine works.

The straight forward approach would be to build your objects in reverse order

...
GotFirstName firstNameStep = new GotFirstName(previousStep);
GotRequired gotRequiredStep = new GotRequired(firstNameStep);
return new PersonBuilder(gotRequiredStep);

To get a more natural ordering, you could use pass through setters

public class GotRequired {
    ...
    GotFirstName setNext(GotFirstName next);
        this.next = next;
        return next;
    }
 }

-
The past tense names of the steps are really hard to read. The class names should be telling you what this class does, rather than telling you what some previous class did. GetMiddleName rather than GotFirstName, for instance. Get/Got are verbs though, which doesn't likely match your coding standards. MiddleNameStep? MiddleNameCollector?

-
One nasty side effect of the way you've written these classes is that you can't re-use them for a different step ordering. For instance, what if you had another kind of test where middle name should be irrelevant? In that case, GetFirstName should proceed immediately to GetLastName.

You might be able to patch this by using generics. Each class in turn is specialized on its next step:

public class GetFirstName {
    private final T next;

    GetFirstName(T next) {
        this.next = next;
    }

    public T addFirstName(String firstName) {
        mFirstName = firstName;
        return next;
    }
}

...
GetFirstName thisStep = new GetFirstName(thatStep);

-
I'm not convinced that this is the right approach -- simplifying writing tests by constraining the programmers to do the right thing isn't as likely to be successful as actually making the tests simpler. You should look at the source for the tests, and examine how many parameters the developers need to set that aren't actually part of the text. If the developers are specifying 7 or 8 parameters for a test that really only cares about one of them, then you are making extra work.

Hard to know for sure without an actual example of a test.

StackExchange Code Review Q#58842, answer score: 4