From e5c502b1cc58fa7829dca0e74946f948e9793e1b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sat, 28 Sep 2019 01:35:31 +0200
Subject: [PATCH 01/10] Split tests checking permissions to vote in a poll

The test was hard to follow, and splitting the test in three it's easier
to read and doesn't create unused variables anymore. On the minus side,
now there's one extra request during the tests.
---
 spec/features/polls/polls_spec.rb | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/spec/features/polls/polls_spec.rb b/spec/features/polls/polls_spec.rb
index e24ebc692..df60a0499 100644
--- a/spec/features/polls/polls_spec.rb
+++ b/spec/features/polls/polls_spec.rb
@@ -84,18 +84,25 @@ describe "Polls" do
 
       expect(page).to have_css(".unverified", count: 3)
       expect(page).to have_content("You must verify your account to participate")
+    end
 
-      poll_district = create(:poll, geozone_restricted: true)
-      verified = create(:user, :level_two)
-      login_as(verified)
+    scenario "Geozone poll" do
+      create(:poll, geozone_restricted: true)
 
+      login_as(create(:user, :level_two))
       visit polls_path
 
       expect(page).to have_css(".cant-answer", count: 1)
       expect(page).to have_content("This poll is not available on your geozone")
+    end
 
+    scenario "Already participated in a poll", :js do
       poll_with_question = create(:poll)
       question = create(:poll_question, :yes_no, poll: poll_with_question)
+
+      login_as(create(:user, :level_two))
+      visit polls_path
+
       vote_for_poll_via_web(poll_with_question, question, "Yes")
 
       visit polls_path

From 5a84dcb534bcddce5bc49aee2e54aadcdc52ab5f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sat, 28 Sep 2019 13:24:29 +0200
Subject: [PATCH 02/10] Rename `unfinished` trait to `open`

So now it's more consistent with the name we use in the rest of the
code.
---
 spec/factories/budgets.rb                      | 2 +-
 spec/features/admin/budget_investments_spec.rb | 2 +-
 spec/features/budgets/investments_spec.rb      | 2 +-
 spec/models/budget/investment_spec.rb          | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/spec/factories/budgets.rb b/spec/factories/budgets.rb
index b1942cf08..177a55b43 100644
--- a/spec/factories/budgets.rb
+++ b/spec/factories/budgets.rb
@@ -121,7 +121,7 @@ FactoryBot.define do
       valuation_finished { true }
     end
 
-    trait :unfinished do
+    trait :open do
       valuation_finished { false }
     end
 
diff --git a/spec/features/admin/budget_investments_spec.rb b/spec/features/admin/budget_investments_spec.rb
index 5195c6c7e..165d991e9 100644
--- a/spec/features/admin/budget_investments_spec.rb
+++ b/spec/features/admin/budget_investments_spec.rb
@@ -334,7 +334,7 @@ describe "Admin budget investments" do
       valuator = create(:valuator, user: user)
       create(:budget_investment,
         :with_administrator,
-        :unfinished,
+        :open,
         title: "Investment without valuation",
         budget: budget,
         valuators: [valuator])
diff --git a/spec/features/budgets/investments_spec.rb b/spec/features/budgets/investments_spec.rb
index 458ce0f10..551150c51 100644
--- a/spec/features/budgets/investments_spec.rb
+++ b/spec/features/budgets/investments_spec.rb
@@ -1259,7 +1259,7 @@ describe "Budget Investments" do
   scenario "Show (unfeasible budget investment with valuation not finished)" do
     investment = create(:budget_investment,
                         :unfeasible,
-                        :unfinished,
+                        :open,
                         budget: budget,
                         heading: heading,
                         unfeasibility_explanation: "Local government is not competent in this matter")
diff --git a/spec/models/budget/investment_spec.rb b/spec/models/budget/investment_spec.rb
index 960acb9bc..5c00c8a5e 100644
--- a/spec/models/budget/investment_spec.rb
+++ b/spec/models/budget/investment_spec.rb
@@ -1152,7 +1152,7 @@ describe Budget::Investment do
     describe "with under_valuation filter" do
       let(:params) { { advanced_filters: ["under_valuation"], budget_id: budget.id } }
       it "returns only investment under valuation" do
-        investment1 = create(:budget_investment, :with_administrator, :unfinished, :with_valuator,
+        investment1 = create(:budget_investment, :with_administrator, :open, :with_valuator,
                              budget: budget)
         create(:budget_investment, :with_administrator, budget: budget)
         create(:budget_investment, budget: budget)

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 03/10] 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

From d09be11a08209d5ad68d4c46f2b4df7c96a13338 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sat, 28 Sep 2019 14:17:24 +0200
Subject: [PATCH 04/10] Make test for feed limit more explicit

The limit parameter wasn't specified in the test but in the default
value in the database, making the test hard to read.

Since now we've moved the other processes to separate tests, now we can
create four processes using `times` and keep the test simple.
---
 spec/models/widget/feed_spec.rb | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/spec/models/widget/feed_spec.rb b/spec/models/widget/feed_spec.rb
