maschwenk/request_feature_spec_subject.md

## request_feature_spec_subject.md

      
    Raw
  

              request_feature_spec_subject.md
            
          
    Request spec subjects

Who TF cares honestly? 🤷‍♂️ 🤷‍♀️

Great question. Let me preface this by saying I HATE talking about RSpec best practices. It's irrelevant to shipping software and tends to churn up a lot of long, useless discussion. However, I think it's useful to consider best practices when there is a demonstrable functional (not stylistic) difference between two approaches that can affect maintainability, flexibility, and speed.
One style I see quite a lot looks a bit like this:
The before hook tower of doom

# top of the spec

before { get bounties_path(code: 'dilbert') }

it 'something happens' do
  expect(response).to have_http_status :ok
end


describe 'derpy derpy derp I wanna make the request fail' do
  before { allow(MyClass).to receive(:your_class).and_raise(OneHorrificError) }

  it 'something happens' do
    # 10 minutes trying to figure out why your mock didn't get activated
    # spoiler -- it ran _after_ the request because before hooks are run in the order they are defined
  end
end

describe 'derpy derpy derp let me go test another route' do
  before { post bounties_path(code: 'dilbert') }

  it 'something happens' do
    # various hard to track down bugs resulting in accidentically 
    # running two requests for a single example (due to the two hooks)
    
    # hey maybe it goes along fine and nobody notices but OOPS ‼ 
    # you just ran 2 requests for every expectation for no reason
    expect(response).to have_http_status :ok
  end
end
before hooks should be used sparingly. The most insidious form of them is when the code inside the hook is very heavy -- say logging in and visiting a page in Capybara (more on that later). I fix quite a lot of intermittent specs in our codebase 💅 💅 💅 and I find that almost every time a clumsy before hook is involved.
Examples:
https://github.com/bugcrowd/crowdcontrol/pull/7685/files
https://github.com/bugcrowd/crowdcontrol/pull/8497/files
https://github.com/bugcrowd/crowdcontrol/pull/8662/files
Solutions

it { is expected to live only in the example }

One very reasonable way of dealing with this is by not using before hooks at all and sticking to just being very explicit/intensional about what you're doing, e.g.
it do
 expect { get(:page, param: 1) }.to do something
end
This is a great way of doing things and has very little chance of biting you due to the explicitness
before { request } ; subject { response }

This is another great approach but does not allow for throwing any types of mocks in front of it once you go into deeper contexts.
Request/Reponse approach 💪💪💪💪💪

Let's try something else...something radicial...something that looks terrible at first glance.
Most request specs are really testing the same thing. The "subject under test" (in the RSpec lingo) is the request and reponse cycle.
The request methods (get, post, delete, etc) provided by RSpec return the numeric response code of the request. That's it. Not much to introspec, not much to make assertions on.
It's not common to make the subject of a spec the result of two method calls, but let me explain why it can be a nice way to do DRY things up and have maintainable specs!
# top of the spec

# you can call this anything, but it's helpful to name it so that it's
# clear its executing the request and returning the response 
# (`request_response` could work if you want to be more succinct)

subject(:request_yielding_response) do 
  http_request
  response
end

# @tim made a good point, it's a good idea to name this something other than `request`
# so that it doesn't shadow `request` which is automatically defined by RSpec
let(:http_request) { get bounties_path(code: 'dilbert') } 

# one line syntax 
# (if you're not familar with RSpec this impliclity called subject -- thus eagerly
# evaluating `request_yielding_response` and executing the query)
it { is_expected.to be_ok } 
it 'blah' do
  expect { request_yielding_response }.to change(Bounty, :count).by(1)
  # but maybe don't make assertions on database changes in a request spec? 🤷‍♂️
end

describe 'a different endpoint'
  let(:http_request) { get another_path (code: 'dilbert') }

  it { is_expected.to be_ok }
end

describe 'the json response' do
  subject(:json_response) { JSON.parse(request_yielding_response) }

  it { is_expected.to be a_hash_including(bizzle: 'dizzle') }
end
.
.
.
.

# Here's the part that actually matters and makes this pattern 🚨🚨'Good'🚨🚨
context 'when the saints' do
  context 'come marchin' do
    context 'in' do
      let(:http_request) { get very_specific_path(code: 'dilbert') }

      # dang...I'm deeply nested in a context but now I need to do some spying/mocking
      # no problem -- request_yielding_response is lazy defined, so we can do whatever
      # shenanigans we want here and not need to worry about execution order
      before { allow(Grizzle).to receive(:bizzle).and_return(:zizzle) }

      it { is_expected.to be_ok }
    end
  end
end

# keep in mind we stil have the flexibility to support anyone's preferred style/pecadillos
# if you don't like implicit subjects or before hooks you can throw them directly into the example
it do
  allow(Zerp).to receive(:merp).and_return(:blerp)
  request_yielding_response
  expect('blah').to eq 'blah'
end

describe 'im a eager spy typa person' do
  # eager evaluation of the spy!
  let!(:mailer_spy) { class_spy('TrackerUserMailer', delay: true).as_stubbed_const }
  
  it do
    request_yielding_response
    expect(mailer_spy).to have_received(:delay).exactly(:twice)
  end
end
Pros


Lazy subject evaluation allows for:

redefining of the subject (useful for testing raw responses as well as json responses)
closer control of the order of execution


RSpec memoizes subject within an example. That means that it's virtually impossible for us to accidentally reissue the request accidentally. It's pretty much perfect for this style.
It's nice to have some basic conventions that we can use (but also not use when it doesn't make sense)
Avoids the aforementioned deeply nested before hooks of doom!
"Sometimes it's good to have some conventions" -- DHH, probably

Cons


Some folks might argue that doing a bunch of deep nesting is uncouth. Totally valid but I think that's a seperate discussion
Read this screed and come up with your own cons!

Capybara: The Sequel

Wow congrats on getting all the way down here.
What if we took the same priciples from above and applied them to feature tests? What is the subject under test of a Capybara test MOST of the time?
The request_yielding approach

subject(:page_after_visiting) do
  login_as(user)
  page_to_visit
  page # RSpec feature specs give us this
end

let(:page_to_visit) { visit bounties_page }

it 'has something interesting on it' do
  is_expected.to have_text 'chicken can be a healthy alternative to beef'
end

context 'another page' do
  let(:page_to_visit) { visit submissions page }

  it 'has something interesting on it' do
    is_expected.to have_text 'chicken can be a healthy alternative to beef'
  end
end

context 'with some existing records' do
  # eager evaluate to make sure it's in the db before loading the page
  # think about how many hours you've wasted thinking there should be records only to realize
  # the page had already loaded when you created the records
  let!(:submission) { create :submission } 

  it { is_expected.to have_text 'when you have existing data something shows here' } 
end
The before hook tower of doom

This approach has all the issues described above and more:
before do
  login as somebody
  visit a page
end

it 'has something interesting on it' do
  is_expected.to have_text 'chicken can be a healthy alternative to beef'
end

context 'herp derp here I test a different page' do
  before { visit another_page }

  it 'has something interesting on it' do
    # Works great 👍 👍 👍
    # BUT ooops you just visited the same page twice...and feature tests
    # are slow so this is not great 👎 👎 👎
    is_expected.to have_text 'chicken can be a healthy alternative to beef'
  end
end