From 7925c47653b4e3c06661ffefac192efcf0c48144 Mon Sep 17 00:00:00 2001 From: Daniel Colson Date: Sun, 21 Oct 2018 11:39:48 -0400 Subject: [PATCH] Remove remaining RuboCop TODOs Closes #1202 We have fixed the bulk of the RuboCop TODOs. The main TODO remaining (line length) involves a large number of files, and I don't think it is worth trying to update them all at once. I did fix a few of the worst offenders so we could bring the max down a bit. We can gradually bring this number down as we fix more of the violations. --- .rubocop.yml | 51 +++++++---------------- lib/factory_bot/attribute_assigner.rb | 4 +- spec/acceptance/build_stubbed_spec.rb | 3 +- spec/acceptance/modify_factories_spec.rb | 6 ++- spec/acceptance/nested_attributes_spec.rb | 4 +- spec/acceptance/traits_spec.rb | 32 ++++++++------ spec/factory_bot/attribute_list_spec.rb | 15 +++++-- spec/factory_bot/registry_spec.rb | 3 +- 8 files changed, 59 insertions(+), 59 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index c8e9fbc..700d2bb 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,47 +1,26 @@ inherit_from: - https://raw.githubusercontent.com/thoughtbot/guides/master/style/ruby/.rubocop.yml + AllCops: TargetRubyVersion: 2.3 Exclude: - 'gemfiles/*' - 'tmp/**/*' -# TODO: -# This configuration was generated by -# `rubocop --auto-gen-config` -# on 2018-09-18 22:44:09 -0400 using RuboCop version 0.54.0. -# 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: CountComments, ExcludedMethods. Metrics/BlockLength: - Max: 26 - -# Offense count: 196 -# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. -# URISchemes: http, https -Metrics/LineLength: - Max: 189 - -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, ProceduralMethods, FunctionalMethods, IgnoredMethods. -# SupportedStyles: line_count_based, semantic, braces_for_chaining -# ProceduralMethods: benchmark, bm, bmbm, create, each_with_object, measure, new, realtime, tap, with_object -# FunctionalMethods: let, let!, subject, watch -# IgnoredMethods: lambda, proc, it -Style/BlockDelimiters: + CountComments: true + Max: 25 + ExcludedMethods: [] Exclude: - - 'spec/**/*' + - "spec/**/*" + - '*.gemspec' + +Style/SymbolArray: + EnforcedStyle: brackets -# Configuration parameters: . -# SupportedStyles: annotated, template, unannotated Style/FormatStringToken: - EnforcedStyle: unannotated + Enabled: false -# Offense count: 4 Style/MethodMissing: Exclude: - 'lib/factory_bot/decorator.rb' @@ -49,8 +28,8 @@ Style/MethodMissing: - 'lib/factory_bot/definition_proxy.rb' - 'lib/factory_bot/evaluator.rb' -# Cop supports --auto-correct. -# Configuration parameters: MinSize. -# SupportedStyles: percent, brackets -Style/SymbolArray: - EnforcedStyle: brackets +# TODO: gradually bring this down to 80 as we fix files +# Let's not open a big PR to fix all of these at once - +# we can fix gradually if we happen to be editing a file that has a violation +Metrics/LineLength: + Max: 142 diff --git a/lib/factory_bot/attribute_assigner.rb b/lib/factory_bot/attribute_assigner.rb index 5d86762..e96a64e 100644 --- a/lib/factory_bot/attribute_assigner.rb +++ b/lib/factory_bot/attribute_assigner.rb @@ -90,7 +90,9 @@ module FactoryBot def alias_names_to_ignore @attribute_list.non_ignored.flat_map do |attribute| - override_names.map { |override| attribute.name if attribute.alias_for?(override) && attribute.name != override && !ignored_attribute_names.include?(override) } + override_names.map do |override| + attribute.name if attribute.alias_for?(override) && attribute.name != override && !ignored_attribute_names.include?(override) + end end.compact end end diff --git a/spec/acceptance/build_stubbed_spec.rb b/spec/acceptance/build_stubbed_spec.rb index 8dcac32..3baabe8 100644 --- a/spec/acceptance/build_stubbed_spec.rb +++ b/spec/acceptance/build_stubbed_spec.rb @@ -176,7 +176,8 @@ describe "defaulting `created_at`" do end it "doesn't allow setting created_at on an object that doesn't define it" do - expect { build_stubbed(:thing_without_timestamp, created_at: Time.now) }.to raise_error(NoMethodError, /created_at=/) + expect { build_stubbed(:thing_without_timestamp, created_at: Time.now) }. + to raise_error(NoMethodError, /created_at=/) end end diff --git a/spec/acceptance/modify_factories_spec.rb b/spec/acceptance/modify_factories_spec.rb index 1e34534..25f7e45 100644 --- a/spec/acceptance/modify_factories_spec.rb +++ b/spec/acceptance/modify_factories_spec.rb @@ -173,10 +173,12 @@ describe "modifying factories" do end it "raises an exception if the factory was not defined before" do - expect { + modify_unknown_factory = -> do FactoryBot.modify do factory :unknown_factory end - }.to raise_error(ArgumentError) + end + + expect(modify_unknown_factory).to raise_error(ArgumentError) end end diff --git a/spec/acceptance/nested_attributes_spec.rb b/spec/acceptance/nested_attributes_spec.rb index 3fe3cef..6692c74 100644 --- a/spec/acceptance/nested_attributes_spec.rb +++ b/spec/acceptance/nested_attributes_spec.rb @@ -25,6 +25,8 @@ describe "association assignment from nested attributes" do end it "assigns the correct amount of comments when overridden" do - expect(FactoryBot.create(:post, comments_attributes: [FactoryBot.attributes_for(:comment)]).comments.count).to eq 1 + post = FactoryBot.create(:post, comments_attributes: [FactoryBot.attributes_for(:comment)]) + + expect(post.comments.count).to eq 1 end end diff --git a/spec/acceptance/traits_spec.rb b/spec/acceptance/traits_spec.rb index f7703d0..ad53f95 100644 --- a/spec/acceptance/traits_spec.rb +++ b/spec/acceptance/traits_spec.rb @@ -147,16 +147,16 @@ describe "an instance generated by a factory with multiple traits" do context "factory created with alternate syntax for specifying trait" do subject { FactoryBot.create(:male_user) } its(:gender) { should eq "Male" } - end - context "factory created with alternate syntax where trait name and attribute are the same" do - subject { FactoryBot.create(:great_user) } - its(:great) { should eq "GREAT!!!" } - end + context "where trait name and attribute are the same" do + subject { FactoryBot.create(:great_user) } + its(:great) { should eq "GREAT!!!" } + end - context "factory created with alternate syntax where trait name and attribute are the same and attribute is overridden" do - subject { FactoryBot.create(:great_user, great: "SORT OF!!!") } - its(:great) { should eq "SORT OF!!!" } + context "where trait name and attribute are the same and attribute is overridden" do + subject { FactoryBot.create(:great_user, great: "SORT OF!!!") } + its(:great) { should eq "SORT OF!!!" } + end end context "child factory created where trait attributes are inherited" do @@ -390,7 +390,9 @@ describe "inline traits overriding existing attributes" do end it "prefers overridden attributes over attributes from traits, inline traits, or attributes on factories" do - expect(FactoryBot.build(:extended_declined_user, :accepted, status: "completely overridden").status).to eq "completely overridden" + user = FactoryBot.build(:extended_declined_user, :accepted, status: "completely overridden") + + expect(user.status).to eq "completely overridden" end end @@ -481,8 +483,10 @@ describe "traits with to_create" do end end - expect(FactoryBot.create(:sub_user_with_trait_and_override, :overridden).name).to eq "completely overridden" - expect(FactoryBot.create(:child_user_with_trait_and_override, :overridden).name).to eq "completely overridden" + sub_user = FactoryBot.create(:sub_user_with_trait_and_override, :overridden) + child_user = FactoryBot.create(:child_user_with_trait_and_override, :overridden) + expect(sub_user.name).to eq "completely overridden" + expect(child_user.name).to eq "completely overridden" end end @@ -555,8 +559,10 @@ describe "traits with initialize_with" do end end - expect(FactoryBot.build(:sub_user_with_trait_and_override, :overridden).name).to eq "completely overridden" - expect(FactoryBot.build(:child_user_with_trait_and_override, :overridden).name).to eq "completely overridden" + sub_user = FactoryBot.build(:sub_user_with_trait_and_override, :overridden) + child_user = FactoryBot.build(:child_user_with_trait_and_override, :overridden) + expect(sub_user.name).to eq "completely overridden" + expect(child_user.name).to eq "completely overridden" end end diff --git a/spec/factory_bot/attribute_list_spec.rb b/spec/factory_bot/attribute_list_spec.rb index 2450898..f151010 100644 --- a/spec/factory_bot/attribute_list_spec.rb +++ b/spec/factory_bot/attribute_list_spec.rb @@ -19,9 +19,9 @@ describe FactoryBot::AttributeList, "#define_attribute" do it "raises if an attribute has already been defined" do attribute = double(:attribute, name: :attribute_name) - expect { + expect do 2.times { subject.define_attribute(attribute) } - }.to raise_error( + end.to raise_error( FactoryBot::AttributeDefinitionError, "Attribute already defined: attribute_name", ) @@ -35,7 +35,12 @@ describe FactoryBot::AttributeList, "#define_attribute with a named attribute li let(:association_with_different_name) { FactoryBot::Attribute::Association.new(:author, :post, {}) } it "raises when the attribute is a self-referencing association" do - expect { subject.define_attribute(association_with_same_name) }.to raise_error(FactoryBot::AssociationDefinitionError, "Self-referencing association 'author' in 'author'") + expect do + subject.define_attribute(association_with_same_name) + end.to raise_error( + FactoryBot::AssociationDefinitionError, + "Self-referencing association 'author' in 'author'", + ) end it "does not raise when the attribute is not a self-referencing association" do @@ -62,7 +67,9 @@ describe FactoryBot::AttributeList, "#apply_attributes" do end describe FactoryBot::AttributeList, "#associations" do - let(:email_attribute) { FactoryBot::Attribute::Dynamic.new(:email, false, ->(u) { "#{u.full_name}@example.com" }) } + let(:email_attribute) do + FactoryBot::Attribute::Dynamic.new(:email, false, ->(u) { "#{u.full_name}@example.com" }) + end let(:author_attribute) { FactoryBot::Attribute::Association.new(:author, :user, {}) } let(:profile_attribute) { FactoryBot::Attribute::Association.new(:profile, :profile, {}) } diff --git a/spec/factory_bot/registry_spec.rb b/spec/factory_bot/registry_spec.rb index 1d64436..df79dda 100644 --- a/spec/factory_bot/registry_spec.rb +++ b/spec/factory_bot/registry_spec.rb @@ -17,7 +17,8 @@ describe FactoryBot::Registry do end it "raises when an object cannot be found" do - expect { subject.find(:object_name) }.to raise_error(ArgumentError, "Great thing not registered: object_name") + expect { subject.find(:object_name) }. + to raise_error(ArgumentError, "Great thing not registered: object_name") end it "adds and returns the object registered" do