index a0bc3faa5..5500c7fde 100644
--- a/spec/models/widget/feed_spec.rb
+++ b/spec/models/widget/feed_spec.rb
@@ -57,15 +57,12 @@ describe Widget::Feed do
     end
 
     describe "#processes" do
-      let(:feed) {  build(:widget_feed, kind: "processes") }
+      let(:feed) {  build(:widget_feed, kind: "processes", limit: 3) }
 
-      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")
+      it "returns a maximum number of open published processes given by the limit" do
+        4.times { create(:legislation_process, :open, :published) }
 
-        expect(feed.processes).to eq([open_process4, open_process3, open_process2])
+        expect(feed.processes.count).to be 3
       end
 
       it "does not return past processes" do

From 802be297739312872291c88ea33cb3529cd02e4a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 25 Sep 2019 23:23:34 +0200
Subject: [PATCH 05/10] Simplify maximum limit of direct messages specs

Now the tests are easier to understand: when the limit is 3, if you
create 3, the fourth one is invalid. If you create 2, the third one is
valid.
---
 spec/models/direct_message_spec.rb | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/spec/models/direct_message_spec.rb b/spec/models/direct_message_spec.rb
index cacb976da..5dda929e2 100644
--- a/spec/models/direct_message_spec.rb
+++ b/spec/models/direct_message_spec.rb
@@ -36,9 +36,7 @@ describe DirectMessage do
 
       it "is not valid if above maximum" do
         sender = create(:user)
-        direct_message1 = create(:direct_message, sender: sender)
-        direct_message2 = create(:direct_message, sender: sender)
-        direct_message3 = create(:direct_message, sender: sender)
+        3.times { create(:direct_message, sender: sender) }
 
         direct_message4 = build(:direct_message, sender: sender)
         expect(direct_message4).not_to be_valid
@@ -46,8 +44,7 @@ describe DirectMessage do
 
       it "is valid if below maximum" do
         sender = create(:user)
-        direct_message1 = create(:direct_message, sender: sender)
-        direct_message2 = create(:direct_message, sender: sender)
+        2.times { create(:direct_message, sender: sender) }
 
         direct_message3 = build(:direct_message, sender: sender)
         expect(direct_message3).to be_valid

From 9f64129be51d45d79099875c594de006d872e020 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Fri, 27 Sep 2019 03:00:58 +0200
Subject: [PATCH 06/10] Remove isolated useless assignments

These variables can be considered a block, and so removing them doesn't
make the test much harder to undestand.

Sometimes these variables formed the setup, sometimes they formed an
isolated part of the setup, and sometimes they were the part of the test
that made the test different from other tests.
---
 .../admin/api/stats_controller_spec.rb        |  6 ++--
 .../admin/poll/booth_assigments_spec.rb       |  9 ++---
 spec/features/admin/poll/shifts_spec.rb       | 10 +++---
 spec/features/budgets/investments_spec.rb     |  2 +-
 spec/features/communities_spec.rb             |  7 ++--
 spec/features/debates_spec.rb                 |  8 ++---
 spec/features/officing/voters_spec.rb         |  4 +--
 spec/lib/graphql_spec.rb                      | 12 +++----
 spec/models/budget/ballot_spec.rb             |  6 ++--
 spec/models/budget/investment_spec.rb         | 36 ++++++++++---------
 spec/models/community_spec.rb                 |  2 +-
 spec/models/poll/voter_spec.rb                | 18 +++++-----
 12 files changed, 63 insertions(+), 57 deletions(-)

diff --git a/spec/controllers/admin/api/stats_controller_spec.rb b/spec/controllers/admin/api/stats_controller_spec.rb
index cd95fe7a6..f45d5e59d 100644
--- a/spec/controllers/admin/api/stats_controller_spec.rb
+++ b/spec/controllers/admin/api/stats_controller_spec.rb
@@ -64,9 +64,9 @@ describe Admin::Api::StatsController do
         time_1 = Time.zone.local(2017, 04, 01)
         time_2 = Time.zone.local(2017, 04, 02)
 
-        budget_investment1 = create(:budget_investment, created_at: time_1)
-        budget_investment2 = create(:budget_investment, created_at: time_2)
-        budget_investment3 = create(:budget_investment, created_at: time_2)
+        create(:budget_investment, created_at: time_1)
+        create(:budget_investment, created_at: time_2)
+        create(:budget_investment, created_at: time_2)
 
         sign_in user
         get :show, params: { budget_investments: true }
diff --git a/spec/features/admin/poll/booth_assigments_spec.rb b/spec/features/admin/poll/booth_assigments_spec.rb
index b70e84da2..18e19186b 100644
--- a/spec/features/admin/poll/booth_assigments_spec.rb
+++ b/spec/features/admin/poll/booth_assigments_spec.rb
@@ -175,14 +175,15 @@ describe "Admin booths assignments" do
       poll = create(:poll, starts_at: 2.weeks.ago, ends_at: 1.week.ago)
       booth = create(:poll_booth)
       booth_assignment = create(:poll_booth_assignment, poll: poll, booth: booth)
