From d410fcbc0e6f33c2ba23aba2023e3e717cedaecd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sat, 28 Sep 2019 13:46:17 +0200
Subject: [PATCH] Split scope tests

In the scenario where we want to test scopes and use `match_array`, we
usually declare variables we never use, which raises a warning in the
Ruby interpreter (since the main cause for an unused variable is a
typo).

So I've decided to just split the tests into cases where every record is
returned and cases were no records are returned, just like we do in
other places.

There are several other options we've considered:

1. Don't declare unused variables, but declare the ones we use
2. Prefix unused variables with un underscore
3. Declare just one variable being an array containing all elements, and
access the elements using Array#[]
4. Don't declare any variables, and compare results against attributes
such as titles

None of these options was met with enthusiasm.
---
 spec/models/budget/heading_spec.rb    |  20 +-
 spec/models/budget/investment_spec.rb | 258 +++++++++++++++++---------
 spec/models/document_spec.rb          |  14 +-
 spec/models/poll/voter_spec.rb        |  24 ++-
 spec/models/widget/card_spec.rb       |  10 +-
 spec/models/widget/feed_spec.rb       |  16 +-
 6 files changed, 227 insertions(+), 115 deletions(-)

diff --git a/spec/models/budget/heading_spec.rb b/spec/models/budget/heading_spec.rb
index 1d28f1720..73d623e7d 100644
--- a/spec/models/budget/heading_spec.rb
+++ b/spec/models/budget/heading_spec.rb
@@ -308,17 +308,25 @@ describe Budget::Heading do
 
   describe "scope allow_custom_content" do
     it "returns headings with allow_custom_content order by name" do
-      excluded_heading = create(:budget_heading, name: "Name A")
-      last_heading     = create(:budget_heading, allow_custom_content: true, name: "Name C")
-      first_heading    = create(:budget_heading, allow_custom_content: true, name: "Name B")
+      last_heading     = create(:budget_heading, allow_custom_content: true, name: "Name B")
+      first_heading    = create(:budget_heading, allow_custom_content: true, name: "Name A")
 
       expect(Budget::Heading.allow_custom_content).to eq [first_heading, last_heading]
     end
 
-    it "returns headings with multiple translations only once" do
-      create(:budget_heading, allow_custom_content: true, name_en: "English", name_es: "Spanish")
+    it "does not return headings which don't allow custom content" do
+      create(:budget_heading, name: "Name A")
 
-      expect(Budget::Heading.allow_custom_content.count).to eq 1
+      expect(Budget::Heading.allow_custom_content).to be_empty
+    end
+
+    it "returns headings with multiple translations only once" do
+      translated_heading = create(:budget_heading,
+                                  allow_custom_content: true,
+                                  name_en: "English",
+                                  name_es: "Spanish")
+
+      expect(Budget::Heading.allow_custom_content).to eq [translated_heading]
     end
   end
 
diff --git a/spec/models/budget/investment_spec.rb b/spec/models/budget/investment_spec.rb
index 5c00c8a5e..d1b1acf6d 100644
--- a/spec/models/budget/investment_spec.rb
+++ b/spec/models/budget/investment_spec.rb
@@ -336,34 +336,53 @@ describe Budget::Investment do
   end
 
   describe "by_valuator" do
-    it "returns investments assigned to specific valuator" do
-      valuator1 = create(:valuator)
-      valuator2 = create(:valuator)
+    it "returns investments assigned to a valuator" do
+      alfred = create(:valuator)
+      batman = create(:valuator)
 
-      investment1 = create(:budget_investment, valuators: [valuator1])
-      investment2 = create(:budget_investment, valuators: [valuator2])
-      investment3 = create(:budget_investment, valuators: [valuator1, valuator2])
+      manor = create(:budget_investment, valuators: [alfred])
+      batcave = create(:budget_investment, valuators: [alfred, batman])
 
-      by_valuator = Budget::Investment.by_valuator(valuator1.id)
+      by_valuator = Budget::Investment.by_valuator(alfred)
 
-      expect(by_valuator).to match_array [investment1, investment3]
+      expect(by_valuator).to match_array [manor, batcave]
+    end
+
+    it "does not return investments assigned to a different valuator" do
+      jekyll = create(:valuator)
+      hyde = create(:valuator)
+
+      create(:budget_investment, valuators: [hyde])
+
+      by_valuator = Budget::Investment.by_valuator(jekyll)
+
+      expect(by_valuator).to be_empty
     end
   end
 
   describe "#by_valuator_group" do
+    let(:aquaman)        { create(:valuator) }
+    let(:justice_league) { create(:valuator_group, valuators: [aquaman]) }
 
     it "returns investments assigned to a valuator's group" do
-      valuator = create(:valuator)
-      valuator_group = create(:valuator_group, valuators: [valuator])
-      assigned_investment = create(:budget_investment, valuators: [valuator],
-                                                       valuator_groups: [valuator_group])
-      another_assigned_investment = create(:budget_investment, valuator_groups: [valuator_group])
-      unassigned_investment = create(:budget_investment, valuators: [valuator], valuator_groups: [])
-      create(:budget_investment, valuators: [valuator], valuator_groups: [create(:valuator_group)])
+      water_power = create(:budget_investment, valuator_groups: [justice_league], valuators: [aquaman])
+      solar_power = create(:budget_investment, valuator_groups: [justice_league])
 
-      by_valuator_group = Budget::Investment.by_valuator_group(valuator.valuator_group_id)
+      by_valuator_group = Budget::Investment.by_valuator_group(justice_league)
 
-      expect(by_valuator_group).to contain_exactly(assigned_investment, another_assigned_investment)
+      expect(by_valuator_group).to contain_exactly(solar_power, water_power)
+    end
+
+    it "does not return investments assigned to no groups" do
+      create(:budget_investment, valuators: [aquaman], valuator_groups: [])
+
+      expect(Budget::Investment.by_valuator_group(justice_league)).to be_empty
+    end
+
+    it "does not return investments assigned to a different group" do
+      create(:budget_investment, valuators: [aquaman], valuator_groups: [create(:valuator_group)])
+
+      expect(Budget::Investment.by_valuator_group(justice_league)).to be_empty
     end
   end
 
@@ -392,130 +411,199 @@ describe Budget::Investment do
 
   describe "scopes" do
     describe "valuation_open" do
-      it "returns all investments with false valuation_finished" do
-        investment1 = create(:budget_investment, :finished)
-        investment2 = create(:budget_investment)
+      it "returns investments with valuation open" do
+        investment = create(:budget_investment, valuation_finished: false)
 
-        valuation_open = Budget::Investment.valuation_open
-
-        expect(valuation_open).to eq [investment2]
-      end
-    end
-
-    describe "without_admin" do
-      it "returns all open investments without assigned admin" do
-        investment1 = create(:budget_investment, :finished)
-        investment2 = create(:budget_investment, :with_administrator)
-        investment3 = create(:budget_investment)
-
-        without_admin = Budget::Investment.without_admin
-
-        expect(without_admin).to eq [investment3]
-      end
-    end
-
-    describe "managed" do
-      it "returns all open investments with assigned admin but without assigned valuators" do
-        investment1 = create(:budget_investment, :with_administrator, :with_valuator)
-        investment2 = create(:budget_investment, :with_administrator, :finished)
-        investment3 = create(:budget_investment, :with_administrator)
-
-        managed = Budget::Investment.managed
-
-        expect(managed).to eq [investment3]
-      end
-    end
-
-    describe "valuating" do
-      it "returns all investments with assigned valuator but valuation not finished" do
-        investment1 = create(:budget_investment)
-        investment2 = create(:budget_investment, :with_valuator)
-        investment3 = create(:budget_investment, :with_valuator, :finished)
-
-        valuating = Budget::Investment.valuating
-
-        expect(valuating).to eq [investment2]
+        expect(Budget::Investment.valuation_open).to eq [investment]
       end
 
-      it "returns all investments with assigned valuator groups but valuation not finished" do
-        investment1 = create(:budget_investment)
-        investment2 = create(:budget_investment, valuator_groups: [create(:valuator_group)])
-        investment3 = create(:budget_investment, :finished, valuator_groups: [create(:valuator_group)])
+      it "does not return investments with valuation finished" do
+        create(:budget_investment, valuation_finished: true)
 
-        valuating = Budget::Investment.valuating
-
-        expect(valuating).to eq [investment2]
+        expect(Budget::Investment.valuation_open).to be_empty
       end
     end
 
     describe "valuation_finished" do
-      it "returns all investments with valuation finished" do
-        investment1 = create(:budget_investment)
-        investment2 = create(:budget_investment, :with_valuator)
-        investment3 = create(:budget_investment, :with_valuator, :finished)
+      it "returns investments with valuation finished" do
+        investment = create(:budget_investment, valuation_finished: true)
 
-        valuation_finished = Budget::Investment.valuation_finished
+        expect(Budget::Investment.valuation_finished).to eq [investment]
+      end
 
-        expect(valuation_finished).to eq [investment3]
+      it "does not return investments with valuation open" do
+        create(:budget_investment, valuation_finished: false)
+
+        expect(Budget::Investment.valuation_finished).to be_empty
+      end
+    end
+
+    describe "without_admin" do
+      it "returns open investments without assigned admin" do
+        investment = create(:budget_investment, :open, administrator: nil)
+
+        expect(Budget::Investment.without_admin).to eq [investment]
+      end
+
+      it "does not return investments with valuation finished" do
+        create(:budget_investment, :finished)
+
+        expect(Budget::Investment.without_admin).to be_empty
+      end
+
+      it "does not return investment with an admin" do
+        create(:budget_investment, :with_administrator)
+
+        expect(Budget::Investment.without_admin).to be_empty
+      end
+    end
+
+    describe "managed" do
+      it "returns open investments with assigned admin but without assigned valuators" do
+        investment = create(:budget_investment, :with_administrator)
+
+        expect(Budget::Investment.managed).to eq [investment]
+      end
+
+      it "does not return investments without assigned admin" do
+        create(:budget_investment, administrator: nil)
+
+        expect(Budget::Investment.managed).to be_empty
+      end
+
+      it "does not return investments with assigned valuator" do
+        create(:budget_investment, :with_administrator, :with_valuator)
+
+        expect(Budget::Investment.managed).to be_empty
+      end
+
+      it "does not return finished investments" do
+        create(:budget_investment, :with_administrator, :finished)
+
+        expect(Budget::Investment.managed).to be_empty
+      end
+    end
+
+    describe "valuating" do
+      it "returns investments with assigned valuator but valuation not finished" do
+        investment = create(:budget_investment, :open, :with_valuator)
+
+        expect(Budget::Investment.valuating).to eq [investment]
+      end
+
+      it "returns investments with assigned valuator groups but valuation not finished" do
+        investment = create(:budget_investment, :open, valuator_groups: [create(:valuator_group)])
+
+        expect(Budget::Investment.valuating).to eq [investment]
+      end
+
+      it "does not return investments with valuation finished" do
+        create(:budget_investment, :finished, :with_valuator)
+        create(:budget_investment, :finished, valuator_groups: [create(:valuator_group)])
+
+        expect(Budget::Investment.valuating).to be_empty
+      end
+
+      it "does not return investments without valuator nor valuator group" do
+        create(:budget_investment, :open)
+
+        expect(Budget::Investment.valuating).to be_empty
       end
     end
 
     describe "feasible" do
-      it "returns all feasible investments" do
+      it "returns feasible investments" do
         feasible_investment = create(:budget_investment, :feasible)
-        create(:budget_investment)
 
         expect(Budget::Investment.feasible).to eq [feasible_investment]
       end
+
+      it "does not return unfeasible nor undecided investments" do
+        create(:budget_investment, :undecided)
+        create(:budget_investment, :unfeasible)
+
+        expect(Budget::Investment.feasible).to be_empty
+      end
     end
 
     describe "unfeasible" do
-      it "returns all unfeasible investments" do
+      it "returns unfeasible investments" do
         unfeasible_investment = create(:budget_investment, :unfeasible)
-        create(:budget_investment, :feasible)
 
         expect(Budget::Investment.unfeasible).to eq [unfeasible_investment]
       end
+
+      it "does not return feasible nor undecided investments" do
+        create(:budget_investment, :feasible)
+        create(:budget_investment, :undecided)
+
+        expect(Budget::Investment.unfeasible).to be_empty
+      end
     end
 
     describe "not_unfeasible" do
-      it "returns all feasible and undecided investments" do
-        unfeasible_investment = create(:budget_investment, :unfeasible)
+      it "returns feasible and undecided investments" do
         undecided_investment = create(:budget_investment, :undecided)
         feasible_investment = create(:budget_investment, :feasible)
 
         expect(Budget::Investment.not_unfeasible).to match_array [undecided_investment, feasible_investment]
       end
+
+      it "does not return unfeasible investments" do
+        create(:budget_investment, :unfeasible)
+
+        expect(Budget::Investment.not_unfeasible).to be_empty
+      end
     end
 
     describe "undecided" do
-      it "returns all undecided investments" do
-        unfeasible_investment = create(:budget_investment, :unfeasible)
+      it "returns undecided investments" do
         undecided_investment = create(:budget_investment, :undecided)
-        feasible_investment = create(:budget_investment, :feasible)
 
         expect(Budget::Investment.undecided).to eq [undecided_investment]
       end
+
+      it "does not return feasible nor unfeasible investments" do
+        create(:budget_investment, :feasible)
+        create(:budget_investment, :unfeasible)
+
+        expect(Budget::Investment.undecided).to be_empty
+      end
     end
 
     describe "selected" do
-      it "returns all selected investments" do
+      it "returns selected investments" do
         selected_investment = create(:budget_investment, :selected)
-        unselected_investment = create(:budget_investment, :unselected)
 
         expect(Budget::Investment.selected).to eq [selected_investment]
       end
+
+      it "does not return unselected investments" do
+        create(:budget_investment, :unselected)
+
+        expect(Budget::Investment.selected).to be_empty
+      end
     end
 
     describe "unselected" do
       it "returns all unselected not_unfeasible investments" do
-        selected_investment = create(:budget_investment, :selected)
-        unselected_unfeasible_investment = create(:budget_investment, :unselected, :unfeasible)
         unselected_undecided_investment = create(:budget_investment, :unselected, :undecided)
         unselected_feasible_investment = create(:budget_investment, :unselected, :feasible)
 
         expect(Budget::Investment.unselected).to match_array [unselected_undecided_investment, unselected_feasible_investment]
       end
+
+      it "does not return selected investments" do
+        create(:budget_investment, :selected)
+
+        expect(Budget::Investment.unselected).to be_empty
+      end
+
+      it "does not return unfeasible investments" do
+        create(:budget_investment, :unselected, :unfeasible)
+
+        expect(Budget::Investment.unselected).to be_empty
+      end
     end
 
     describe "sort_by_title" do
diff --git a/spec/models/document_spec.rb b/spec/models/document_spec.rb
index 8caa59c7b..e4d601ecd 100644
--- a/spec/models/document_spec.rb
+++ b/spec/models/document_spec.rb
@@ -6,18 +6,18 @@ describe Document do
   it_behaves_like "document validations", "proposal_document"
 
   context "scopes" do
-
     describe "#admin" do
-
       it "returns admin documents" do
-        user_document = create(:document)
-        admin_document1 = create(:document, :admin)
-        admin_document2 = create(:document, :admin)
-        admin_document3 = create(:document, :admin)
+        admin_document = create(:document, :admin)
 
-        expect(Document.admin).to match_array [admin_document1, admin_document2, admin_document3]
+        expect(Document.admin).to eq [admin_document]
       end
 
+      it "does not return user documents" do
+        create(:document, admin: false)
+
+        expect(Document.admin).to be_empty
+      end
     end
   end
 end
diff --git a/spec/models/poll/voter_spec.rb b/spec/models/poll/voter_spec.rb
index 4c7e6dc62..017ec76e8 100644
--- a/spec/models/poll/voter_spec.rb
+++ b/spec/models/poll/voter_spec.rb
@@ -121,25 +121,29 @@ describe Poll::Voter do
 
     describe "#web" do
       it "returns voters with a web origin" do
-        voter1 = create(:poll_voter, :from_web)
-        voter2 = create(:poll_voter, :from_web)
-        voter3 = create(:poll_voter, :from_booth)
+        voter = create(:poll_voter, :from_web)
 
-        web_voters = Poll::Voter.web
+        expect(Poll::Voter.web).to eq [voter]
+      end
 
-        expect(web_voters).to match_array [voter1, voter2]
+      it "does not return voters with a booth origin" do
+        create(:poll_voter, :from_booth)
+
+        expect(Poll::Voter.web).to be_empty
       end
     end
 
     describe "#booth" do
       it "returns voters with a booth origin" do
-        voter1 = create(:poll_voter, :from_booth)
-        voter2 = create(:poll_voter, :from_booth)
-        voter3 = create(:poll_voter, :from_web)
+        voter = create(:poll_voter, :from_booth)
 
-        booth_voters = Poll::Voter.booth
+        expect(Poll::Voter.booth).to eq [voter]
+      end
 
-        expect(booth_voters).to match_array [voter1, voter2]
+      it "does not return voters with a web origin" do
+        create(:poll_voter, :from_web)
+
+        expect(Poll::Voter.booth).to be_empty
       end
     end
 
diff --git a/spec/models/widget/card_spec.rb b/spec/models/widget/card_spec.rb
index cd563df56..b9e81f450 100644
--- a/spec/models/widget/card_spec.rb
+++ b/spec/models/widget/card_spec.rb
@@ -14,13 +14,17 @@ describe Widget::Card do
   end
 
   describe "#header" do
-
-    it "returns the header card" do
+    it "returns header cards" do
       header = create(:widget_card, header: true)
-      card = create(:widget_card, header: false)
 
       expect(Widget::Card.header).to eq([header])
     end
+
+    it "does not return regular cards" do
+      create(:widget_card, header: false)
+
+      expect(Widget::Card.header).to be_empty
+    end
   end
 
   describe "#body" do
diff --git a/spec/models/widget/feed_spec.rb b/spec/models/widget/feed_spec.rb
index 2d8521f70..a0bc3faa5 100644
--- a/spec/models/widget/feed_spec.rb
+++ b/spec/models/widget/feed_spec.rb
@@ -57,20 +57,28 @@ describe Widget::Feed do
     end
 
     describe "#processes" do
+      let(:feed) {  build(:widget_feed, kind: "processes") }
 
       it "returns open and published processes" do
         open_process1 = create(:legislation_process, :open, :published, title: "Open process 1")
         open_process2 = create(:legislation_process, :open, :published, title: "Open process 2")
         open_process3 = create(:legislation_process, :open, :published, title: "Open process 3")
         open_process4 = create(:legislation_process, :open, :published, title: "Open process 4")
-        open_process5 = create(:legislation_process, :open, :not_published, title: "Open process 5")
-        past_process = create(:legislation_process, :past, title: "Past process")
-
-        feed = build(:widget_feed, kind: "processes")
 
         expect(feed.processes).to eq([open_process4, open_process3, open_process2])
       end
 
+      it "does not return past processes" do
+        create(:legislation_process, :past)
+
+        expect(feed.processes).to be_empty
+      end
+
+      it "does not return unpublished processes" do
+        create(:legislation_process, :open, :not_published)
+
+        expect(feed.processes).to be_empty
+      end
     end
 
   end