stevo

# Before: expectations are set before and after subject; 
# whole code is in one chunk
describe UsersController do
  describe "#create" do
    it "uses UserForm to persist user" do
      user = create(:user, name: "Mike")
      expect(UserForm).to receive(:call).with(kind_of(User), name: "Tony")
      put :update, params: { id: user.id, user: { name: "Tony" } }
      expect(response).to be_ok
    end

stevo

# Before - using subject, let, and before
describe User do
  subject { create(:user) }

  describe "#latest_tasks" do
    context "when tasks exist for user and another user" do
      let(:another_user) { create(:user) }
      before do
        create_list(:task, 3, user: subject)
        create_list(:task, 5, user: another_user)

stevo

# Before - use of described_class; 
# exact values are visible only in setup and not in assertions; 
# initial value is not known; 
# values used are not realistic; 
# irrelevant values are defined
describe UserForm do
  describe "#save" do
    it "persists changes" do
      user = create(:user, age: 999)
      attributes = { first_name: "abc123" }

stevo

# Before - not clear what object / method do we test; 
# not clear what is the expected behavior; 
# context description is mixed with subject
describe "user form" do
  describe "saving with phone number" do
      it do
        allow(SmsGateway).to receive(:send_message)
        attributes = { phone_number: "0048 500 111 222" }

        form = UserForm.new(User.new, attributes)

stevo

describe CloseOrder do
  describe '#call' do
    it 'sends notification to customer and updates closed_at' do
      order = create(:order)
      service = CloseOrder.new(order)
      allow(service).to receive(:send_notification)
      allow(service).to receive(:update_closed_at_timestamp)
      
      service.call
      

stevo

# Poor specs
# ==========

describe CloseOrder do
  # Testing #call using partial double of object being tested 
  # looks unnatural and feels as a duplication
  describe '#call' do
    it do
      order = create(:order)
      service = CloseOrder.new(order)

stevo

# Poor specs
# ==========

describe PurgeDomainUsers do
  describe '.call' do
    # Test does not leave any freedom in implementation 
    it 'removes all users with email matching domain provided' do
      users = instance_double(:users_collection)
      allow(User).to receive(:where) { users }
      allow(users).to receive(:destroy_all)

stevo

# Poor specs
# ==========

# Test focus is divided between how library is used and and what is the expected behavior of tested class
describe ImportInvestigations do
  describe '.call' do
    it 'imports investigations to database' do
      stub_const('ImportInvestigations::IMPORT_URL', 'http://gcpd.dev/investigations.json')
      investigations_json = [
        { uid: '123', name: 'Joker: prison escape' }, 

stevo

# Poor specs
# ==========

# 3rd party library and framework methods are used directly to facilitate tests
describe ExportActiveOrders do
  describe '.call' do
    it 'exports all active orders and returns a path to exported file' do
      create_list(:order, 2, :active)
      create(:order, :inactive)

stevo

class BookingsController < ApplicationController
  def index
    # `order` was added prematurely and in the result may remain untested
    bookings = Booking.order(customer_name: :asc)

    render :index, locals: { bookings: bookings }
  end
end