-      officer_assignment_1 = create(:poll_officer_assignment, booth_assignment: booth_assignment, date: poll.starts_at)
-      officer_assignment_2 = create(:poll_officer_assignment, booth_assignment: booth_assignment, date: poll.ends_at)
-      final_officer_assignment = create(:poll_officer_assignment, :final, booth_assignment: booth_assignment, date: poll.ends_at)
+
+      create(:poll_officer_assignment, booth_assignment: booth_assignment, date: poll.starts_at)
+      create(:poll_officer_assignment, booth_assignment: booth_assignment, date: poll.ends_at)
+      create(:poll_officer_assignment, :final, booth_assignment: booth_assignment, date: poll.ends_at)
 
       create(:poll_voter, poll: poll, booth_assignment: booth_assignment, created_at: poll.starts_at.to_date)
       create(:poll_voter, poll: poll, booth_assignment: booth_assignment, created_at: poll.ends_at.to_date)
 
-      booth_assignment_2 = create(:poll_booth_assignment, poll: poll)
+      create(:poll_booth_assignment, poll: poll)
 
       visit admin_poll_path(poll)
       click_link "Booths (2)"
diff --git a/spec/features/admin/poll/shifts_spec.rb b/spec/features/admin/poll/shifts_spec.rb
index 9b226b839..84ddada0d 100644
--- a/spec/features/admin/poll/shifts_spec.rb
+++ b/spec/features/admin/poll/shifts_spec.rb
@@ -13,8 +13,8 @@ describe "Admin shifts" do
     booth1 = create(:poll_booth)
     booth2 = create(:poll_booth)
 
-    shift1 = create(:poll_shift, officer: officer, booth: booth1, date: Date.current)
-    shift2 = create(:poll_shift, officer: officer, booth: booth2, date: Time.zone.tomorrow)
+    create(:poll_shift, officer: officer, booth: booth1, date: Date.current)
+    create(:poll_shift, officer: officer, booth: booth2, date: Time.zone.tomorrow)
 
     visit new_admin_booth_shift_path(booth1)
 
@@ -99,8 +99,8 @@ describe "Admin shifts" do
     booth = create(:poll_booth, polls: [poll])
     officer = create(:poll_officer)
 
-    shift1 = create(:poll_shift, :vote_collection_task, officer: officer, booth: booth, date: Date.current)
-    shift2 = create(:poll_shift, :recount_scrutiny_task, officer: officer, booth: booth, date: Time.zone.tomorrow)
+    create(:poll_shift, :vote_collection_task, officer: officer, booth: booth, date: Date.current)
+    create(:poll_shift, :recount_scrutiny_task, officer: officer, booth: booth, date: Time.zone.tomorrow)
 
     vote_collection_dates = (Date.current..poll.ends_at.to_date).to_a
                                                                 .reject { |date| date == Date.current }
@@ -243,7 +243,7 @@ describe "Admin shifts" do
     booth = create(:poll_booth)
     officer = create(:poll_officer)
 
-    shift = create(:poll_shift, officer: officer, booth: booth)
+    create(:poll_shift, officer: officer, booth: booth)
     officer.destroy
 
     visit new_admin_booth_shift_path(booth)
diff --git a/spec/features/budgets/investments_spec.rb b/spec/features/budgets/investments_spec.rb
index 551150c51..b327d02c0 100644
--- a/spec/features/budgets/investments_spec.rb
+++ b/spec/features/budgets/investments_spec.rb
@@ -1377,8 +1377,8 @@ describe "Budget Investments" do
 
         group2 = create(:budget_group, budget: budget)
         another_heading1 = create(:budget_heading, group: group2)
-        another_heading2 = create(:budget_heading, group: group2)
 
+        create(:budget_heading, group: group2)
         create(:budget_investment, heading: heading, title: "Investment", voters: [author])
         create(:budget_investment, heading: another_heading1, title: "Another investment")
 
diff --git a/spec/features/communities_spec.rb b/spec/features/communities_spec.rb
index 5e67056d6..0ac369f8b 100644
--- a/spec/features/communities_spec.rb
+++ b/spec/features/communities_spec.rb
@@ -51,9 +51,10 @@ describe "Communities" do
       topic1 = create(:topic, community: community)
       topic2 = create(:topic, community: community)
       topic3 = create(:topic, community: community)
-      topic1_comment = create(:comment, commentable: topic1)
-      topic3_comment = create(:comment, commentable: topic3)
-      topic3_comment = create(:comment, commentable: topic3)
+
+      create(:comment, :with_confidence_score, commentable: topic1)
+      create(:comment, :with_confidence_score, commentable: topic3)
+      create(:comment, :with_confidence_score, commentable: topic3)
 
       visit community_path(community, order: :most_commented)
 
diff --git a/spec/features/debates_spec.rb b/spec/features/debates_spec.rb
index 43ccafc59..3d16ee1d1 100644
--- a/spec/features/debates_spec.rb
+++ b/spec/features/debates_spec.rb
@@ -957,10 +957,10 @@ describe "Debates" do
       proposal = create(:proposal, tag_list: "Sport")
       user = create(:user, recommended_debates: true, followables: [proposal])
 
