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

Exception when running test suite (3.3.1) – build_scoped_id_for': undefined method call' for :method:Symbol #1

Closed
dgmstuart opened this issue Nov 5, 2015 · 6 comments

Comments

@dgmstuart
Copy link
Contributor

This error happens on RSpec 3.3.1 and above when trying to run any spec (not just those which use the spryte dsl). Full trace: rspec/rspec-core#2008

The inclusion which causes the errors is:

config.extend Spryte::RSpec::Macros 

(config.include Spryte::RSpec::Helpers seems to be fine)

With this in the rails_helper a spec comprising one empty it block is sufficient to throw the error.

@dgmstuart
Copy link
Contributor Author

The offending method seems to be this:

# spryte/lib/spryte/rspec/macros.rb
def method(verb)
  let(:method) { verb.to_sym }
end

Presumably verb is expected to be one of the http verbs, but unless I've misunderstood what I'm looking at, it's getting passed :next_runnable_index_for at the point at which it's blowing up.

@dgmstuart
Copy link
Contributor Author

:next_runnable_index_for is related to this commit: Add ids to examples and groups.

I strongly suspect that the issue relates to this:

# lib/rspec/core/example_group.rb
@metadata = Metadata::ExampleGroupHash.create(
  superclass_metadata, user_metadata,
  superclass.method(:next_runnable_index_for),
  description, *args, &example_group_block
)

So this is a nameclash between Object#method and Spryte::RSpec::Macros#method.

I guess one solution would be to change the Spryte DSL to use verb/http_verb/http_method instead of method. I'll mention this on the RSpec issue as well though.

As an intermediate/additional step, it might also make sense to only accept a whitelisted set of verbs, and raise an exception if anything else is passed.

@zeeraw thoughts on how to proceed?

@zeevallin
Copy link
Owner

@dgmstuart I'm very much in favour of using something else since the clash is with ruby core.

Referring to it as method rather than verb aligns with the HTTP RFC / 9 HTTP Method Definitions. I think using verb would be confusing.

describe "list the toys" do
  verb :get
  path "/api/toys"
end

I think http_method is too verbose. Using the DSL we already know we're in the context of making an HTTP request.

describe "insert a mug in to the cupboard" do
  http_method :post
  path "/kitchen/cupboard/2/mugs"
end

My suggestion would be to use something along the lines of through.

describe "move king to D3" do
  path "/chess/board/D/3"
  through :put
end

What are your thoughts?

@dgmstuart
Copy link
Contributor Author

Noted.

I think any word is going to be some sort of compromise, since verb and method seem to be the only standard terms used.

through sounds fine. Doesn't seem to appear in Ruby or Rails.

Any way it could clash with shoulda-matchers?

it { should have_many(:projects).through(:clients) }

thoughtbot/shoulda-matchers#646 (comment)

@zeevallin
Copy link
Owner

@dgmstuart Not really, since the shoulda-matchers #through exists in the assertion context. The spryte #through method would exist in the definition context or whatever it's called.

describe "thing" do
  # here lives spryte
  it { expect(true).to # here lives shoulda-matchers  }
end

zeevallin added a commit that referenced this issue Nov 11, 2015
As explained in #1 by @dgmstuart, the `#method` in spryte has a nameclash with `Object#method`.

Chose to name it to `#through` rather than `#http_method` or `#verb` since it's a semantically intuitive DSL.

Made sure that we output a deprecation warning pointing users to the new method and that the test suite will fail once we upgrade major version without removing the deprecated method.
@zeevallin
Copy link
Owner

Released 📦 v0.1.0

Thanks for your contributions @dgmstuart, much appreciated!

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

No branches or pull requests

2 participants