From 01f8e7cc9772296782b23455b90399b774f74f33 Mon Sep 17 00:00:00 2001
From: Jon Atack <jonnyatack@gmail.com>
Date: Fri, 25 Sep 2015 16:09:15 +0200
Subject: [PATCH] Standardize the test syntax to simplify for contributors and

for searching in the code:

- Uniform syntax in the current code avoids contributors wondering what
syntax to use. Fewer questions encourage contributions :)

- Using local vars called `s` instead of `search` makes searching in
the tests more productive because only test names use the word 'search'.

- Newer Rubyists generally know and use Ruby 1.9+ hash syntax and
Ransack stopped supporting Ruby 1.8 since version 1.5.0 (Ruby 1.8 users
can still use pre-1.5.0 versions of Ransack).

Yes, this adds code churn and changes the git history, but I reckon the
reasons above justify it.
---
 .../adapters/active_record/base_spec.rb       | 162 +++++++++---------
 .../adapters/active_record/context_spec.rb    |  34 ++--
 2 files changed, 97 insertions(+), 99 deletions(-)

diff --git a/spec/ransack/adapters/active_record/base_spec.rb b/spec/ransack/adapters/active_record/base_spec.rb
index 34cf492..46401e1 100644
--- a/spec/ransack/adapters/active_record/base_spec.rb
+++ b/spec/ransack/adapters/active_record/base_spec.rb
@@ -20,45 +20,45 @@ module Ransack
 
           context 'with scopes' do
             before do
-              Person.stub :ransackable_scopes => [:active, :over_age, :of_age]
+              Person.stub ransackable_scopes: [:active, :over_age, :of_age]
             end
 
-            it "applies true scopes" do
+            it 'applies true scopes' do
               s = Person.ransack('active' => true)
               expect(s.result.to_sql).to (include 'active = 1')
             end
 
-            it "applies stringy true scopes" do
+            it 'applies stringy true scopes' do
               s = Person.ransack('active' => 'true')
               expect(s.result.to_sql).to (include 'active = 1')
             end
 
-            it "applies stringy boolean scopes with true value in an array" do
+            it 'applies stringy boolean scopes with true value in an array' do
               s = Person.ransack('of_age' => ['true'])
               expect(s.result.to_sql).to (include 'age >= 18')
             end
 
-            it "applies stringy boolean scopes with false value in an array" do
+            it 'applies stringy boolean scopes with false value in an array' do
               s = Person.ransack('of_age' => ['false'])
               expect(s.result.to_sql).to (include 'age < 18')
             end
 
-            it "ignores unlisted scopes" do
+            it 'ignores unlisted scopes' do
               s = Person.ransack('restricted' => true)
               expect(s.result.to_sql).to_not (include 'restricted')
             end
 
-            it "ignores false scopes" do
+            it 'ignores false scopes' do
               s = Person.ransack('active' => false)
               expect(s.result.to_sql).not_to (include 'active')
             end
 
-            it "ignores stringy false scopes" do
+            it 'ignores stringy false scopes' do
               s = Person.ransack('active' => 'false')
               expect(s.result.to_sql).to_not (include 'active')
             end
 
-            it "passes values to scopes" do
+            it 'passes values to scopes' do
               s = Person.ransack('over_age' => 18)
               expect(s.result.to_sql).to (include 'age > 18')
             end
@@ -77,7 +77,7 @@ module Ransack
             #   expect(s.result.to_sql).to (include 'age > 0')
             # end
 
-            it "chains scopes" do
+            it 'chains scopes' do
               s = Person.ransack('over_age' => 18, 'active' => true)
               expect(s.result.to_sql).to (include 'age > 18')
               expect(s.result.to_sql).to (include 'active = 1')
@@ -89,7 +89,7 @@ module Ransack
           end
 
           it 'does not modify the parameters' do
-            params = { :name_eq => '' }
+            params = { name_eq: '' }
             expect { Person.ransack(params) }.not_to change { params }
           end
         end
@@ -98,7 +98,7 @@ module Ransack
           # For infix tests
           def self.sane_adapter?
             case ::ActiveRecord::Base.connection.adapter_name
