Conversation

sineed

Related to #308

This PR add a method option that allows to use custom methods as a parameter value

For example:

post "/orders" do
  parameter :name, "Order name", required: true, method: :custom_name

  let(:custom_name) { "Custom order name" }

  ...
end

Also made some refactorings

@oestrichoestrich merged commit ba62ae1 into zipmark:master Dec 8, 2016
@oestrich

Thanks! I think this will fix a good deal of issues people have.

Retrieving of parameter value goes through several steps:
1. if `method` option is defined and test case responds to this method then this method is used;
2. if test case responds to scoped method then this method is used;
3. overwise unscoped method is used.
Copy link
Contributor

@rafaelsales rafaelsales Jul 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oestrich @sineed This fallback won't work for some scenarios.

Example:

get '/foo' do
  parameter :status, "Blah", method: :status_param # this param is not required

  example 'meh' do
    expect(status).to eq 200
  end
end

Even before entering the example block, rspec api documentation calls status to prepare the request, because status_param is not defined. When it calls status, it fails with No response yet. Request a page first, because the status method is only supposed to be called after a request is done.

I don't see a reason to have this fallback mechanism. If the programmer specifies a different method name for a param, just use it if present, if not, do not use the param at all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your example is working as intended. If instead the example is an example_request then we hit your failure, which does exist. Status is a weird method to override, and we've always had an issue with it.

The problem line seems like it would be this one. I will look at a PR if you want to fix it, otherwise I don't see why you would change the method and not define it.

Copy link
Contributor

@rafaelsales rafaelsales Jul 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oestrich The status was just an example, but this issue happens to other reserved words on the context of a test example as well. I created a PR: #352
Thanks

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

Successfully merging this pull request may close these issues.