nulldb/nulldb — Issue #19: [philosophical] is the 'Employee.connection.should have_executed(:insert)' testing pattern a good idea?
Author: willsu
Created: 2012-02-14 20:15:07+00:00
State: open

This is really not a specific issue with your project, but I wanted to add a few thoughts:

I think that your idea for using a null object pattern to stub the DB layer is good. I agree that unit tests should have clearly isolate their concerns, and that interacting with external services (like a DB) is good example of introducing external dependencies that add lag and 'integration' type behavior to your alleged unit tests. Of course, I still believe that integration tests are mandatory when your unit tests are properly isolated, but I digress.

You say that:

'unit tests should be exercising only the business logic contained in your models, not ActiveRecord'.

But you provide the following example of how to test the SQL queries executed by ActiveRecord on your null adapter:

'Employee.connection.should have_executed(:insert)'

IMHO, this is actually worse then testing a round trip to the DB. Testing the SQL generated by ActiveRecord is a spec that, I believe, belongs in the ActiveRecord library itself. And furthermore, looking for the keyword 'insert' really doesn't tell us much about the query we're executing (to what table is it inserting?, what values?, etc).

Personally, I would use your adapter to stub the interaction with the DB, but not to test SQL statements at the adapter level. I'd still test interactions with ActiveRecord via RSpec::Mock(s) and Stub(s). I think that your project presents an excellent way to achieve this. Even if I was trying to do a 'persistence layer' test, I couldn't pull an object back out of the DB since the creation of that object is stubbed by your adapter.

Do you think it's better to test the SQL then to do a round trip test on your object? I feel that the following statement has less dependencies on ActiveRecord then testing the SQL at the adapter layer:

it "creates a user" do
Foo.some_method_that_creates_a_user('username')
User.find_by_username('username').should be_present
end

This way, it really doesn't matter what SQL was executed as long as the user was created (yeah, I have a dependency on ActiveRecord in my testcase but I think you get the idea). I believe that it is more maintainable and easier to read then asserting SQL. You can argue that this is more of an integration level test, and that it would add lag to your alleged 'unit' tests but at least it still doesn't care about 'how' the record was created. I realize that the purpose of your project is to get people away from that type of testing, so I think I already know how you would respond to this ;).

Anyway, I really enjoy talking about this stuff so I'm just interested in hearing your opinion. I'm really just trying to be pragmatic here.. not trying to defend 'religion' of any sort. Thanks for reading this.

-- Will