-      debate1 = create(:debate, title: "Show you got",      cached_votes_total: 10,  tag_list: "Sport")
-      debate2 = create(:debate, title: "Show what you got", cached_votes_total: 1,   tag_list: "Sport")
-      debate3 = create(:debate, title: "Do not display with same tag", cached_votes_total: 100, tag_list: "Sport")
-      debate4 = create(:debate, title: "Do not display",    cached_votes_total: 1)
+      create(:debate, title: "Show you got",      cached_votes_total: 10,  tag_list: "Sport")
+      create(:debate, title: "Show what you got", cached_votes_total: 1,   tag_list: "Sport")
+      create(:debate, title: "Do not display with same tag", cached_votes_total: 100, tag_list: "Sport")
+      create(:debate, title: "Do not display",    cached_votes_total: 1)
 
       login_as(user)
       visit debates_path
diff --git a/spec/features/officing/voters_spec.rb b/spec/features/officing/voters_spec.rb
index 9c57a9d9c..167cb8443 100644
--- a/spec/features/officing/voters_spec.rb
+++ b/spec/features/officing/voters_spec.rb
@@ -140,8 +140,8 @@ describe "Voters" do
       booth1 = create(:poll_booth)
       booth2 = create(:poll_booth)
 
-      officer_assignment1 = create(:poll_officer_assignment, officer: officer, poll: poll1, booth: booth1)
-      officer_assignment2 = create(:poll_officer_assignment, officer: officer, poll: poll2, booth: booth2)
+      create(:poll_officer_assignment, officer: officer, poll: poll1, booth: booth1)
+      create(:poll_officer_assignment, officer: officer, poll: poll2, booth: booth2)
 
       set_officing_booth(booth1)
       visit new_officing_residence_path
diff --git a/spec/lib/graphql_spec.rb b/spec/lib/graphql_spec.rb
index 3b9523caa..66dec9825 100644
--- a/spec/lib/graphql_spec.rb
+++ b/spec/lib/graphql_spec.rb
@@ -176,9 +176,9 @@ describe "Consul Schema" do
     end
 
     it "only retruns tags with kind nil or category" do
-      tag          = create(:tag, name: "Parks")
-      category_tag = create(:tag, :category, name: "Health")
-      admin_tag    = create(:tag, name: "Admin tag", kind: "admin")
+      create(:tag, name: "Parks")
+      create(:tag, :category, name: "Health")
+      create(:tag, name: "Admin tag", kind: "admin")
 
       proposal = create(:proposal, tag_list: "Parks, Health, Admin tag")
 
@@ -244,9 +244,9 @@ describe "Consul Schema" do
     end
 
     it "only retruns tags with kind nil or category" do
-      tag          = create(:tag, name: "Parks")
-      category_tag = create(:tag, :category, name: "Health")
-      admin_tag    = create(:tag, name: "Admin tag", kind: "admin")
+      create(:tag, name: "Parks")
+      create(:tag, :category, name: "Health")
+      create(:tag, name: "Admin tag", kind: "admin")
 
       debate = create(:debate, tag_list: "Parks, Health, Admin tag")
 
diff --git a/spec/models/budget/ballot_spec.rb b/spec/models/budget/ballot_spec.rb
index bcfcc6c28..9d2f3e547 100644
--- a/spec/models/budget/ballot_spec.rb
+++ b/spec/models/budget/ballot_spec.rb
@@ -115,12 +115,10 @@ describe Budget::Ballot do
     it "returns nil if there are no headings with balloted investments in a group" do
       budget = create(:budget)
       group = create(:budget_group, budget: budget)
-
-      heading1 = create(:budget_heading, group: group)
-      heading2 = create(:budget_heading, group: group)
-
       ballot = create(:budget_ballot, budget: budget)
 
+      2.times { create(:budget_heading, group: group) }
+
       expect(ballot.heading_for_group(group)).to eq nil
     end
 
diff --git a/spec/models/budget/investment_spec.rb b/spec/models/budget/investment_spec.rb
index d1b1acf6d..145689bba 100644
--- a/spec/models/budget/investment_spec.rb
+++ b/spec/models/budget/investment_spec.rb
@@ -822,20 +822,21 @@ describe Budget::Investment do
         carabanchel = create(:budget_heading, group: group)
         salamanca   = create(:budget_heading, group: group)
 
-        carabanchel_investment = create(:budget_investment, heading: carabanchel, voters: [user])
-        salamanca_investment   = create(:budget_investment, heading: salamanca)
+        create(:budget_investment, heading: carabanchel, voters: [user])
+
+        salamanca_investment = create(:budget_investment, heading: salamanca)
 
         expect(salamanca_investment.valid_heading?(user)).to eq(false)
       end
 
       it "accepts votes in multiple headings of the same group" do
         group.update(max_votable_headings: 2)
-
         carabanchel = create(:budget_heading, group: group)
         salamanca   = create(:budget_heading, group: group)
 
