- Follow Rubocop: Always consider our Rubocop rules when writing code. If uncertain about a style or convention, check
.rubocop.ymlor existing code patterns.
Use Hash Value Omission: For Ruby 3.1+ code, use hash value omission syntax when the key and value are the same.
- BAD:
{ instance: instance, duration: duration } - GOOD:
{ instance:, duration: } - BAD:
create(:account, instance: instance, status: status) - GOOD:
create(:account, instance:, status:)
This applies to:
- Method arguments
- Hash literals
- Factory definitions
- Any hash where keys match variable names
Avoid Ugly Multi-line Formatting: When array or hash elements span multiple lines, put each element on its own line. Never create partial multi-line elements that result in awkward ,], or ,), patterns on the same line.
-
BAD - partial multi-line creates ugly
,],:rows: [ ['Label', format_number(value), format_change(value, previous),], ]
-
GOOD - each element on its own line:
rows: [ [ 'Label', format_number(value), format_change(value, previous), ], ]
-
ALSO GOOD - single line if short enough:
rows: [ ['Label', format_number(value), format_change(value, previous)], ]
Note: Trailing commas are fine (and enforced by Rubocop). The issue is the formatting that creates visual clutter.
Use Ruby Ranges: Prefer Ruby range syntax over string comparisons for date/time and numeric ranges.
- BAD:
.where('created_at > ?', 1.day.ago) - GOOD:
.where(created_at: 1.day.ago..)(Use endless range for>=) - GOOD:
.where(created_at: ..1.day.ago)(Use beginless range for<=) - GOOD:
.where(created_at: ...1.day.ago)(Use range with 3 dots for<)
Always specify the exception class in rescue clauses:
- BAD:
rescue => e - GOOD:
rescue StandardError => e - BETTER:
rescue SpecificError => e
Let errors propagate naturally unless you have a specific reason to catch them.
Always use rails generate migration to create new migrations. Avoid creating migration files manually to ensure proper timestamps and naming conventions.
No Side Effects in Models: Avoid putting side-effects (like sending notifications or enqueuing jobs) in models. These belong in the Service Layer / Interactors.
DO NOT write specs that test basic ActiveRecord validations like:
validates :field, presence: truevalidates :field, uniqueness: truevalidates :field, numericality: true- Other standard Rails validations
- These validations are tested by Rails itself
- They add maintenance overhead without adding value
- They make specs brittle to simple model changes
- Complex custom validations with business logic
- Validation behavior in integration/request specs
- The actual user-facing behavior that relies on validations
BAD - Testing simple validations:
describe Favorite do
it { is_expected.to validate_presence_of(:user_id) }
it { is_expected.to validate_uniqueness_of(:topic_id).scoped_to(:user_id) }
endGOOD - Testing through behavior:
describe "POST /api/v1/favorites" do
subject(:request) do
post "/api/v1/favorites", params: { topic_id: }
end
let(:user) { create(:user) }
let(:topic) { create(:topic) }
let(:topic_id) { topic.id }
it "creates a favorite" do
expect { request }.to change(Favorite, :count).by(1)
expect(response).to have_http_status(:created)
end
endGOOD - Testing complex custom validations:
describe 'custom validation' do
subject(:request) { described_class.new(start_date:, end_date:) }
context 'when both start_date and end_date are present' do
let(:start_date) { Date.today }
let(:end_date) { Date.yesterday }
it 'requires end_date to be after start_date' do
expect(request).not_to be_valid
expect(request.errors[:end_date]).to include('must be after start date')
end
end
endPrefer using instance_spy and the expect(...).to receive style for verification over have_received. This mocking style is generally more readable and separates setup from verification effectively.
Use instance_spy primarily when you stub the initializer to return the spy. This allows you to avoid defining the interface of a double explicitly, as spies allow any message.
BAD:
let(:client) { instance_double(ClientClass, call: nil) }
before do
allow(ClientClass).to receive(:new).and_return(client)
end
it 'calls the client' do
subject.perform
expect(client).to have_received(:call)
endGOOD:
let(:client) { instance_spy(ClientClass) }
before do
allow(ClientClass).to receive(:new).and_return(client)
end
it 'calls the client' do
expect(client).to receive(:call)
subject.perform
endFor class methods, set expectations directly before the action - do not use allow + have_received.
BAD:
before do
allow(UserActivityLog).to receive(:create!)
end
it "logs user activity" do
action.call
expect(UserActivityLog).to have_received(:create!).with(
user:, topic:, action_type: :view,
)
endGOOD:
it "logs user activity" do
expect(UserActivityLog).to receive(:create!).with(
user:, topic:, action_type: :view,
)
action.call
endAlways use context blocks to organize specs by conditions or states. This makes specs more readable and clearly separates different scenarios.
Use context for:
- Different states:
context 'when user is authenticated' - Different inputs:
context 'with valid params' - Different configurations:
context 'with feature flag enabled' - Error conditions:
context 'when API fails' - Variations:
context 'with force: true'
BAD:
describe "#call" do
it "returns error for missing topic" do
# ...
end
it "creates favorite for authenticated user" do
user.update!(login_count: 1)
# ...
end
it "logs activity when user is present" do
# ...
end
endGOOD:
describe "#call" do
context "when topic does not exist" do
it "returns an error" do
# ...
end
end
context "with authenticated user" do
let(:user) { create(:user, login_count: 1) }
it "creates a favorite" do
# ...
end
it "logs the favorite action" do
# ...
end
end
context "when database error occurs" do
before do
allow(Favorite).to receive(:create!).and_raise(ActiveRecord::RecordInvalid)
end
it "captures exception and returns error" do
# ...
end
end
endBenefits:
- Clear separation of different scenarios
- Easier to locate specific test cases
- Better test organization and readability
letblocks can be scoped to specific contexts
When creating records that exist only for their side effects (not directly referenced in tests), use before blocks instead of let!. This keeps let blocks clean and focused on records that are actually referenced.
BAD - Creating side-effect records in let!:
let!(:topic_with_activity) do
topic = create(:topic, content_provider:)
create(:user_activity_log, topic:, user:, action_type: :view)
topic
endGOOD - Separate the side-effect into a before block:
let!(:topic_with_activity) do
create(:topic, content_provider:)
end
before do
create(:user_activity_log, topic: topic_with_activity, user:, action_type: :view)
endWhy?
letblocks should return the record they're named after- Side effects hidden inside
letblocks are harder to understand beforeblocks clearly signal "setup that happens before tests"- Makes test data setup more explicit and readable
- Run DB migrations: Make sure to always run any pending database migrations with
rails db:migrate RAILS_ENV=testcommand before running tests. - Run Verification: Once a chunk of work is completed, always run both Rubocop checks and relevant tests to ensure no regressions or style violations.