Puppet: System Administration Automated

Another Testing Conundrum: Mocks or Real Objects?


So, I have a very simple method for generating a certificate request:

# How to create a certificate request with our system defaults.
def generate(key)
    Puppet.info "Creating a new SSL certificate request for %s" % name

    csr = OpenSSL::X509::Request.new
    csr.version = 0
    csr.subject = OpenSSL::X509::Name.new([["CN", name]])
    csr.public_key = key.public_key
    csr.sign(key, OpenSSL::Digest::MD5.new)

    raise Puppet::Error, "CSR sign verification failed; you need to clean the certificate request for %s on the server" % name unless csr.verify(key.public_key)

    @content = csr
end

Short, readable, clear. Theoretically, this should be easy to test, too, and hopefully the tests should be about as short.

If you follow the strategy of using a mock request, your first pass ends up looking disappointingly like this:

describe "when generating" do
    before do
        @instance = @class.new("myname")

        key = Puppet::SSL::Key.new("myname")
        @key = key.generate

        @request = mock 'request'
        OpenSSL::X509::Request.expects(:new).returns(@request)

        @request.stubs(:version=)
        @request.stubs(:public_key=)
        @request.stubs(:subject=)
        @request.stubs(:sign)
        @request.stubs(:verify).returns(true)
    end

    it "should log that it is creating a new certificate request" do
        Puppet.expects(:info)
        @instance.generate(@key)
    end

    it "should set the subject to [CN, name]" do
        subject = mock 'subject'
        OpenSSL::X509::Name.expects(:new).with([["CN", @instance.name]]).returns(subject)
        @request.expects(:subject=).with(subject)
        @instance.generate(@key)
    end

    it "should set the version to 0" do
        @request.expects(:version=).with(0)
        @instance.generate(@key)
    end
    ...

Obviously, the test isn't done yet. Total line count for a 13 line method? 66 lines. (Admittedly, I'm a bit generous on white space.)

The thing I dislike most about this test is that the setup code has to know exactly how many tests there are -- if I add a new setting to the request, then I need to change my setup code, which has bitten me multiple times and is a real pain. One option for removing that is to skip the mock:

describe "when generating" do
    before do
        @instance = @class.new("myname")

        key = Puppet::SSL::Key.new("myname")
        @key = key.generate

        @request = OpenSSL::X509::Request.new
        OpenSSL::X509::Request.expects(:new).returns(@request)

        @request.stubs(:verify).returns(true)
    end
    ...

I still need to stub the verify method, because of the check that I do on the result, but skipping the use of mocks makes it simpler.

Note that I've been using a real SSL key the whole time -- this slows things down a little, since the ssl keys aren't free to make, but it simplifies my setup code in the same way that the above setup is simpler than the top one.

Of course, a third option is to keep the mock but use stub_everything. This is apparently another indication of code smell, and it seems the difference between this and just using a real object is that I need to write a separate integration test if I use a mock, but with this, I don't really.

I think I'm going to keep this last setup method. It's still 65 lines of code for a 13 line method, but at least it lists each behaviour separately. My previous test included this behaviour:

# It just doesn't make sense to work so hard around mocking all of this crap five times in order to get this test down to one expectation
# per test.
it "should create a new certificate request with the subject set to [CN, name], the version set to 0, the public key set to the privided key's public key, and signed by the provided key" do
    @request = mock 'request'
    OpenSSL::X509::Request.expects(:new).returns(@request)

    subject = mock 'subject'
    OpenSSL::X509::Name.expects(:new).with([["CN", @instance.name]]).returns(subject)
    @request.expects(:version=).with 0

    # For some reason, this is failing, even though the values are correct.
    # It seems to be considering the values different if i use 'with'.
    @request.expects(:public_key=)
    @request.expects(:subject=).with subject

    # Again, this is weirdly failing, even though it's painfully simple.
    @request.expects(:sign)

    @request.stubs(:verify).returns(true)

    @instance.generate(@key).should == @request
end

I'm not fond of this, because it doesn't list each behaviour separately, but it seemed to be appropriate for such a simple method.

Unfortunately, the 'verify' method (assuming I didn't use a mock) was nearly as long as this method, since it has to stub everything.

I'm thoroughly convinced that this generate method is reasonable, but the rules of the game are that it's got code smell because it's difficult to test, which I don't buy at all. So now I've got 5:1 tests to code for a really simple and maintainable method, meaning I'm basically doomed to always spend more time on the tests than the functionality, which is utter crap.

add to del.icio.us Add to Blinkslist add to furl Digg it add to ma.gnolia Stumble It! add to simpy seed the vine TailRank post to facebook

Wed, 12 Mar 2008 | Tags: , , ,