Skip to content

Commit

Permalink
Improve Rubocop config, fix offenses
Browse files Browse the repository at this point in the history
  • Loading branch information
n-rodriguez committed Aug 31, 2024
1 parent bee170e commit 5849609
Show file tree
Hide file tree
Showing 10 changed files with 119 additions and 80 deletions.
27 changes: 26 additions & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ AllCops:
- bin/*
- gemfiles/*
- benchmarks/*
- spec/**/*
- spec/dummy/**/*

Gemspec/RequireMFA:
Enabled: false
Expand All @@ -35,6 +35,9 @@ Style/StringLiterals:
Style/ArgumentsForwarding:
Enabled: false

Style/BlockDelimiters:
AllowedPatterns: ['expect']

##########
# LAYOUT #
##########
Expand Down Expand Up @@ -66,3 +69,25 @@ Layout/IndentationConsistency:

Naming/BlockForwarding:
Enabled: false

#########
# RSPEC #
#########

RSpec/MultipleExpectations:
Max: 5

RSpec/NestedGroups:
Max: 4

RSpec/ExampleLength:
Enabled: false

RSpec/VerifiedDoubles:
Enabled: false

RSpec/MetadataStyle:
EnforcedStyle: hash

FactoryBot/SyntaxMethods:
Enabled: false
2 changes: 2 additions & 0 deletions spec/config_capybara.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

def register_driver(driver_name, args = [])
opts = { js_errors: true, headless: true, window_size: [1920, 1200], browser_options: {} }
args.each do |arg|
Expand Down
11 changes: 7 additions & 4 deletions spec/config_rspec.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
# frozen_string_literal: true

# Configure RSpec
RSpec.configure do |config|
config.include Capybara::DSL
config.include HaveTextMatcher

# Use DB agnostic schema by default
load Rails.root.join('db', 'schema.rb').to_s
load Rails.root.join("db", "schema.rb").to_s

config.order = :random
Kernel.srand config.seed
Expand All @@ -19,7 +22,7 @@
config.use_transactional_fixtures = true

# run retry only on features
config.around :each, :js do |ex|
config.around :each, js: true do |ex|
ex.run_with_retry retry: 3
end

Expand All @@ -31,11 +34,11 @@
DatabaseCleaner.strategy = :transaction
end

config.before(:each) do
config.before do
DatabaseCleaner.start
end

config.after(:each) do
config.after do
DatabaseCleaner.clean
end
end
8 changes: 5 additions & 3 deletions spec/draper/decorator_spec.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
require 'spec_helper'
# frozen_string_literal: true

require "spec_helper"

RSpec.describe Draper::Decorator do

Expand Down Expand Up @@ -115,10 +117,10 @@

describe "#attributes" do
it "returns only the object's attributes that are implemented by the decorator" do
decorator = described_class.new(double(attributes: {foo: "bar", baz: "qux"}))
decorator = described_class.new(double(attributes: { foo: "bar", baz: "qux" }))
allow(decorator).to receive(:foo)

expect(decorator.attributes).to eq({foo: "bar"})
expect(decorator.attributes).to eq({ foo: "bar" })
end
end

Expand Down
4 changes: 3 additions & 1 deletion spec/draper/draper_spec.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
require 'spec_helper'
# frozen_string_literal: true

require "spec_helper"

RSpec.describe Draper do
describe ".decorate" do
Expand Down
30 changes: 16 additions & 14 deletions spec/draper/view_context/build_strategy_spec.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
require 'spec_helper'
# frozen_string_literal: true

require "spec_helper"

RSpec.describe "ViewContext" do
def fake_view_context
Expand All @@ -15,7 +17,7 @@ def fake_controller(view_context = fake_view_context)
it "returns the controller's view context" do
view_context = fake_view_context
allow(Draper::ViewContext).to receive_messages controller: fake_controller(view_context)
strategy = Draper::ViewContext::BuildStrategy::Full.new
strategy = described_class.new

