How to combine the benefits of unit and feature testing and not fall into coverage trap
How many methods of testing an app have you seen? Do you have a friend who only believes in the legitimacy of acceptance tests? Or are you loyal fans of TDD and striving for 100% coverage? What is worth testing and when might it be better to let go? How to test an app so that the tests don’t absorb us and really prove useful?
The example
Let's use a simple Rails app with a relational database as an example. Imagine that we have two models in our system: the user and the team. Our PO has created a ticket titled, "inviting users to teams."
We know that we have to create some sort of associative table and wonder what's the easiest way to handle inviting. Let's perhaps add a "status" field as an enum: "pending," "accepted," "rejected" to the aforementioned table and we're done! This is obviously not the perfect solution, but it will suffice for the moment.
Unit/integration tests
What we often do in such a situation is to put the entire code into the controller (simple AR callbacks create/update/destroy) and test the controllers with the following tests of requests:
it 'sends an invitation request' do
post '/api/team_members', { user_id: user.id, team_id: team.id }
expect(TeamMember.last.user_id).to eq(user.id)
expect(TeamMember.last.team_id).to eq(team.id)
expect(TeamMember.last.status).to eq('pending')
end
it 'sends an accept invitation request' do
team_member = TeamMember.create! (..), status: 'pending'
put "/api/team_members/#{team_member.id}/accept"
expect(team_member.reload.status).to eq('accepted')
end
After some time, when looking at the tickets, we realize that our additional “status” field was not the best choice. It turns out that, e.g., you can now invite using emails and a token is generated (of course, we do not want to keep all of this information in the associative table).
What follows is an internal change of the implementation which doesn’t have any impact on the functionality. Some refactoring could be useful, perhaps creating some service or a new model.
The question that arises is: how many tests we’ll have to write from scratch and why so many! Despite the lack of change in the functionality and only the way it’s implemented, the tests will fail. The question is: should they?
Feature tests (end to end)
Perhaps BDD would be a better solution? If we tested it “in the browser” we could go ahead and completely change the implementation while the tests would remain the same. This is undoubtedly an advantage — the representation of the production environment is also much greater. Often, we conduct thousands of unit tests and in the end, are faced with a 500 error which would be caught by a feature test.
The advantages, unfortunately, end here. Each of us probably knows that pain when, e.g., Capybara begins to randomly fail or goes on running for tens of minutes.
Let’s create a summary of the benefits of each solution and then try to… combine them!
Unit tets
- Real TDD and Red-Green-Refactor cycle
- Good influence on architecture
- Fast and stable
Feature tests
- Better reflection of production env
- They test cooperation between different classes/modules
- They test the functionality, not the method of implementation
Abstraction level
Let’s think for a moment about the last point and take a look at the following test of a calculator:
it 'adds two numbers' do
expect(calculator.add(1, 2)).to eq(3)
expect(calculator.add(5, 5)).to eq(10)
end
If we find a better implementation of adding on Stack Overflow, we’ll throw out our own and replace it with the new one and all of the tests will still work. This is the situation that we expect. However, if we remove the “status” field in our example and replace it with another solution, we’ll have to write all the tests from scratch.
Our example calculator operates on one abstraction level. It takes the numbers in, returns a number, that’s it — it doesn’t penetrate anything more!
In our tests, we mix everything. On one hand, we have a use case — “accept an invitation with the specified ID” presented as a request — on the other, implementation details such as specific tables in the database or features typical of Rails, such as Active Record.
This makes it so that if we try to replace the AR, the table, the database, our tests will turn red.
Interactors
https://www.youtube.com/watch?v=WpkDN78P884
I first heard about interactors in one of the first episodes of CleanCode. An interactor is a “special service/class” that maps users' actions to code by exposing high-level API (use cases).
In our case, it would have the “invite” and “accept” methods that would contain the whole logic of these actions without any knowledge about delivery mechanisms (rails/desktop/console).
But wait, our whole logic is based around calling one ActiveRecord method. Wouldn’t it be overkill to create a special service for this purpose?
In a sense, it would, but if we throw all logic controllers (except for permit params, current_user etc) to plain old Ruby objects, then what’s the point of testing controllers in the first place?
Of course, we can test the returned JSON (does it agree with the documentation) but the logic of our app is separated and we can focus on its proper testing. Furthermore, in many cases, this one line turns in the future into a dozen or so lines.
it 'creates a invitation' do
invitation = TeamInteractor.new(user).invite user_id: second_user.id, team_id: team.id
expect(invitation.user).to eq(second_user)
expect(invitation.team).to eq(team)
end
Seemingly, not a lot has changed but our only intention is for the returned object to contain the user and the team (to, e.g., then pass it to serialization in the case of the API). We aren’t testing if the object was actually saved because it doesn’t really interest us. Whether it will be stored in a DB, a text file, or the memory does not interest us in terms of the test.
it 'accepts an invitation' do
result = TeamInteractor.new(second_user).accept(invitation.id)
expect(result).to eq(true)
end
We are interested in the fact that such a creation process returns an object with an ID. We can pass it to the “accept” action that won’t end with the error “cannot find invitation with id=4” but will return true or false (or a model with errors, error text, error code, etc.).
Seeds
The approach to seeding our app is also changing. When we are testing the execution of an action on a given resource, we have to create it beforehand. We usually do this through factory or manually. This causes our tests to be worse and worse at reflecting production. On the server, our client performs an “invite” action on a self-created user object and not on the object from the factory.
Having thrown the whole logic into the interactors, we don’t need any seeds. If we need to test the “accept” action, we need a team, a user and a valid invitation:
user = UserInteractor.new.register email: 'test@example.com', password: 'password'
team = TeamInteractor.new(user).create name: 'Clean Code Team!'
second_user = UserInteractor.new.register email: 'test2@example.com', password: 'password'
invitation = InvitationInteractor.new(user).invite_to_team user_id: second_user.id, team_id: team.id
expect(InvitationInteractor.new(second_user).accept(invitation_id: invitation.id).to eq(true)
These are the exact steps of the client on a production server required to perform such an action:
- Registering an account (and logging in)
- Creating a team
- Registering a second account for the purpose of the test
- Sending an invitation
- Checking if accepting invitations works on the second account
When we have whole logic in interactors, we are able to write high level tests (like Capybara) but still we are testing just pure Ruby classes.
The code example
https://twitter.com/borgiarts/status/593321016899670016
We will try to create our ticket to invite users to the team again, and, despite testing from a “high level perspective,” we’ll try to stick to the three principles of unit testing:
- You must write a failing test before you write any production code.
- You must not write more of a test than is sufficient to fail, or fail to compile.
- You must not write more production code than is sufficient to make the currently failing test pass.
Let’s start with the test then:
it 'creates invitation' do
interactor = described_class.new user: current_user
invitation = interactor.invite team_id: example_team.id, user_id: second_user.id
expect(invitation.user).to eq(second_user)
expect(invitation.team).to eq (example_team)
end
And the simplest implementation:
def invite(team_id:, user_id:)
TeamMember.new team_id: team_id, user_id: user_id
end
Note that we aren’t even saving this model anywhere. It obviously is a non-production implementation but it is sufficient to satisfy the test.
The code above maps to the user’s action “send the invitation.” Now, let’s add the code responsible for “display my invitations.”
it ‘returns invitations’ do
interactor = described_class.new user: second_user
invitations = interactor.invitations
expect(invitations.count).to eq(1)
expect(invitations.first.team).to eq(example_team)
end
The implementation could look like this:
def invite(team_id:, user_id:)
TeamMember.create team_id: team_id, user_id: user_id
end
def invitations
@user.team_members
end
Let’s focus on accepting the invitation now:
it ‘accepts an invitation’ do
interactor = described_class.new user: second_user
invitation = interactor.invitations.first
expect(interactor.accept invitation_id: invitation.id).to eq(true)
end
And the implementation:
def accept(invitation_id:)
true
end
The invitation should be removed from the list after it is accepted!
it ‘removes an invitation from list after accept’ do
interactor = described_class.new user: second_user
invitation = interactor.invitations.first
interactor.accept invitation_id: invitation.id
expect(interactor.invitations.count).to eq(0)
end
A change of the implementation:
def invite(team_id:, user_id:)
TeamMember.create team_id: team_id, user_id: user_id, status: :pending
end
def invitations
@user.team_members.where(status: :pending)
end
def accept(invitation_id:)
TeamMember.find_by(id: invitation_id).update status: :accepted
end
I think this is where we’ll stop. What’s cool about this approach is that we feel just as nicely as when writing “normal” TDD tests. We work in small steps, in the red-green-refactor cycle, but if we want to connect to a great service written by a colleague from the team, if we move invitations to another server or to a ‘third party provider’, the tests will remain unchanged because we are only focusing on whether a given functionality works in a particular situation and not on how it works.
Gist with the example above is available at:
https://gist.github.com/mjaneczek/adc1bf4ecfc5f3c3a67a5fb855030c41
Thanks!
Keep in mind that the approach presented here is only a loose idea supported by one small project on which I’m currently working. For a long time, I’ve been trying to put logic onto plain Ruby services.
I just started experimenting with not using seeds or simply not reviewing an entry in the database after carrying out an action.
What do you think? Any ideas or improvements? Does it even make any sense to you? Thanks so much for your time, I invite you to comment and experiment!