-        carabanchel_investment = create(:budget_investment, heading: carabanchel, voters: [user])
-        salamanca_investment   = create(:budget_investment, heading: salamanca)
+        create(:budget_investment, heading: carabanchel, voters: [user])
+
+        salamanca_investment = create(:budget_investment, heading: salamanca)
 
         expect(salamanca_investment.valid_heading?(user)).to eq(true)
       end
@@ -855,35 +856,38 @@ describe Budget::Investment do
 
       it "allows votes in a group with a single heading" do
         all_city_investment = create(:budget_investment, heading: heading)
+
         expect(all_city_investment.valid_heading?(user)).to eq(true)
       end
 
       it "allows votes in a group with a single heading after voting in that heading" do
-        all_city_investment1 = create(:budget_investment, heading: heading, voters: [user])
-        all_city_investment2 = create(:budget_investment, heading: heading)
+        create(:budget_investment, heading: heading, voters: [user])
 
-        expect(all_city_investment2.valid_heading?(user)).to eq(true)
+        investment_for_same_heading = create(:budget_investment, heading: heading)
+
+        expect(investment_for_same_heading.valid_heading?(user)).to eq(true)
       end
 
       it "allows votes in a group with a single heading after voting in another group" do
         districts = create(:budget_group, budget: budget)
         carabanchel = create(:budget_heading, group: districts)
 
-        all_city_investment    = create(:budget_investment, heading: heading)
-        carabanchel_investment = create(:budget_investment, heading: carabanchel, voters: [user])
+        create(:budget_investment, heading: carabanchel, voters: [user])
 
-        expect(all_city_investment.valid_heading?(user)).to eq(true)
+        investment_from_different_group = create(:budget_investment, heading: heading)
+
+        expect(investment_from_different_group.valid_heading?(user)).to eq(true)
       end
 
       it "allows votes in a group with multiple headings after voting in group with a single heading" do
         districts = create(:budget_group, budget: budget)
-        carabanchel = create(:budget_heading, group: districts)
-        salamanca   = create(:budget_heading, group: districts)
+        2.times { create(:budget_heading, group: districts) }
 
-        all_city_investment    = create(:budget_investment, heading: heading, voters: [user])
-        carabanchel_investment = create(:budget_investment, heading: carabanchel)
+        create(:budget_investment, heading: heading, voters: [user])
 
-        expect(carabanchel_investment.valid_heading?(user)).to eq(true)
+        investment = create(:budget_investment, heading: districts.headings.sample)
+
+        expect(investment.valid_heading?(user)).to eq(true)
       end
 
       describe "#can_vote_in_another_heading?" do
diff --git a/spec/models/community_spec.rb b/spec/models/community_spec.rb
index b17c4d449..90ac3279f 100644
--- a/spec/models/community_spec.rb
+++ b/spec/models/community_spec.rb
@@ -19,7 +19,7 @@ RSpec.describe Community, type: :model do
       topic1 = create(:topic, community: community, author: user1)
       create(:comment, commentable: topic1, author: user1)
       create(:comment, commentable: topic1, author: user2)
-      topic2 = create(:topic, community: community, author: user2)
+      create(:topic, community: community, author: user2)
 
       expect(community.participants).to match_array [user1, user2, proposal.author]
     end
diff --git a/spec/models/poll/voter_spec.rb b/spec/models/poll/voter_spec.rb
index 017ec76e8..88cdc02ce 100644
--- a/spec/models/poll/voter_spec.rb
+++ b/spec/models/poll/voter_spec.rb
@@ -29,19 +29,21 @@ describe Poll::Voter do
     end
 
     it "is not valid if the user has already voted in the same poll or booth_assignment" do
-      voter1 = create(:poll_voter, user: user, poll: poll)
-      voter2 = build(:poll_voter, user: user, poll: poll)
+      create(:poll_voter, user: user, poll: poll)
 
-      expect(voter2).not_to be_valid
-      expect(voter2.errors.messages[:document_number]).to eq(["User has already voted"])
+      voter = build(:poll_voter, user: user, poll: poll)
+
+      expect(voter).not_to be_valid
+      expect(voter.errors.messages[:document_number]).to eq(["User has already voted"])
     end
 
     it "is not valid if the user has already voted in the same poll/booth" do
-      voter1 = create(:poll_voter, user: user, poll: poll, booth_assignment: booth_assignment)
-      voter2 = build(:poll_voter, user: user, poll: poll, booth_assignment: booth_assignment)
+      create(:poll_voter, user: user, poll: poll, booth_assignment: booth_assignment)
 
-      expect(voter2).not_to be_valid
-      expect(voter2.errors.messages[:document_number]).to eq(["User has already voted"])
+      voter = build(:poll_voter, user: user, poll: poll, booth_assignment: booth_assignment)
+
+      expect(voter).not_to be_valid
+      expect(voter.errors.messages[:document_number]).to eq(["User has already voted"])
     end
 
     it "is not valid if the user has already voted in different booth in the same poll" do

From d0d1c9972c60d33189c15776af5dafd7407c52ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sat, 28 Sep 2019 01:11:42 +0200
Subject: [PATCH 07/10] Remove unused variables following their parent