expect(strategy.call).to be view_context
end
Expand All @@ -24,7 +26,7 @@ def fake_controller(view_context = fake_view_context)
context "when a current controller is not set" do
it "uses ApplicationController" do
expect(Draper::ViewContext.controller).to be_nil
view_context = Draper::ViewContext::BuildStrategy::Full.new.call
view_context = described_class.new.call
expect(view_context.controller).to eq Draper::ViewContext.controller
expect(view_context.controller).to be_an ApplicationController
end
Expand All @@ -33,7 +35,7 @@ def fake_controller(view_context = fake_view_context)
it "adds a request if one is not defined" do
controller = Class.new(ActionController::Base).new
allow(Draper::ViewContext).to receive_messages controller: controller
strategy = Draper::ViewContext::BuildStrategy::Full.new
strategy = described_class.new

expect(controller.request).to be_nil
strategy.call
Expand All @@ -48,14 +50,14 @@ def fake_controller(view_context = fake_view_context)
it "compatible with rails 5.1 change on ActionController::TestRequest.create method" do
ActionController::TestRequest.class_eval do
if ActionController::TestRequest.method(:create).parameters.first == []
def create controller_class
def create(_controller_class)
create
end
end
end
controller = Class.new(ActionController::Base).new
allow(Draper::ViewContext).to receive_messages controller: controller
strategy = Draper::ViewContext::BuildStrategy::Full.new
strategy = described_class.new

expect(controller.request).to be_nil
strategy.call
Expand All @@ -64,7 +66,7 @@ def create controller_class

it "adds methods to the view context from the constructor block" do
allow(Draper::ViewContext).to receive(:controller).and_return(fake_controller)
strategy = Draper::ViewContext::BuildStrategy::Full.new do
strategy = described_class.new do
def a_helper_method; end
end

Expand All @@ -77,7 +79,7 @@ def a_helper_method; end
helpers = Module.new do
def a_helper_method; end
end
strategy = Draper::ViewContext::BuildStrategy::Full.new do
strategy = described_class.new do
include helpers
end

Expand All @@ -89,7 +91,7 @@ def a_helper_method; end
describe Draper::ViewContext::BuildStrategy::Fast do
describe "#call" do
it "returns an instance of a subclass of ActionView::Base" do
strategy = Draper::ViewContext::BuildStrategy::Fast.new
strategy = described_class.new

returned = strategy.call

Expand All @@ -98,19 +100,19 @@ def a_helper_method; end
end

it "returns different instances each time" do
strategy = Draper::ViewContext::BuildStrategy::Fast.new
strategy = described_class.new

expect(strategy.call).not_to be strategy.call
end

it "returns the same subclass each time" do
strategy = Draper::ViewContext::BuildStrategy::Fast.new
strategy = described_class.new

expect(strategy.call.class).to be strategy.call.class
expect(strategy.call.class).to be strategy.call.class # rubocop:disable RSpec/IdenticalEqualityAssertion
end

it "adds methods to the view context from the constructor block" do
strategy = Draper::ViewContext::BuildStrategy::Fast.new do
strategy = described_class.new do
def a_helper_method; end
end

Expand All @@ -121,7 +123,7 @@ def a_helper_method; end
helpers = Module.new do
def a_helper_method; end
end
strategy = Draper::ViewContext::BuildStrategy::Fast.new do
strategy = described_class.new do
include helpers
end

Expand Down
42 changes: 22 additions & 20 deletions spec/draper/view_context_spec.rb
Original file line number Diff line number Diff line change
@@ -1,28 +1,30 @@
require 'spec_helper'
# frozen_string_literal: true

require "spec_helper"

RSpec.describe Draper::ViewContext do

describe ".current" do
it "returns the stored view context from RequestStore" do
allow(RequestStore).to receive_messages store: { current_view_context: :stored_view_context }

expect(Draper::ViewContext.current).to be :stored_view_context
expect(described_class.current).to be :stored_view_context
end

context "when no view context is stored" do
it "builds a view context" do
allow(RequestStore).to receive_messages store: {}
allow(Draper::ViewContext).to receive_messages build_strategy: -> { :new_view_context }
allow(described_class).to receive_messages build_strategy: -> { :new_view_context }

