Skip to content

Commit

Permalink
Fix leaky modules in specs (#2504)
Browse files Browse the repository at this point in the history
* Replace modules/classes declaration by anonymous modules/classes.

* Rename validator spec with suffix validator_spec.
Rubocop autocorrect

* Fix RSpec/DescribeClass

* Fix rubocop

* Add CHANGELOG.md
  • Loading branch information
ericproulx authored Oct 6, 2024
1 parent 5b0066c commit 277fe3e
Show file tree
Hide file tree
Showing 34 changed files with 1,951 additions and 2,018 deletions.
42 changes: 9 additions & 33 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,18 +1,11 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2024-07-23 11:24:53 UTC using RuboCop version 1.64.1.
# on 2024-10-06 16:00:59 UTC using RuboCop version 1.66.1.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.

# Offense count: 1
# Configuration parameters: AllowedMethods.
# AllowedMethods: enums
Lint/ConstantDefinitionInBlock:
Exclude:
- 'spec/grape/validations/validators/except_values_spec.rb'

# Offense count: 1
# Configuration parameters: CountComments, Max, CountAsOne, AllowedMethods, AllowedPatterns.
Metrics/MethodLength:
Expand All @@ -29,17 +22,6 @@ Naming/VariableNumber:
- 'spec/grape/exceptions/validation_errors_spec.rb'
- 'spec/grape/validations_spec.rb'

# Offense count: 1
# Configuration parameters: IgnoredMetadata.
RSpec/DescribeClass:
Exclude:
- '**/spec/features/**/*'
- '**/spec/requests/**/*'
- '**/spec/routing/**/*'
- '**/spec/system/**/*'
- '**/spec/views/**/*'
- 'spec/grape/named_api_spec.rb'

# Offense count: 2
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: SkipBlocks, EnforcedStyle, OnlyStaticConstants.
Expand Down Expand Up @@ -68,7 +50,7 @@ RSpec/ExpectActual:
# Offense count: 1
RSpec/ExpectInHook:
Exclude:
- 'spec/grape/validations/validators/values_spec.rb'
- 'spec/grape/validations/validators/values_validator_spec.rb'

# Offense count: 6
# Configuration parameters: Max, AllowedIdentifiers, AllowedPatterns.
Expand All @@ -78,7 +60,7 @@ RSpec/IndexedLet:
- 'spec/grape/presenters/presenter_spec.rb'
- 'spec/shared/versioning_examples.rb'

# Offense count: 39
# Offense count: 38
# Configuration parameters: AssignmentOnly.
RSpec/InstanceVariable:
Exclude:
Expand All @@ -87,12 +69,6 @@ RSpec/InstanceVariable:
- 'spec/grape/middleware/base_spec.rb'
- 'spec/grape/middleware/versioner/accept_version_header_spec.rb'
- 'spec/grape/middleware/versioner/header_spec.rb'
- 'spec/grape/validations/validators/except_values_spec.rb'

# Offense count: 6
RSpec/LeakyConstantDeclaration:
Exclude:
- 'spec/grape/validations/validators/except_values_spec.rb'

# Offense count: 1
RSpec/MessageChain:
Expand All @@ -109,22 +85,22 @@ RSpec/RepeatedDescription:
Exclude:
- 'spec/grape/api_spec.rb'
- 'spec/grape/endpoint_spec.rb'
- 'spec/grape/validations/validators/allow_blank_spec.rb'
- 'spec/grape/validations/validators/values_spec.rb'
- 'spec/grape/validations/validators/allow_blank_validator_spec.rb'
- 'spec/grape/validations/validators/values_validator_spec.rb'

# Offense count: 6
RSpec/RepeatedExample:
Exclude:
- 'spec/grape/middleware/versioner/accept_version_header_spec.rb'
- 'spec/grape/validations/validators/allow_blank_spec.rb'
- 'spec/grape/validations/validators/allow_blank_validator_spec.rb'

# Offense count: 10
RSpec/RepeatedExampleGroupDescription:
Exclude:
- 'spec/grape/api_spec.rb'
- 'spec/grape/endpoint_spec.rb'
- 'spec/grape/util/inheritable_setting_spec.rb'
- 'spec/grape/validations/validators/values_spec.rb'
- 'spec/grape/validations/validators/values_validator_spec.rb'

# Offense count: 4
RSpec/StubbedMock:
Expand All @@ -133,7 +109,7 @@ RSpec/StubbedMock:
- 'spec/grape/dsl/routing_spec.rb'
- 'spec/grape/middleware/formatter_spec.rb'

# Offense count: 121
# Offense count: 118
RSpec/SubjectStub:
Exclude:
- 'spec/grape/api_spec.rb'
Expand All @@ -150,7 +126,7 @@ RSpec/SubjectStub:
- 'spec/grape/middleware/globals_spec.rb'
- 'spec/grape/middleware/stack_spec.rb'

# Offense count: 23
# Offense count: 22
# Configuration parameters: IgnoreNameless, IgnoreSymbolicNames.
RSpec/VerifiedDoubles:
Exclude:
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#### Fixes

* [#2504](https://github.com/ruby-grape/grape/pull/2504): Fix leaky modules in specs - [@ericproulx](https://github.com/ericproulx).
* Your contribution here.

### 2.2.0 (2024-09-14)
Expand Down
43 changes: 22 additions & 21 deletions spec/grape/api/deeply_included_options_spec.rb
Original file line number Diff line number Diff line change
@@ -1,21 +1,17 @@
# frozen_string_literal: true

module DeeplyIncludedOptionsSpec
module Defaults
extend ActiveSupport::Concern
included do
format :json
describe Grape::API do
let(:app) do
main_api = api
Class.new(Grape::API) do
mount main_api
end
end

module Admin
module Defaults
extend ActiveSupport::Concern
include DeeplyIncludedOptionsSpec::Defaults
end

class Users < Grape::API
include DeeplyIncludedOptionsSpec::Admin::Defaults
let(:api) do
deeply_included_options = options
Class.new(Grape::API) do
include deeply_included_options

resource :users do
get do
Expand All @@ -25,16 +21,21 @@ class Users < Grape::API
end
end

class Main < Grape::API
mount DeeplyIncludedOptionsSpec::Admin::Users
let(:options) do
deep_included_options_default = default
Module.new do
extend ActiveSupport::Concern
include deep_included_options_default
end
end
end

describe Grape::API do
subject { DeeplyIncludedOptionsSpec::Main }

def app
subject
let(:default) do
Module.new do
extend ActiveSupport::Concern
included do
format :json
end
end
end

it 'works for unspecified format' do
Expand Down
64 changes: 30 additions & 34 deletions spec/grape/dsl/callbacks_spec.rb
Original file line number Diff line number Diff line change
@@ -1,45 +1,41 @@
# frozen_string_literal: true

module Grape
module DSL
module CallbacksSpec
class Dummy
include Grape::DSL::Callbacks
end
end
describe Grape::DSL::Callbacks do
subject { dummy_class }

describe Callbacks do
subject { Class.new(CallbacksSpec::Dummy) }
let(:dummy_class) do
Class.new do
include Grape::DSL::Callbacks
end
end

let(:proc) { -> {} }
let(:proc) { -> {} }

describe '.before' do
it 'adds a block to "before"' do
expect(subject).to receive(:namespace_stackable).with(:befores, proc)
subject.before(&proc)
end
end
describe '.before' do
it 'adds a block to "before"' do
expect(subject).to receive(:namespace_stackable).with(:befores, proc)
subject.before(&proc)
end
end

describe '.before_validation' do
it 'adds a block to "before_validation"' do
expect(subject).to receive(:namespace_stackable).with(:before_validations, proc)
subject.before_validation(&proc)
end
end
describe '.before_validation' do
it 'adds a block to "before_validation"' do
expect(subject).to receive(:namespace_stackable).with(:before_validations, proc)
subject.before_validation(&proc)
end
end

describe '.after_validation' do
it 'adds a block to "after_validation"' do
expect(subject).to receive(:namespace_stackable).with(:after_validations, proc)
subject.after_validation(&proc)
end
end
describe '.after_validation' do
it 'adds a block to "after_validation"' do
expect(subject).to receive(:namespace_stackable).with(:after_validations, proc)
subject.after_validation(&proc)
end
end

describe '.after' do
it 'adds a block to "after"' do
expect(subject).to receive(:namespace_stackable).with(:afters, proc)
subject.after(&proc)
end
end
describe '.after' do
it 'adds a block to "after"' do
expect(subject).to receive(:namespace_stackable).with(:afters, proc)
subject.after(&proc)
end
end
end
93 changes: 45 additions & 48 deletions spec/grape/dsl/headers_spec.rb
Original file line number Diff line number Diff line change
@@ -1,62 +1,59 @@
# frozen_string_literal: true

module Grape
module DSL
module HeadersSpec
class Dummy
include Grape::DSL::Headers
end
describe Grape::DSL::Headers do
subject { dummy_class.new }

let(:dummy_class) do
Class.new do
include Grape::DSL::Headers
end
describe Headers do
subject { HeadersSpec::Dummy.new }
end

let(:header_data) do
{ 'first key' => 'First Value',
'second key' => 'Second Value' }
end

context 'when headers are set' do
describe '#header' do
before do
header_data.each { |k, v| subject.header(k, v) }
end

let(:header_data) do
{ 'first key' => 'First Value',
'second key' => 'Second Value' }
describe 'get' do
it 'returns a specifc value' do
expect(subject.header['first key']).to eq 'First Value'
expect(subject.header['second key']).to eq 'Second Value'
end

it 'returns all set headers' do
expect(subject.header).to eq header_data
expect(subject.headers).to eq header_data
end
end

context 'when headers are set' do
describe '#header' do
before do
header_data.each { |k, v| subject.header(k, v) }
end

describe 'get' do
it 'returns a specifc value' do
expect(subject.header['first key']).to eq 'First Value'
expect(subject.header['second key']).to eq 'Second Value'
end

it 'returns all set headers' do
expect(subject.header).to eq header_data
expect(subject.headers).to eq header_data
end
end

describe 'set' do
it 'returns value' do
expect(subject.header('third key', 'Third Value'))
expect(subject.header['third key']).to eq 'Third Value'
end
end

describe 'delete' do
it 'deletes a header key-value pair' do
expect(subject.header('first key')).to eq header_data['first key']
expect(subject.header).not_to have_key('first key')
end
end
describe 'set' do
it 'returns value' do
expect(subject.header('third key', 'Third Value'))
expect(subject.header['third key']).to eq 'Third Value'
end
end

context 'when no headers are set' do
describe '#header' do
it 'returns nil' do
expect(subject.header['first key']).to be_nil
expect(subject.header('first key')).to be_nil
end
describe 'delete' do
it 'deletes a header key-value pair' do
expect(subject.header('first key')).to eq header_data['first key']
expect(subject.header).not_to have_key('first key')
end
end
end
end

context 'when no headers are set' do
describe '#header' do
it 'returns nil' do
expect(subject.header['first key']).to be_nil
expect(subject.header('first key')).to be_nil
end
end
end
end
Loading

0 comments on commit 277fe3e

Please sign in to comment.