Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge rules and recommendations from betterspecs.org #55

Merged
merged 18 commits into from
Nov 7, 2018
Merged
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -711,16 +711,33 @@ meant to be able to change with it.
easier understanding and reading of a test.

* <a name="factories"></a>
Use [Factory Bot](https://github.com/thoughtbot/factory_bot) to create test
objects in integration tests. You should very rarely have to use
`ModelName.create` within an integration spec. Do **not** use fixtures as they
are not nearly as maintainable as factories.
Use [Factory Bot](https://github.com/thoughtbot/factory_bot) to
create test data in integration tests. You should very rarely
have to use `ModelName.create` within an integration spec. Do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the ModelName.create example is specific to Rails. Should we consider moving this section down to Rails - models section, or perhaps just add a note in the text?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wording implies Rails, but the advice itself is quite generic. Fixtures here are some objects kept in an storage, not even necessarily database. Also:

Although factory_bot is written to work with ActiveRecord out of the box, it can also work with any Ruby class.

I'd rather keep the rule in this section, but changed ModelName.create into something that applies both to ActiveRecord and, say, file system.

**not** use fixtures as they are not nearly as maintainable as
factories.
<sup>[[link](#factories)]</sup>

```ruby
subject(:article) { FactoryBot.create(:some_article) }
# bad
subject(:article) do
Article.create(
title: 'Piccolina',
author: 'John Archer',
published_at: '17 August 2172',
approved: true
)
end

# good
subject(:article) { FactoryBot.create(:article) }
```

*NOTE*: When talking about unit tests the best practice would be to
use neither fixtures nor factories. Put as much of your domain logic
in libraries that can be tested without needing complex, time
consuming setup with either factories or fixtures.
pirj marked this conversation as resolved.
Show resolved Hide resolved

* <a name="needed-data"></a>
Do not load more data than needed to test your code.
<sup>[[link](#needed-data)]</sup>
Expand Down