There's a very common pattern in our test, where the setup only has two
lines:

variable = create(:something)
unused_variable = create(:something_else, something: variable)

In this case, since there's a blank line below these ones and then we'll
get to the body of the test, and the second variable is going to be
created based on the first variable, we can remove the useless
assignment and the readability is still OK.

Another option we almost unanimously discarded was:

variable = create(:something)
_unused_variable = create(:something_else, something: variable)

We don't use it anywhere else, either.

One more option we considered but found a bit too much for simple tests:

variable = create(:something) do |something|
  create(:something_else, something: variable)
end

Then of course we could move the setup to `let` and `before` blocks, but
the tests could get over-structured really quickly.
---
 spec/features/admin/legislation/draft_versions_spec.rb |  2 +-
 .../admin/poll/questions/answers/answers_spec.rb       |  2 +-
 spec/features/budgets/executions_spec.rb               |  3 +--
 spec/features/dashboard/dashboard_spec.rb              |  2 +-
 spec/features/notifications_spec.rb                    |  2 +-
 spec/features/officing/voters_spec.rb                  | 10 +++++-----
 6 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/spec/features/admin/legislation/draft_versions_spec.rb b/spec/features/admin/legislation/draft_versions_spec.rb
index 06808979f..6880dc927 100644
--- a/spec/features/admin/legislation/draft_versions_spec.rb
+++ b/spec/features/admin/legislation/draft_versions_spec.rb
@@ -69,7 +69,7 @@ describe "Admin legislation draft versions" do
   context "Update" do
     scenario "Valid legislation draft version", :js do
       process = create(:legislation_process, title: "An example legislation process")
-      draft_version = create(:legislation_draft_version, title: "Version 1", process: process)
+      create(:legislation_draft_version, title: "Version 1", process: process)
 
       visit admin_root_path
 
diff --git a/spec/features/admin/poll/questions/answers/answers_spec.rb b/spec/features/admin/poll/questions/answers/answers_spec.rb
index a2d0c78bc..96212cd48 100644
--- a/spec/features/admin/poll/questions/answers/answers_spec.rb
+++ b/spec/features/admin/poll/questions/answers/answers_spec.rb
@@ -24,7 +24,7 @@ describe "Answers" do
 
   scenario "Create second answer and place after the first one" do
     question = create(:poll_question)
-    answer = create(:poll_question_answer, title: "First", question: question, given_order: 1)
+    create(:poll_question_answer, title: "First", question: question, given_order: 1)
 
     visit admin_question_path(question)
     click_link "Add answer"
diff --git a/spec/features/budgets/executions_spec.rb b/spec/features/budgets/executions_spec.rb
index a6896885b..c370a43cf 100644
--- a/spec/features/budgets/executions_spec.rb
+++ b/spec/features/budgets/executions_spec.rb
@@ -291,8 +291,7 @@ describe "Executions" do
 
     scenario "Milestone not yet published" do
       status = create(:milestone_status)
-      unpublished_milestone = create(:milestone, milestoneable: investment1,
-                                     status: status, publication_date: Date.tomorrow)
+      create(:milestone, milestoneable: investment1, status: status, publication_date: Date.tomorrow)
 
       visit budget_executions_path(budget, status: status.id)
 
diff --git a/spec/features/dashboard/dashboard_spec.rb b/spec/features/dashboard/dashboard_spec.rb
index 46952266b..6eee343ab 100644
--- a/spec/features/dashboard/dashboard_spec.rb
+++ b/spec/features/dashboard/dashboard_spec.rb
@@ -133,7 +133,7 @@ describe "Proposal's dashboard" do
 
   scenario "Dashboard progress can unexecute proposed action" do
     action = create(:dashboard_action, :proposed_action, :active)
-    executed_action = create(:dashboard_executed_action, proposal: proposal, action: action)
+    create(:dashboard_executed_action, proposal: proposal, action: action)
 
     visit progress_proposal_dashboard_path(proposal)
     expect(page).to have_content(action.title)
diff --git a/spec/features/notifications_spec.rb b/spec/features/notifications_spec.rb
index f9d60ffb3..4dd3a891f 100644
--- a/spec/features/notifications_spec.rb
+++ b/spec/features/notifications_spec.rb
@@ -39,7 +39,7 @@ describe "Notifications" do
 
   scenario "View single notification" do
     proposal = create(:proposal)
-    notification = create(:notification, user: user, notifiable: proposal)
+    create(:notification, user: user, notifiable: proposal)
 
     click_notifications_icon
 
diff --git a/spec/features/officing/voters_spec.rb b/spec/features/officing/voters_spec.rb
index 167cb8443..e2dc73e1e 100644
--- a/spec/features/officing/voters_spec.rb
+++ b/spec/features/officing/voters_spec.rb
@@ -37,7 +37,7 @@ describe "Voters" do
 
   scenario "Cannot vote" do
     unvotable_poll = create(:poll, :current, geozone_restricted: true, geozones: [create(:geozone, census_code: "02")])
