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

#let doesn't appear to be invoked when defined in a top level describe block in 2.13.0 #817

Closed
jeroenj opened this issue Mar 6, 2013 · 24 comments
Assignees
Milestone

Comments

@jeroenj
Copy link

jeroenj commented Mar 6, 2013

The following code is a simplified snippet of one of my controller specs. After updating RSpec to 2.13.0 (from 2.12.2) the let block doesn't get invoked.

describe MyController do
  before :all do
    @client = FactoryGirl.create :client
  end

  let(:default_parameters){{auth_token: @client.authentication_token}}

  context '#index' do
    it 'should be authorized' do
      get :index, default_parameters
    end
  end
end

However when I put it inside the context '#index' block, it does get invoked and the default_parameters are set as expected.

Has this behavior changed in 2.13.0 and am I doing it wrong, or is it really broken now?

@myronmarston
Copy link
Member

Has this behavior changed in 2.13.0 and am I doing it wrong, or is it really broken now?

The behavior hasn't purposefully changed. You may have found a regression.

I tried to repro your case in an isolated example (since I can't run the spec you have above without having your app):

require 'ostruct'

describe "MyController" do
  before :all do
    @client = OpenStruct.new(authentication_token: 'abcd')
  end 

  let(:default_parameters){{auth_token: @client.authentication_token}}

  context '#index' do
    it 'should be authorized' do
      expect(default_parameters).to include(auth_token: 'abcd')
    end 
  end 
end

...and it passes just fine for me. I'm not sure what "let doesn't appear to be invoked" means -- you are calling the method it defines in your example. What error are you getting? Please include the full backtrace (with --backtrace). To solve this, I'll also need something I can run to reproduce it -- can you create an isolated example out of this? Or can you point me at a project I can clone and run to repro this?

@jeroenj
Copy link
Author

jeroenj commented Mar 7, 2013

Hi, let me try to explain myself more clearly.

I have a helper in spec/support/request_helpers.rb with the following code:

module RequestHelpers
  extend ActiveSupport::Concern

  included do
    [:get, :post, :put, :delete, :head].each do |method|
      class_eval <<-EOV
      def #{method} path, parameters = nil, headers = nil
        parameters = combine_parameters parameters, default_parameters
        super #path, parameters, headers
      end
      EOV
    end
  end

  def default_parameters
  end

  private
  def combine_parameters argument, default
    if argument.is_a?(Hash) && default.is_a?(Hash)
      default.merge argument
    else
      argument || default
    end
  end
end

This helper is included in my spec_helper.

So my example spec looks like this:

describe PostsController do
  before :all do
    @client = Client.create auth_token: 'foobar'
  end

  let(:default_parameters){{auth_token: @client.authentication_token}}

  context '#index' do
    it 'should be authorized' do
      get :index, {filter: 'active'}
    end
  end
end

In this case, the get call should do a request to posts#index with the following params {filter: 'active', auth_token: 'foobar'} since I set the auth_token in the let block. My helper would pick it up and pass it on through the overridden get method.
This behavior worked fine in 2.12.2 and older, but is broken in 2.13.0. As soon as I move the let block inside the context block, it is picked up correctly in 2.13.0 too.

@myronmarston
Copy link
Member

Thanks, that's helpful...although, you still haven't explained how it is specifically broken on 2.13.0. Are you getting an error when the let is invoked? "Broken" can mean many different things, and I need to know exactly how it is broken.

@jeroenj
Copy link
Author

jeroenj commented Mar 7, 2013

When calling get, the parameters hash would be filled with the value given in the let-block (in this example it would be {auth_token: 'foobar'}. However, it doesn't happen anymore. It falls back to the method default_parameters defined in RequestHelpers which is basically nil.

When you would call default_parameters inside of my spec (so right before the get call), it would return the hash ({auth_token: 'foobar'}) when using RSpec 2.12.2 (or older). When doing the exact same thing when using RSpec 2.13.0 it just returns nil.

I'll try to abstract this example from my app into a sample app.

@myronmarston
Copy link
Member

I'll try to abstract this example from my app into a sample app.

Thanks, that would be helpful.

@pjg
Copy link
Contributor

pjg commented Mar 12, 2013

Today I have run into the very same issue, present both in 2.13.0 and the just released 2.13.1 (rspec-core).

It goes like this:

describe ...
  let(:params) do
    {
      something: something
    }
  end

  context ...
     it "..."
       something = Something.new(params)
       ...
       other = Something.new(params)

On the 2nd invocation of Something.new, the params Hash is reset to {}. I have no idea why and this is very much limited to this one new spec I wrote today, which is really weird as well (i.e. I'm using similar technique all over the place).

@myronmarston
Copy link
Member

I'd be happy to take a look and solve this issue if someone can provide a reproducible example I can run. I haven't been able to repro it in my attempts.

@jeroenj
Copy link
Author

jeroenj commented Mar 12, 2013

I'm still planning to give you an example. But I won't be able to do that before next week.

On 12 Mar 2013, at 21:12, Myron Marston notifications@github.com wrote:

I'd be happy to take a look and solve this issue if someone can provide a reproducible example I can run. I haven't been able to repro it in my attempts.


Reply to this email directly or view it on GitHub.

@dchelimsky
Copy link
Contributor

Without the context of the rest of this thread, the first question that comes to my mind from @pjg's example is whether Something.new is modifying the incoming params object. Possible?

@pjg
Copy link
Contributor

pjg commented Mar 12, 2013

@dchelimsky That was a very good question. Something#initialize is indeed deleting the params Hash. A rookie mistake on my part, sorry :) You can ignore my case.

@moll
Copy link

moll commented Apr 1, 2013

I'm experiencing something possibly similar with the following helper:

let(:body) { JSON.parse(response.body) }

Inside an it block nested in another describe, the output of JSON.parse(response.body) and body differ. The latter returns what looks like a placeholder:

"<!DOCTYPE html PUBLIC \"-//W3C//DTD HTML 4.0 Transitional//EN\" \"http://www.w3.org/TR/REC-html40/loose.dtd\">\n\n"

@myronmarston
Copy link
Member

@moll -- without seeing your full example, it's impossible to know for sure what's going on, but one thing to bear in mind is that let creates a memoized method, so the first time body is referenced, it eval's the block and parses whatever response.body is at that point in time. If the value of response.body changes during your example, then the values of body and JSON.parse(response.body) could diverge, as you have seen.

@JonRowe
Copy link
Member

JonRowe commented Apr 2, 2013

@myronmarston didn't we merge in that change to the require order of let / before in 2.13? This sounds like a require order problem to me judging by:

As soon as I move the let block inside the context block, it is picked up correctly in 2.13.0 too.

I'd guess the order in which the let / var in the helper is defined has changed...

@myronmarston
Copy link
Member

@myronmarston didn't we merge in that change to the require order of let / before in 2.13?

I don't understand what you are referring to...

@JonRowe
Copy link
Member

JonRowe commented Apr 2, 2013

I was thinking of #845

@myronmarston
Copy link
Member

We haven't cut a release that includes #845 yet.

@JonRowe
Copy link
Member

JonRowe commented Apr 2, 2013

Then consider my comment redundant :)

@jeroenj
Copy link
Author

jeroenj commented Apr 3, 2013

Hi @myronmarston,

I finally found the time to abstract this issue into an example app. You can find it at https://github.com/jeroenj/rspec-demo.
If you check the commits you can find all cases where I was able to make it work.

After some more investigation it looks like the helper method default_parameters is loaded after the let block has been evaluated.

I hope this could help you (or us) to get a better picture of what is going on.

@myronmarston
Copy link
Member

Thanks for putting that together, @jeroenj. It'll definitely help us track this down.

@moll
Copy link

moll commented Apr 3, 2013

@myronmarston: I did check @_memoized which was empty prior to calling it. If @jeroenj's test case doesn't bring any light to the problem, I'll try to find out more.

@JonRowe
Copy link
Member

JonRowe commented Apr 11, 2013

@moll I'm pretty sure request.body must be rewound after accessing it

@moll
Copy link

moll commented Apr 11, 2013

@JonRowe Rewound? You mean reading from request.body also clears it?

@JonRowe
Copy link
Member

JonRowe commented Apr 11, 2013

The raw Rack request.body object is a StringIO object, not a string, so reading from it moves the cursor to the end, as if awaiting more input. So in order to replay it you need to rewind it. Try using:

let(:body) { request.body }
let(:json) { JSON.parse body }

@ghost ghost assigned JonRowe Apr 11, 2013
@JonRowe
Copy link
Member

JonRowe commented Apr 11, 2013

@jeroenj The short version of this problem is that it's a combination of shadowing the let variable, and something in rspec-rails breaking how lets are defined. I'm closing this and opening an issue on rspec-rails.

The longer version... (for the benefit of @myronmarston @samphippen @soulcutter)

The problem with the original issue here is the shadowing of the let variable. Prior to '2.13.0' we were just applying instance_eval to the block and memoizing the result, in 2.13.0 we added support for using super and other such niceties by defining the memoization and block definition separately.

On pure rspec-core the ancestor order of these blocks (with the included helper) is correct and we get the definition from the let rather than the shadowed method definition in the helper.

However when combined with rspec-rails in your example, the let definition module is being defined on the original defintion of ExampleGroup and not the nested child, and thus the contents of the shadowed method is memoized by our helpers instead of the desired block contents.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants