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

Custom parameter method #312

Merged
merged 3 commits into from
Dec 8, 2016

Conversation

sineed
Copy link

@sineed sineed commented Nov 15, 2016

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

@oestrich oestrich merged commit ba62ae1 into zipmark:master Dec 8, 2016
@oestrich
Copy link
Contributor

oestrich commented Dec 8, 2016

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 GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants