-
Notifications
You must be signed in to change notification settings - Fork 57
Refactoring specs #881
Description
The good news is: this app has a lot of great tests!
The bad news is: it is hard to figure out if there is test coverage for a feature or not.
Part of the reason for this is that we are using various spec types in unusual ways.
Examples:
Controller specs
What they should check for: rendered templates
redirects
database changes
instance variables assigned in the controller to be shared with the view
What we are using them to check for: flash messages
database changes
emails sent
redirects
Request specs:
What they should check for: everything controller specs check for + response body
What we are using them to check for: same stuff we are using controller specs for
Feature specs:
What they should check for: Feature specs are high-level tests meant to exercise slices of functionality
through an application. They should drive the application only via its external
interface, usually web pages.
What we use them to check for: database changes
emails sent
redirects
content on page
As might be obvious, now that you read the above lists, we are using controller, request, and feature specs in almost the same way.
I vote for the following refactor:
Update feature specs from the perspective of a user clicking around the application and filling in forms. This means the spec expectations would also be for items on the page rather than looking at the database in an expectation (eg: "Proposal.find(1)`)
Consolidate controller and request specs. I would vote to move controller specs to be request specs because they test at the same level but have the added benefit of allowing to check for the response body.
This is just a suggestion and I am definitely open to others' ideas. I do think this refactor is warranted in some form, though, because right now it is very difficult to determine whether a feature is covered by specs due to all of the overlap.