-    officer_assignment = create(:poll_officer_assignment, officer: officer, poll: unvotable_poll, booth: booth)
+    create(:poll_officer_assignment, officer: officer, poll: unvotable_poll, booth: booth)
 
     set_officing_booth(booth)
     visit new_officing_residence_path
@@ -87,7 +87,7 @@ describe "Voters" do
 
     scenario "Display current polls assigned to a booth" do
       poll = create(:poll, :current)
-      officer_assignment = create(:poll_officer_assignment, officer: officer, poll: poll, booth: booth)
+      create(:poll_officer_assignment, officer: officer, poll: poll, booth: booth)
 
       set_officing_booth(booth)
       visit new_officing_residence_path
@@ -99,7 +99,7 @@ describe "Voters" do
 
     scenario "Display polls that the user can vote" do
       votable_poll = create(:poll, :current, geozone_restricted: true, geozones: [Geozone.first])
-      officer_assignment = create(:poll_officer_assignment, officer: officer, poll: votable_poll, booth: booth)
+      create(:poll_officer_assignment, officer: officer, poll: votable_poll, booth: booth)
 
       set_officing_booth(booth)
       visit new_officing_residence_path
@@ -111,7 +111,7 @@ describe "Voters" do
 
     scenario "Display polls that the user cannot vote" do
       unvotable_poll = create(:poll, :current, geozone_restricted: true, geozones: [create(:geozone, census_code: "02")])
-      officer_assignment = create(:poll_officer_assignment, officer: officer, poll: unvotable_poll, booth: booth)
+      create(:poll_officer_assignment, officer: officer, poll: unvotable_poll, booth: booth)
 
       set_officing_booth(booth)
       visit new_officing_residence_path
@@ -123,7 +123,7 @@ describe "Voters" do
 
     scenario "Do not display expired polls" do
       expired_poll = create(:poll, :expired)
-      officer_assignment = create(:poll_officer_assignment, officer: officer, poll: expired_poll, booth: booth)
+      create(:poll_officer_assignment, officer: officer, poll: expired_poll, booth: booth)
 
       set_officing_booth(booth)
       visit new_officing_residence_path

From 4c5104d03d9ba5f323c870e8f08b8789f01244e9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Thu, 26 Sep 2019 00:19:55 +0200
Subject: [PATCH 08/10] Avoid unused variables in notification spec

We can change the code a bit so the useless assignment is either part of
the setup (where only another variable was present) or isolated in the
"action" part of the test.
---
 spec/models/notification_spec.rb          |  4 +++-
 spec/models/proposal_notification_spec.rb | 10 ++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb
index bd527d5d4..15c05837a 100644
--- a/spec/models/notification_spec.rb
+++ b/spec/models/notification_spec.rb
@@ -105,6 +105,7 @@ describe Notification do
       user = create(:user)
       comment1 = create(:comment)
       comment2 = create(:comment)
+
       create(:notification, user: user, notifiable: comment1)
 
       expect(Notification.existent(user, comment2)).to eq(nil)
@@ -114,7 +115,8 @@ describe Notification do
       user1 = create(:user)
       user2 = create(:user)
       comment = create(:comment)
-      notification = create(:notification, user: user1, notifiable: comment)
+
+      create(:notification, user: user1, notifiable: comment)
 
       expect(Notification.existent(user2, comment)).to eq(nil)
     end
diff --git a/spec/models/proposal_notification_spec.rb b/spec/models/proposal_notification_spec.rb
index 1aa42b918..48bee9d69 100644
--- a/spec/models/proposal_notification_spec.rb
+++ b/spec/models/proposal_notification_spec.rb
@@ -50,21 +50,19 @@ describe ProposalNotification do
 
     it "is not valid if below minimum interval" do
       proposal = create(:proposal)
+      create(:proposal_notification, proposal: proposal)
 
-      notification1 = create(:proposal_notification, proposal: proposal)
-      notification2 = build(:proposal_notification, proposal: proposal)
+      notification2 = build(:proposal_notification, proposal: proposal.reload)
 
-      proposal.reload
       expect(notification2).not_to be_valid
     end
 
     it "is valid if notifications above minimum interval" do
       proposal = create(:proposal)
+      create(:proposal_notification, proposal: proposal, created_at: 4.days.ago)
 
-      notification1 = create(:proposal_notification, proposal: proposal, created_at: 4.days.ago)
-      notification2 = build(:proposal_notification, proposal: proposal)
+      notification2 = build(:proposal_notification, proposal: proposal.reload)
 
-      proposal.reload
       expect(notification2).to be_valid
     end
 

From f5fe8c1279e739fbeef685e39c12a176cb0b728b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Fri, 27 Sep 2019 20:41:47 +0200
Subject: [PATCH 09/10] Use factory bot blocks to create complex data

We had four headings, some of them had investments, and some of them
didn't, and it was very hard to scan the code and check which investment
belongs to which heading.

Grouping the investments inside the block creating the heading makes
that task much easier, even if the code is still not 100% readable.

