Skip to content

Commit

Permalink
Merge pull request #1526 from cpetschnig/avoid_ruby_warnings
Browse files Browse the repository at this point in the history
Avoid noise when code runs with Ruby warnings
  • Loading branch information
dm1try authored Dec 1, 2016
2 parents b511add + dba6064 commit dc5e4b0
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 44 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Next Release
* [#1517](https://github.com/ruby-grape/grape/pull/1517), [#1089](https://github.com/ruby-grape/grape/pull/1089): Fix: priority of ANY routes - [@namusyaka](https://github.com/namusyaka), [@wagenet](https://github.com/wagenet).
* [#1512](https://github.com/ruby-grape/grape/pull/1512): Fix: deeply nested parameters are included within `#declared(params)` - [@krbs](https://github.com/krbs).
* [#1510](https://github.com/ruby-grape/grape/pull/1510): Fix: inconsistent validation for multiple parameters - [@dgasper](https://github.com/dgasper).
* [#1526](https://github.com/ruby-grape/grape/pull/1526): Reduce warnings caused by instance variables not initialized - [@cpetschnig](https://github.com/cpetschnig).

0.18.0 (10/7/2016)
==================
Expand Down
2 changes: 2 additions & 0 deletions lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ def initialize(new_settings, options = {}, &block)
@options[:route_options] ||= {}

@lazy_initialize_lock = Mutex.new
@lazy_initialized = nil
@block = nil

return unless block_given?

Expand Down
1 change: 1 addition & 0 deletions lib/grape/middleware/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class Base
def initialize(app, **options)
@app = app
@options = default_options.merge(**options)
@app_response = nil
end

def default_options
Expand Down
1 change: 1 addition & 0 deletions lib/grape/validations/params_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def initialize(opts, &block)
@group = opts[:group] || {}
@dependent_on = opts[:dependent_on]
@declared_params = []
@index = nil

instance_eval(&block) if block_given?

Expand Down
2 changes: 1 addition & 1 deletion spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1202,7 +1202,7 @@ class PhonyMiddleware
def initialize(app, *args)
@args = args
@app = app
@block = true if block_given?
@block = block_given? ? true : nil
end

def call(env)
Expand Down
69 changes: 39 additions & 30 deletions spec/grape/middleware/formatter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,46 +4,53 @@
subject { Grape::Middleware::Formatter.new(app) }
before { allow(subject).to receive(:dup).and_return(subject) }

let(:app) { ->(_env) { [200, {}, [@body || { 'foo' => 'bar' }]] } }
let(:body) { { 'foo' => 'bar' } }
let(:app) { ->(_env) { [200, {}, [body]] } }

context 'serialization' do
let(:body) { { 'abc' => 'def' } }
it 'looks at the bodies for possibly serializable data' do
@body = { 'abc' => 'def' }
_, _, bodies = *subject.call('PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/json')
bodies.each { |b| expect(b).to eq(MultiJson.dump(@body)) }
bodies.each { |b| expect(b).to eq(MultiJson.dump(body)) }
end

it 'calls #to_json since default format is json' do
@body = ['foo']
@body.instance_eval do
def to_json
'"bar"'
context 'default format' do
let(:body) { ['foo'] }
it 'calls #to_json since default format is json' do
body.instance_eval do
def to_json
'"bar"'
end
end
end

subject.call('PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/json').to_a.last.each { |b| expect(b).to eq('"bar"') }
subject.call('PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/json').to_a.last.each { |b| expect(b).to eq('"bar"') }
end
end

it 'calls #to_json if the content type is jsonapi' do
@body = { 'foos' => [{ 'bar' => 'baz' }] }
@body.instance_eval do
def to_json
'{"foos":[{"bar":"baz"}] }'
context 'jsonapi' do
let(:body) { { 'foos' => [{ 'bar' => 'baz' }] } }
it 'calls #to_json if the content type is jsonapi' do
body.instance_eval do
def to_json
'{"foos":[{"bar":"baz"}] }'
end
end
end

subject.call('PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/vnd.api+json').to_a.last.each { |b| expect(b).to eq('{"foos":[{"bar":"baz"}] }') }
subject.call('PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/vnd.api+json').to_a.last.each { |b| expect(b).to eq('{"foos":[{"bar":"baz"}] }') }
end
end

it 'calls #to_xml if the content type is xml' do
@body = 'string'
@body.instance_eval do
def to_xml
'<bar/>'
context 'xml' do
let(:body) { 'string' }
it 'calls #to_xml if the content type is xml' do
body.instance_eval do
def to_xml
'<bar/>'
end
end
end

subject.call('PATH_INFO' => '/somewhere.xml', 'HTTP_ACCEPT' => 'application/json').to_a.last.each { |b| expect(b).to eq('<bar/>') }
subject.call('PATH_INFO' => '/somewhere.xml', 'HTTP_ACCEPT' => 'application/json').to_a.last.each { |b| expect(b).to eq('<bar/>') }
end
end
end

Expand Down Expand Up @@ -189,10 +196,12 @@ def to_xml
_, _, body = subject.call('PATH_INFO' => '/info.custom')
expect(body.body).to eq(['CUSTOM FORMAT'])
end
it 'uses default json formatter' do
@body = ['blah']
_, _, body = subject.call('PATH_INFO' => '/info.json')
expect(body.body).to eq(['["blah"]'])
context 'default' do
let(:body) { ['blah'] }
it 'uses default json formatter' do
_, _, body = subject.call('PATH_INFO' => '/info.json')
expect(body.body).to eq(['["blah"]'])
end
end
it 'uses custom json formatter' do
subject.options[:formatters][:json] = ->(_obj, _env) { 'CUSTOM JSON FORMAT' }
Expand Down Expand Up @@ -284,10 +293,10 @@ def to_xml
end

context 'send file' do
let(:app) { ->(_env) { [200, {}, @body] } }
let(:body) { Grape::ServeFile::FileResponse.new('file') }
let(:app) { ->(_env) { [200, {}, body] } }

it 'returns Grape::Uril::SendFileReponse' do
@body = Grape::ServeFile::FileResponse.new('file')
env = { 'PATH_INFO' => '/somewhere', 'HTTP_ACCEPT' => 'application/json' }
expect(subject.call(env)).to be_a(Grape::ServeFile::SendfileResponse)
end
Expand Down
25 changes: 15 additions & 10 deletions spec/grape/middleware/versioner/param_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

describe Grape::Middleware::Versioner::Param do
let(:app) { ->(env) { [200, env, env['api.version']] } }
subject { Grape::Middleware::Versioner::Param.new(app, @options || {}) }
let(:options) { {} }
subject { Grape::Middleware::Versioner::Param.new(app, options) }

it 'sets the API version based on the default param (apiver)' do
env = Rack::MockRequest.env_for('/awesome', params: { 'apiver' => 'v1' })
Expand All @@ -22,7 +23,7 @@
end

context 'with specified parameter name' do
before { @options = { version_options: { parameter: 'v' } } }
let(:options) { { version_options: { parameter: 'v' } } }
it 'sets the API version based on the custom parameter name' do
env = Rack::MockRequest.env_for('/awesome', params: { 'v' => 'v1' })
expect(subject.call(env)[1]['api.version']).to eq('v1')
Expand All @@ -34,7 +35,7 @@
end

context 'with specified versions' do
before { @options = { versions: %w(v1 v2) } }
let(:options) { { versions: %w(v1 v2) } }
it 'throws an error if a non-allowed version is specified' do
env = Rack::MockRequest.env_for('/awesome', params: { 'apiver' => 'v3' })
expect(catch(:error) { subject.call(env) }[:status]).to eq(404)
Expand All @@ -45,13 +46,17 @@
end
end

it 'returns a 200 when no version is set (matches the first version found)' do
@options = {
versions: ['v1'],
version_options: { using: :header }
}
env = Rack::MockRequest.env_for('/awesome', params: {})
expect(subject.call(env).first).to eq(200)
context 'when no version is set' do
let(:options) do
{
versions: ['v1'],
version_options: { using: :header }
}
end
it 'returns a 200 (matches the first version found)' do
env = Rack::MockRequest.env_for('/awesome', params: {})
expect(subject.call(env).first).to eq(200)
end
end

context 'when there are multiple versions without a custom param' do
Expand Down
7 changes: 4 additions & 3 deletions spec/grape/middleware/versioner/path_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

describe Grape::Middleware::Versioner::Path do
let(:app) { ->(env) { [200, env, env['api.version']] } }
subject { Grape::Middleware::Versioner::Path.new(app, @options || {}) }
let(:options) { {} }
subject { Grape::Middleware::Versioner::Path.new(app, options) }

it 'sets the API version based on the first path' do
expect(subject.call('PATH_INFO' => '/v1/awesome').last).to eq('v1')
Expand All @@ -17,7 +18,7 @@
end

context 'with a pattern' do
before { @options = { pattern: /v./i } }
let(:options) { { pattern: /v./i } }
it 'sets the version if it matches' do
expect(subject.call('PATH_INFO' => '/v1/awesome').last).to eq('v1')
end
Expand All @@ -29,7 +30,7 @@

[%w(v1 v2), [:v1, :v2], [:v1, 'v2'], ['v1', :v2]].each do |versions|
context "with specified versions as #{versions}" do
before { @options = { versions: versions } }
let(:options) { { versions: versions } }

it 'throws an error if a non-allowed version is specified' do
expect(catch(:error) { subject.call('PATH_INFO' => '/v3/awesome') }[:status]).to eq(404)
Expand Down

0 comments on commit dc5e4b0

Please sign in to comment.