Improve documentation and awareness about feature specs (good and bad practices)
specs/feature/
)
We need to clarify the goal of feature specs (- they shouldn't test every possible cases
- they should test a happy path, and a less happy path but that's it
- every other possible paths should be tested with
- unit (finders, helpers, migrations, policies, presenters, routing, serializers, services, tasks, uploaders, views, workers) tests
- controllers tests
- requests tests
- JavaScript (Karma) tests
If we're confident that the low-level components work well (and we should be if we have enough unit tests), we shouldn't need to duplicate their thorough testing at the feature spec level.
It's very easy to add specs, but a lot harder to remove or improve specs, so one should take care of not introducing too many (slow and duplicated) specs.
Why these rules?
- They are slow to run since they spin up the entire application stack in a headless browser, and even slower when they integrate a JS driver
- With feature specs, the specs are run in different thread than the application. This means it does not share a connection to the database and your test will have to commit the transactions in order for the running application to see the data (and vice-versa). In that case we need to truncate the database after each spec instead of simply rolling back a transaction (the faster strategy that's in use for other kind of specs). This is slower than transactions, however, so we want to use truncation only when necessary.
A good example
- it doesn't create a lot of DB records
- it tests a happy path
- it tests a less happy path
- it tests what's on the page
A bad example
- it creates a lot of records (32 + associated records per example)
- it includes 77 examples
- it performs 421 HTTP queries
- it performs 46K DB queries
- it takes around 6 minutes to run
- the filtering functionality seems to be well-tested at the JavaScript level: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/spec/javascripts/filtered_search
Stats can be found at https://redash.gitlab.com/dashboard/test-suite-statistics.
Note: I'm not blaming the author, the intention of testing thoroughly is good!
What can we do about it?
-
Document how to use feature specs
- Good practices
- Bad practices
- Tips
-
Ensure new feature specs are reviewed by a backend reviewer/maintainer