-            when "SQLite3", "PostgreSQL"
+            when 'SQLite3', 'PostgreSQL'
               true
             else
               false
@@ -116,77 +116,77 @@ module Ransack
           # end
 
           it 'creates ransack attributes' do
-            s = Person.ransack(:reversed_name_eq => 'htimS cirA')
+            s = Person.ransack(reversed_name_eq: 'htimS cirA')
             expect(s.result.size).to eq(1)
 
             expect(s.result.first).to eq Person.where(name: 'Aric Smith').first
           end
 
           it 'can be accessed through associations' do
-            s = Person.ransack(:children_reversed_name_eq => 'htimS cirA')
+            s = Person.ransack(children_reversed_name_eq: 'htimS cirA')
             expect(s.result.to_sql).to match(
               /#{quote_table_name("children_people")}.#{
                  quote_column_name("name")} = 'Aric Smith'/
             )
           end
 
-          it 'allows an "attribute" to be an InfixOperation' do
-            s = Person.ransack(:doubled_name_eq => 'Aric SmithAric Smith')
+          it 'allows an attribute to be an InfixOperation' do
+            s = Person.ransack(doubled_name_eq: 'Aric SmithAric Smith')
             expect(s.result.first).to eq Person.where(name: 'Aric Smith').first
           end if defined?(Arel::Nodes::InfixOperation) && sane_adapter?
 
-          it "doesn't break #count if using InfixOperations" do
-            s = Person.ransack(:doubled_name_eq => 'Aric SmithAric Smith')
+          it 'does not break #count if using InfixOperations' do
+            s = Person.ransack(doubled_name_eq: 'Aric SmithAric Smith')
             expect(s.result.count).to eq 1
           end if defined?(Arel::Nodes::InfixOperation) && sane_adapter?
 
-          it "should remove empty key value pairs from the params hash" do
-            s = Person.ransack(:children_reversed_name_eq => '')
+          it 'should remove empty key value pairs from the params hash' do
+            s = Person.ransack(children_reversed_name_eq: '')
             expect(s.result.to_sql).not_to match /LEFT OUTER JOIN/
           end
 
-          it "should keep proper key value pairs in the params hash" do
-            s = Person.ransack(:children_reversed_name_eq => 'Testing')
+          it 'should keep proper key value pairs in the params hash' do
+            s = Person.ransack(children_reversed_name_eq: 'Testing')
             expect(s.result.to_sql).to match /LEFT OUTER JOIN/
           end
 
-          it "should function correctly when nil is passed in" do
+          it 'should function correctly when nil is passed in' do
             s = Person.ransack(nil)
           end
 
-          it "should function correctly when a blank string is passed in" do
+          it 'should function correctly when a blank string is passed in' do
             s = Person.ransack('')
           end
 