expect(Draper::ViewContext.current).to be :new_view_context
expect(described_class.current).to be :new_view_context
end

it "stores the built view context" do
store = {}
allow(RequestStore).to receive_messages store: store
allow(Draper::ViewContext).to receive_messages build_strategy: -> { :new_view_context }
allow(described_class).to receive_messages build_strategy: -> { :new_view_context }

Draper::ViewContext.current
described_class.current
expect(store[:current_view_context]).to be :new_view_context
end
end
Expand All @@ -33,7 +35,7 @@
store = {}
allow(RequestStore).to receive_messages store: store

Draper::ViewContext.current = :stored_view_context
described_class.current = :stored_view_context
expect(store[:current_view_context]).to be :stored_view_context
end
end
Expand All @@ -42,7 +44,7 @@
it "returns the stored controller from RequestStore" do
allow(RequestStore).to receive_messages store: { current_controller: :stored_controller }

expect(Draper::ViewContext.controller).to be :stored_controller
expect(described_class.controller).to be :stored_controller
end
end

Expand All @@ -51,7 +53,7 @@
store = {}
allow(RequestStore).to receive_messages store: store

Draper::ViewContext.controller = :stored_controller
described_class.controller = :stored_controller
expect(store[:current_controller]).to be :stored_controller
end

Expand All @@ -63,7 +65,7 @@

allow(RequestStore).to receive_messages store: store

Draper::ViewContext.controller = :other_stored_controller
described_class.controller = :other_stored_controller

expect(store).to include(current_controller: :other_stored_controller)
expect(store).not_to include(:current_view_context)
Expand All @@ -77,7 +79,7 @@

allow(RequestStore).to receive_messages store: store

Draper::ViewContext.controller = :stored_controller
described_class.controller = :stored_controller

expect(store).to include(current_controller: :stored_controller)
expect(store).to include(current_view_context: :stored_view_context)
Expand All @@ -86,25 +88,25 @@

describe ".build" do
it "returns a new view context using the build strategy" do
allow(Draper::ViewContext).to receive_messages build_strategy: -> { :new_view_context }
allow(described_class).to receive_messages build_strategy: -> { :new_view_context }

expect(Draper::ViewContext.build).to be :new_view_context
expect(described_class.build).to be :new_view_context
end
end

describe ".build!" do
it "returns a helper proxy for the new view context" do
allow(Draper::ViewContext).to receive_messages build_strategy: -> { :new_view_context }
allow(described_class).to receive_messages build_strategy: -> { :new_view_context }

expect(Draper::ViewContext.build!).to be :new_view_context
expect(described_class.build!).to be :new_view_context
end

it "stores the helper proxy" do
store = {}
allow(RequestStore).to receive_messages store: store
allow(Draper::ViewContext).to receive_messages build_strategy: -> { :new_view_context }
allow(described_class).to receive_messages build_strategy: -> { :new_view_context }

Draper::ViewContext.build!
described_class.build!
expect(store[:current_view_context]).to be :new_view_context
end
end
Expand All @@ -114,19 +116,19 @@
store = { current_controller: :stored_controller, current_view_context: :stored_view_context }
allow(RequestStore).to receive_messages store: store

Draper::ViewContext.clear!
described_class.clear!
expect(store).not_to have_key :current_controller
expect(store).not_to have_key :current_view_context
end
end

describe ".build_strategy" do
it "defaults to full" do
expect(Draper::ViewContext.build_strategy).to be_a Draper::ViewContext::BuildStrategy::Full
expect(described_class.build_strategy).to be_a Draper::ViewContext::BuildStrategy::Full
end

it "memoizes" do
expect(Draper::ViewContext.build_strategy).to be Draper::ViewContext.build_strategy
expect(described_class.build_strategy).to be described_class.build_strategy # rubocop:disable RSpec/IdenticalEqualityAssertion
end
end
end
Loading

0 comments on commit 5849609

Please sign in to comment.