We also avoid unused variables which were there to keep the code
vertically algined.
---
 spec/features/budgets/ballots_spec.rb | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/spec/features/budgets/ballots_spec.rb b/spec/features/budgets/ballots_spec.rb
index c9207e1f7..9d8e088ef 100644
--- a/spec/features/budgets/ballots_spec.rb
+++ b/spec/features/budgets/ballots_spec.rb
@@ -94,16 +94,21 @@ describe "Ballots" do
       end
 
       scenario "Investments" do
-        city_heading1     = create(:budget_heading, group: city,      name: "Above the city")
-        city_heading2     = create(:budget_heading, group: city,      name: "Under the city")
-        district_heading1 = create(:budget_heading, group: districts, name: "District 1")
-        district_heading2 = create(:budget_heading, group: districts, name: "District 2")
+        create(:budget_heading, group: city, name: "Under the city")
 
-        create(:budget_investment, :selected, heading: city_heading1, title: "Solar panels")
-        create(:budget_investment, :selected, heading: city_heading1, title: "Observatory")
-        create(:budget_investment, :selected, heading: district_heading1, title: "New park")
-        create(:budget_investment, :selected, heading: district_heading1, title: "Zero-emission zone")
-        create(:budget_investment, :selected, heading: district_heading2, title: "Climbing wall")
+        create(:budget_heading, group: city, name: "Above the city") do |heading|
+          create(:budget_investment, :selected, heading: heading, title: "Solar panels")
+          create(:budget_investment, :selected, heading: heading, title: "Observatory")
+        end
+
+        create(:budget_heading, group: districts, name: "District 1") do |heading|
+          create(:budget_investment, :selected, heading: heading, title: "New park")
+          create(:budget_investment, :selected, heading: heading, title: "Zero-emission zone")
+        end
+
+        create(:budget_heading, group: districts, name: "District 2") do |heading|
+          create(:budget_investment, :selected, heading: heading, title: "Climbing wall")
+        end
 
         visit budget_path(budget)
         click_link "City"

From 77835474190582bb40082bc9f2dec54efb151022 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sun, 29 Sep 2019 17:37:08 +0200
Subject: [PATCH 10/10] Remove rest of unused variables

I haven't found an elegant way to remove them, but since they were the
only three variables left out of 383 we used to have, I can live with
this low percentage of inelegant solutions.
---
 .../features/admin/poll/questions/answers/answers_spec.rb | 2 +-
 spec/features/tags/debates_spec.rb                        | 8 ++++----
 spec/features/tags/proposals_spec.rb                      | 8 ++++----
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/spec/features/admin/poll/questions/answers/answers_spec.rb b/spec/features/admin/poll/questions/answers/answers_spec.rb
index 96212cd48..4ed637f38 100644
--- a/spec/features/admin/poll/questions/answers/answers_spec.rb
+++ b/spec/features/admin/poll/questions/answers/answers_spec.rb
@@ -40,7 +40,7 @@ describe "Answers" do
   scenario "Update" do
     question = create(:poll_question)
     answer = create(:poll_question_answer, question: question, title: "Answer title", given_order: 2)
-    answer2 = create(:poll_question_answer, question: question, title: "Another title", given_order: 1)
+    create(:poll_question_answer, question: question, title: "Another title", given_order: 1)
 
     visit admin_answer_path(answer)
 
diff --git a/spec/features/tags/debates_spec.rb b/spec/features/tags/debates_spec.rb
index 2cc2b86ff..d54fde94a 100644
--- a/spec/features/tags/debates_spec.rb
+++ b/spec/features/tags/debates_spec.rb
@@ -168,16 +168,16 @@ describe "Tags" do
     end
 
     scenario "From show" do
-      debate1 = create(:debate, tag_list: "Education")
-      debate2 = create(:debate, tag_list: "Health")
+      debate = create(:debate, tag_list: "Education")
+      create(:debate, tag_list: "Health")
 
-      visit debate_path(debate1)
+      visit debate_path(debate)
 
       click_link "Education"
 
       within("#debates") do
         expect(page).to have_css(".debate", count: 1)
-        expect(page).to have_content(debate1.title)
+        expect(page).to have_content(debate.title)
       end
     end
 
diff --git a/spec/features/tags/proposals_spec.rb b/spec/features/tags/proposals_spec.rb
index 348d198f4..fdf7bb813 100644
--- a/spec/features/tags/proposals_spec.rb
+++ b/spec/features/tags/proposals_spec.rb
@@ -202,16 +202,16 @@ describe "Tags" do
     end
 
     scenario "From show" do
-      proposal1 = create(:proposal, tag_list: "Education")
-      proposal2 = create(:proposal, tag_list: "Health")
+      proposal = create(:proposal, tag_list: "Education")
+      create(:proposal, tag_list: "Health")
 
-      visit proposal_path(proposal1)
+      visit proposal_path(proposal)
 
       click_link "Education"
 
       within("#proposals") do
         expect(page).to have_css(".proposal", count: 1)
-        expect(page).to have_content(proposal1.title)
+        expect(page).to have_content(proposal.title)
       end
     end