-          it "should function correctly with a multi-parameter attribute" do
+          it 'should function correctly with a multi-parameter attribute' do
             ::ActiveRecord::Base.default_timezone = :utc
             Time.zone = 'UTC'
 
             date = Date.current
             s = Person.ransack(
-              { "created_at_gteq(1i)" => date.year,
-                "created_at_gteq(2i)" => date.month,
-                "created_at_gteq(3i)" => date.day
+              { 'created_at_gteq(1i)' => date.year,
+                'created_at_gteq(2i)' => date.month,
+                'created_at_gteq(3i)' => date.day
               }
             )
             expect(s.result.to_sql).to match />=/
             expect(s.result.to_sql).to match date.to_s
           end
 
-          it "should function correctly when using fields with dots in them" do
-            s = Person.ransack(:email_cont => "example.com")
+          it 'should function correctly when using fields with dots in them' do
+            s = Person.ransack(email_cont: 'example.com')
             expect(s.result.exists?).to be true
           end
 
-          it "should function correctly when using fields with % in them" do
-            p = Person.create!(:name => "110%-er")
-            s = Person.ransack(:name_cont => "10%")
+          it 'should function correctly when using fields with % in them' do
+            p = Person.create!(name: '110%-er')
+            s = Person.ransack(name_cont: '10%')
             expect(s.result.to_a).to eq [p]
           end
 
-          it "should function correctly when using fields with backslashes in them" do
-            p = Person.create!(:name => "\\WINNER\\")
-            s = Person.ransack(:name_cont => "\\WINNER\\")
+          it 'should function correctly when using fields with backslashes in them' do
+            p = Person.create!(name: "\\WINNER\\")
+            s = Person.ransack(name_cont: "\\WINNER\\")
             expect(s.result.to_a).to eq [p]
           end
 
@@ -194,33 +194,33 @@ module Ransack
             # when escaping is supported right in LIKE expression without adding extra expressions
             def self.simple_escaping?
               case ::ActiveRecord::Base.connection.adapter_name
-                when "Mysql2", "PostgreSQL"
+                when 'Mysql2', 'PostgreSQL'
                   true
                 else
                   false
               end
             end
 
-            it "should search correctly if matches exist" do
-              p = Person.create!(:name => "name_with_underscore")
-              s = Person.ransack(:name_cont => "name_")
+            it 'should search correctly if matches exist' do
+              p = Person.create!(name: 'name_with_underscore')
+              s = Person.ransack(name_cont: 'name_')
               expect(s.result.to_a).to eq [p]
             end if simple_escaping?
 
-            it "should return empty result if no matches" do
-              Person.create!(:name => "name_with_underscore")
-              s = Person.ransack(:name_cont => "n_")
+            it 'should return empty result if no matches' do
+              Person.create!(name: 'name_with_underscore')
+              s = Person.ransack(name_cont: 'n_')
               expect(s.result.to_a).to eq []
             end if simple_escaping?
           end
 
-          context "searching on an `in` predicate with a ransacker" do
-            it "should function correctly when passing an array of ids" do
+          context 'searching on an `in` predicate with a ransacker' do
+            it 'should function correctly when passing an array of ids' do
               s = Person.ransack(array_users_in: true)
               expect(s.result.count).to be > 0
             end
 
-            it "should function correctly when passing an array of strings" do
+            it 'should function correctly when passing an array of strings' do
               Person.create!(name: Person.first.id.to_s)
               s = Person.ransack(array_names_in: true)
               expect(s.result.count).to be > 0
@@ -234,57 +234,57 @@ module Ransack
             end
           end
 
-          context "search on an `in` predicate with an array" do
-            it "should function correctly when passing an array of ids" do
+          context 'search on an `in` predicate with an array' do
+            it 'should function correctly when passing an array of ids' do
               array = Person.all.map(&:id)
               s = Person.ransack(id_in: array)
               expect(s.result.count).to eq array.size
             end
           end
 
-          it "should function correctly when an attribute name ends with '_start'" do
-            p = Person.create!(:new_start => 'Bar and foo', :name => 'Xiang')
+          it 'should work correctly when an attribute name ends with _start' do
+            p = Person.create!(new_start: 'Bar and foo', name: 'Xiang')
 
-            s = Person.ransack(:new_start_end => ' and foo')
+            s = Person.ransack(new_start_end: ' and foo')
             expect(s.result.to_a).to eq [p]
 
-            s = Person.ransack(:name_or_new_start_start => 'Xia')
+            s = Person.ransack(name_or_new_start_start: 'Xia')
             expect(s.result.to_a).to eq [p]
 
-            s = Person.ransack(:new_start_or_name_end => 'iang')
+            s = Person.ransack(new_start_or_name_end: 'iang')
             expect(s.result.to_a).to eq [p]
           end
 
-          it "should function correctly when an attribute name ends with '_end'" do
-            p = Person.create!(:stop_end => 'Foo and bar', :name => 'Marianne')
+          it 'should work correctly when an attribute name ends with _end' do
+            p = Person.create!(stop_end: 'Foo and bar', name: 'Marianne')
 
-            s = Person.ransack(:stop_end_start => 'Foo and')
+            s = Person.ransack(stop_end_start: 'Foo and')
             expect(s.result.to_a).to eq [p]
 
-            s = Person.ransack(:stop_end_or_name_end => 'anne')
+            s = Person.ransack(stop_end_or_name_end: 'anne')
             expect(s.result.to_a).to eq [p]
 
-            s = Person.ransack(:name_or_stop_end_end => ' bar')
+            s = Person.ransack(name_or_stop_end_end: ' bar')
             expect(s.result.to_a).to eq [p]
           end
 
-          it "should function correctly when an attribute name has 'and' in it" do
-            p = Person.create!(:terms_and_conditions => true)
-            s = Person.ransack(:terms_and_conditions_eq => true)
+          it 'should work correctly when an attribute name has `and` in it' do
+            p = Person.create!(terms_and_conditions: true)
+            s = Person.ransack(terms_and_conditions_eq: true)
             expect(s.result.to_a).to eq [p]
           end
 
-          it "should translate attribute aliased column names",
-          :if => Ransack::SUPPORTS_ATTRIBUTE_ALIAS do
-            s = Person.ransack(:full_name_eq => 'Nicolas Cage')
+          it 'should translate attribute aliased column names',
+          if: Ransack::SUPPORTS_ATTRIBUTE_ALIAS do
+            s = Person.ransack(full_name_eq: 'Nicolas Cage')
             expect(s.result.to_sql).to match(
               /WHERE #{quote_table_name("people")}.#{quote_column_name("name")}/
             )
           end
 
-          it 'allows sort by "only_sort" field' do
+          it 'allows sort by `only_sort` field' do
             s = Person.ransack(
-              "s" => { "0" => { "dir" => "asc", "name" => "only_sort" } }
+              's' => { '0' => { 'dir' => 'asc', 'name' => 'only_sort' } }
             )
             expect(s.result.to_sql).to match(
               /ORDER BY #{quote_table_name("people")}.#{
@@ -292,9 +292,9 @@ module Ransack
             )
           end
 
-          it "doesn't sort by 'only_search' field" do
+          it 'does not sort by `only_search` field' do
             s = Person.ransack(
-              "s" => { "0" => { "dir" => "asc", "name" => "only_search" } }
+              's' => { '0' => { 'dir' => 'asc', 'name' => 'only_search' } }
             )
             expect(s.result.to_sql).not_to match(
               /ORDER BY #{quote_table_name("people")}.#{
@@ -302,25 +302,25 @@ module Ransack
             )
           end
 
-          it 'allows search by "only_search" field' do
-            s = Person.ransack(:only_search_eq => 'htimS cirA')
+          it 'allows search by `only_search` field' do
+            s = Person.ransack(only_search_eq: 'htimS cirA')
             expect(s.result.to_sql).to match(
               /WHERE #{quote_table_name("people")}.#{
                 quote_column_name("only_search")} = 'htimS cirA'/
             )
           end
 
