From 02b8bc6f69c3ceabe534a1b865abe0e0c0e036d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 30 Aug 2018 18:06:31 +0200 Subject: [PATCH 1/6] Simplify the way to freeze time in specs --- spec/features/officing/residence_spec.rb | 10 +--------- spec/features/officing/results_spec.rb | 7 +------ spec/spec_helper.rb | 8 ++++++++ 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/spec/features/officing/residence_spec.rb b/spec/features/officing/residence_spec.rb index e5f060366..034d47420 100644 --- a/spec/features/officing/residence_spec.rb +++ b/spec/features/officing/residence_spec.rb @@ -1,16 +1,8 @@ require 'rails_helper' -feature 'Residence' do +feature 'Residence', :with_frozen_time do let(:officer) { create(:poll_officer) } - background do - travel_to Time.now # TODO: use `freeze_time` after migrating to Rails 5. - end - - after do - travel_back - end - feature "Officers without assignments" do scenario "Can not access residence verification" do diff --git a/spec/features/officing/results_spec.rb b/spec/features/officing/results_spec.rb index 821e4b001..376629314 100644 --- a/spec/features/officing/results_spec.rb +++ b/spec/features/officing/results_spec.rb @@ -1,9 +1,8 @@ require 'rails_helper' -feature 'Officing Results' do +feature 'Officing Results', :with_frozen_time do background do - travel_to Time.now # TODO: use `freeze_time` after migrating to Rails 5. @poll_officer = create(:poll_officer) @officer_assignment = create(:poll_officer_assignment, :final, officer: @poll_officer) @poll = @officer_assignment.booth_assignment.poll @@ -18,10 +17,6 @@ feature 'Officing Results' do login_as(@poll_officer.user) end - after do - travel_back - end - scenario 'Only polls where user is officer for results are accessible' do regular_officer_assignment_1 = create(:poll_officer_assignment, officer: @poll_officer) regular_officer_assignment_2 = create(:poll_officer_assignment, officer: @poll_officer) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 044697bef..f14ee4041 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -82,6 +82,14 @@ RSpec.configure do |config| Bullet.end_request end + config.before(:each, :with_frozen_time) do + travel_to Time.now # TODO: use `freeze_time` after migrating to Rails 5. + end + + config.after(:each, :with_frozen_time) do + travel_back + end + # Allows RSpec to persist some state between runs in order to support # the `--only-failures` and `--next-failure` CLI options. config.example_status_persistence_file_path = "spec/examples.txt" From b787e33883bd917bcfb5887af54b5882e4698ab2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 30 Aug 2018 22:01:33 +0200 Subject: [PATCH 2/6] Use the same system to freeze time in all specs This required changing the `voted_before_sign_in` slightly in order to change what the method returns if the user signed in and voted at the exact same microsecond. It doesn't affect production code because it would be impossible for the user to do both things at the same time. As a side effect, the method now returns what the method name suggests. Before this change, the correct method name would have been `voted_before_or_at_the_same_time_of_sign_in`. As a less desirable side effect, in the tests now we need to make sure at least one second passes between the moment a user votes and the moment a user signs in again. One microsecond wouldn't work because the method `travel_to` automatically sets microseconds to zero in order to avoid rounding issues. --- app/helpers/polls_helper.rb | 2 +- spec/features/polls/polls_spec.rb | 10 ++-------- spec/features/polls/voter_spec.rb | 18 ++++++++++++------ 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/app/helpers/polls_helper.rb b/app/helpers/polls_helper.rb index 0d5b0a605..0cb875dff 100644 --- a/app/helpers/polls_helper.rb +++ b/app/helpers/polls_helper.rb @@ -46,7 +46,7 @@ module PollsHelper end def voted_before_sign_in(question) - question.answers.where(author: current_user).any? { |vote| current_user.current_sign_in_at >= vote.updated_at } + question.answers.where(author: current_user).any? { |vote| current_user.current_sign_in_at > vote.updated_at } end end diff --git a/spec/features/polls/polls_spec.rb b/spec/features/polls/polls_spec.rb index a2e009f77..b95bb8129 100644 --- a/spec/features/polls/polls_spec.rb +++ b/spec/features/polls/polls_spec.rb @@ -387,18 +387,12 @@ feature 'Polls' do end end - context 'Booth & Website' do + context 'Booth & Website', :with_frozen_time do - let(:poll) { create(:poll, summary: "Summary", description: "Description", starts_at: '2017-12-01', ends_at: '2018-02-01') } + let(:poll) { create(:poll, summary: "Summary", description: "Description") } let(:booth) { create(:poll_booth) } let(:officer) { create(:poll_officer) } - before do - allow(Date).to receive(:current).and_return Date.new(2018,1,1) - allow(Date).to receive(:today).and_return Date.new(2018,1,1) - allow(Time).to receive(:current).and_return Time.zone.parse("2018-01-01 12:00:00") - end - scenario 'Already voted on booth cannot vote on website', :js do create(:poll_shift, officer: officer, booth: booth, date: Date.current, task: :vote_collection) diff --git a/spec/features/polls/voter_spec.rb b/spec/features/polls/voter_spec.rb index 5ba89fc3c..f7f1b58c8 100644 --- a/spec/features/polls/voter_spec.rb +++ b/spec/features/polls/voter_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' feature "Voter" do - context "Origin" do + context "Origin", :with_frozen_time do let(:poll) { create(:poll, :current) } let(:question) { create(:poll_question, poll: poll) } @@ -125,12 +125,18 @@ feature "Voter" do click_link "Sign out" - login_as user - visit poll_path(poll) + # Time needs to pass between the moment we vote and the moment + # we log in; otherwise the link to vote won't be available. + # It's safe to advance one second because this test isn't + # affected by possible date changes. + travel 1.second do + login_as user + visit poll_path(poll) - within("#poll_question_#{question.id}_answers") do - expect(page).to have_link(answer_yes.title) - expect(page).to have_link(answer_no.title) + within("#poll_question_#{question.id}_answers") do + expect(page).to have_link(answer_yes.title) + expect(page).to have_link(answer_no.title) + end end end end From ec3d4c4449388506c1184fdc90ceaadc49df5121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 30 Aug 2018 22:17:33 +0200 Subject: [PATCH 3/6] Fix flaky officing dashboard spec It was failing when executed right before midnight due to today's officer assigments changing during the test. --- spec/features/officing_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/officing_spec.rb b/spec/features/officing_spec.rb index b2ead66f2..d5e410afb 100644 --- a/spec/features/officing_spec.rb +++ b/spec/features/officing_spec.rb @@ -122,7 +122,7 @@ feature 'Poll Officing' do expect(page).not_to have_css('#moderation_menu') end - scenario 'Officing dashboard available for multiple sessions', :js do + scenario 'Officing dashboard available for multiple sessions', :js, :with_frozen_time do poll = create(:poll) booth = create(:poll_booth) booth_assignment = create(:poll_booth_assignment, poll: poll, booth: booth) From d7b9ed1bc46dc0ce6500f313caaae0a1c4c9ce65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 2 Sep 2018 23:24:47 +0200 Subject: [PATCH 4/6] Use dynamic times and dates in factories (part 2) Most factories were using dynamic times and dates since commit 0cf799a. However: * At the time, commits AyuntamientoMadrid/consul@71f5351 and AyuntamientoMadrid/consul@a476a30 (which introduced static times/dates in factories) hadn't been backported. * The changes in commit 0cf799a overlooked the factory `proposal_notification`. --- spec/factories/budgets.rb | 6 +++--- spec/factories/notifications.rb | 2 +- spec/factories/proposals.rb | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/factories/budgets.rb b/spec/factories/budgets.rb index e509b2cad..d713302fd 100644 --- a/spec/factories/budgets.rb +++ b/spec/factories/budgets.rb @@ -149,11 +149,11 @@ FactoryBot.define do end trait :hidden do - hidden_at Time.current + hidden_at { Time.current } end trait :with_ignored_flag do - ignored_flag_at Time.current + ignored_flag_at { Time.current } end trait :flagged do @@ -163,7 +163,7 @@ FactoryBot.define do end trait :with_confirmed_hide do - confirmed_hide_at Time.current + confirmed_hide_at { Time.current } end end diff --git a/spec/factories/notifications.rb b/spec/factories/notifications.rb index 08f4edb86..b43aa7660 100644 --- a/spec/factories/notifications.rb +++ b/spec/factories/notifications.rb @@ -18,7 +18,7 @@ FactoryBot.define do trait :sent do recipients_count 1 - sent_at Time.current + sent_at { Time.current } end end end diff --git a/spec/factories/proposals.rb b/spec/factories/proposals.rb index 01928cdb1..358411d79 100644 --- a/spec/factories/proposals.rb +++ b/spec/factories/proposals.rb @@ -64,15 +64,15 @@ FactoryBot.define do end trait :ignored do - ignored_at Date.current + ignored_at { Date.current } end trait :hidden do - hidden_at Date.current + hidden_at { Date.current } end trait :with_confirmed_hide do - confirmed_hide_at Time.current + confirmed_hide_at { Time.current } end end From c1bb1fb2e1b53ec43667e3fea3de7c15880bffe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 2 Sep 2018 23:30:10 +0200 Subject: [PATCH 5/6] Fix flaky admin notifications spec It was failing when executed right before midnight due to the date changing between the moment the notification is created and the moment the test checks the notification shows the current date. --- spec/features/admin/admin_notifications_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/admin/admin_notifications_spec.rb b/spec/features/admin/admin_notifications_spec.rb index 9bea144d4..9c4237ccb 100644 --- a/spec/features/admin/admin_notifications_spec.rb +++ b/spec/features/admin/admin_notifications_spec.rb @@ -34,7 +34,7 @@ feature "Admin Notifications" do end context "Index" do - scenario "Valid Admin Notifications" do + scenario "Valid Admin Notifications", :with_frozen_time do draft = create(:admin_notification, segment_recipient: :all_users, title: 'Not yet sent') sent = create(:admin_notification, :sent, segment_recipient: :administrators, title: 'Sent one') From 86aa56b5e81f3c5fadc2b0b56c2350a0c78a9b30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 2 Sep 2018 23:48:02 +0200 Subject: [PATCH 6/6] Fix flaky legislation processes specs They were failing if executed right before midnight. If the process is created right before midnight and then the date changes, when we visit the process path the phase will aready be open. --- spec/features/legislation/processes_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/features/legislation/processes_spec.rb b/spec/features/legislation/processes_spec.rb index 0f233a577..f8bd9f09c 100644 --- a/spec/features/legislation/processes_spec.rb +++ b/spec/features/legislation/processes_spec.rb @@ -144,7 +144,7 @@ feature 'Legislation' do end context 'debate phase' do - scenario 'not open' do + scenario 'not open', :with_frozen_time do process = create(:legislation_process, debate_start_date: Date.current + 1.day, debate_end_date: Date.current + 2.days) visit legislation_process_path(process) @@ -179,7 +179,7 @@ feature 'Legislation' do end context 'draft publication phase' do - scenario 'not open' do + scenario 'not open', :with_frozen_time do process = create(:legislation_process, draft_publication_date: Date.current + 1.day) visit draft_publication_legislation_process_path(process) @@ -199,7 +199,7 @@ feature 'Legislation' do end context 'allegations phase' do - scenario 'not open' do + scenario 'not open', :with_frozen_time do process = create(:legislation_process, allegations_start_date: Date.current + 1.day, allegations_end_date: Date.current + 2.days) visit allegations_legislation_process_path(process) @@ -219,7 +219,7 @@ feature 'Legislation' do end context 'final version publication phase' do - scenario 'not open' do + scenario 'not open', :with_frozen_time do process = create(:legislation_process, result_publication_date: Date.current + 1.day) visit result_publication_legislation_process_path(process)