-          it "can't be searched by 'only_sort'" do
-            s = Person.ransack(:only_sort_eq => 'htimS cirA')
+          it 'cannot be searched by `only_sort`' do
+            s = Person.ransack(only_sort_eq: 'htimS cirA')
             expect(s.result.to_sql).not_to match(
               /WHERE #{quote_table_name("people")}.#{
                 quote_column_name("only_sort")} = 'htimS cirA'/
             )
           end
 
-          it 'allows sort by "only_admin" field, if auth_object: :admin' do
+          it 'allows sort by `only_admin` field, if auth_object: :admin' do
             s = Person.ransack(
-              { "s" => { "0" => { "dir" => "asc", "name" => "only_admin" } } },
+              { 's' => { '0' => { 'dir' => 'asc', 'name' => 'only_admin' } } },
               { auth_object: :admin }
             )
             expect(s.result.to_sql).to match(
@@ -329,9 +329,9 @@ module Ransack
             )
           end
 
-          it "doesn't sort by 'only_admin' field, if auth_object: nil" do
+          it 'does not sort by `only_admin` field, if auth_object: nil' do
             s = Person.ransack(
-              "s" => { "0" => { "dir" => "asc", "name" => "only_admin" } }
+              's' => { '0' => { 'dir' => 'asc', 'name' => 'only_admin' } }
             )
             expect(s.result.to_sql).not_to match(
               /ORDER BY #{quote_table_name("people")}.#{
@@ -339,10 +339,10 @@ module Ransack
             )
           end
 
-          it 'allows search by "only_admin" field, if auth_object: :admin' do
+          it 'allows search by `only_admin` field, if auth_object: :admin' do
             s = Person.ransack(
-              { :only_admin_eq => 'htimS cirA' },
-              { :auth_object => :admin }
+              { only_admin_eq: 'htimS cirA' },
+              { auth_object: :admin }
             )
             expect(s.result.to_sql).to match(
               /WHERE #{quote_table_name("people")}.#{
@@ -350,8 +350,8 @@ module Ransack
             )
           end
 
-          it "can't be searched by 'only_admin', if auth_object: nil" do
-            s = Person.ransack(:only_admin_eq => 'htimS cirA')
+          it 'cannot be searched by `only_admin`, if auth_object: nil' do
+            s = Person.ransack(only_admin_eq: 'htimS cirA')
             expect(s.result.to_sql).not_to match(
               /WHERE #{quote_table_name("people")}.#{
                 quote_column_name("only_admin")} = 'htimS cirA'/
diff --git a/spec/ransack/adapters/active_record/context_spec.rb b/spec/ransack/adapters/active_record/context_spec.rb
index cb21bc5..c25128d 100644
--- a/spec/ransack/adapters/active_record/context_spec.rb
+++ b/spec/ransack/adapters/active_record/context_spec.rb
@@ -9,11 +9,11 @@ module Ransack
       describe Context do
         subject { Context.new(Person) }
 
-        if AR_version >= '3.1'
-          it 'has an Active Record alias tracker method' do
-            expect(subject.alias_tracker)
-            .to be_an ::ActiveRecord::Associations::AliasTracker
-          end
+
+        it 'has an Active Record alias tracker method',
+        if: AR_version >= '3.1' do
+          expect(subject.alias_tracker)
+          .to be_an ::ActiveRecord::Associations::AliasTracker
         end
 
         describe '#relation_for' do
@@ -24,8 +24,8 @@ module Ransack
 
         describe '#evaluate' do
           it 'evaluates search objects' do
-            search = Search.new(Person, :name_eq => 'Joe Blow')
-            result = subject.evaluate(search)
+            s = Search.new(Person, name_eq: 'Joe Blow')
+            result = subject.evaluate(s)
 
             expect(result).to be_an ::ActiveRecord::Relation
             expect(result.to_sql)
@@ -33,25 +33,25 @@ module Ransack
           end
 
           it 'SELECTs DISTINCT when distinct: true' do
-            search = Search.new(Person, :name_eq => 'Joe Blow')
-            result = subject.evaluate(search, :distinct => true)
+            s = Search.new(Person, name_eq: 'Joe Blow')
+            result = subject.evaluate(s, distinct: true)
 
             expect(result).to be_an ::ActiveRecord::Relation
             expect(result.to_sql).to match /SELECT DISTINCT/
           end
         end
 
-        describe "sharing context across searches" do
+        describe 'sharing context across searches' do
           let(:shared_context) { Context.for(Person) }
 
           before do
-            Search.new(Person, { :parent_name_eq => 'A' },
+            Search.new(Person, { parent_name_eq: 'A' },
               context: shared_context)
-            Search.new(Person, { :children_name_eq => 'B' },
+            Search.new(Person, { children_name_eq: 'B' },
               context: shared_context)
           end
 
-          describe '#join_associations', :if => AR_version <= '4.0' do
+          describe '#join_associations', if: AR_version <= '4.0' do
             it 'returns dependent join associations for all searches run
                 against the context' do
               parents, children = shared_context.join_associations
@@ -69,18 +69,16 @@ module Ransack
           end
 
           describe '#join_sources' do
-            # FIXME: fix this test for Rails 4.2.
+            # FIXME: fix this test for Rails 4.2 and 5.0.
             it 'returns dependent arel join nodes for all searches run against
-            the context',
-            :if => %w(3.1 3.2 4.0 4.1).include?(AR_version) do
+            the context', if: %w(3.1 3.2 4.0 4.1).include?(AR_version) do
               parents, children = shared_context.join_sources
-
               expect(children.left.name).to eq "children_people"
               expect(parents.left.name).to eq "parents_people"
             end
 
             it 'can be rejoined to execute a valid query',
-            :if => AR_version >= '3.1' do
+            if: AR_version >= '3.1' do
               parents, children = shared_context.join_sources
 
               expect { Person.joins(parents).joins(children).to_a }