From 4318b371b065559ac729219e7f8c65a4048bd43a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 29 Mar 2022 21:17:02 +0200 Subject: [PATCH 1/5] Add expectations after submit in attachments specs There were cases where we clicked the button to submit the form and immediately we visited a different page. In the past, we've had similar code produce PG::ProtocolViolation errors in similar situations. Since we've had these errors a few times in the nested imageable specs, there's a chance they're related to the absence of the expectation. Although I'm not even remotely sure this will fix these issues, at least we now follow the convention of making expectations after every request. Note we're changing both the nested imageable and nested documentable specs. Only the nested imageable would need to be changed because it's the one where there's a `visit` inside the `imageable_redirected_to_resource_show_or_navigate_to` method. I'm changing both for consistency. --- spec/shared/system/nested_documentable.rb | 5 +++++ spec/shared/system/nested_imageable.rb | 3 +++ 2 files changed, 8 insertions(+) diff --git a/spec/shared/system/nested_documentable.rb b/spec/shared/system/nested_documentable.rb index dafc01796..08225a442 100644 --- a/spec/shared/system/nested_documentable.rb +++ b/spec/shared/system/nested_documentable.rb @@ -210,6 +210,8 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na documentable_attach_new_file(file_fixture("empty.pdf")) click_on submit_button + expect(page).to have_content documentable_success_notice + documentable_redirected_to_resource_show_or_navigate_to expect(page).to have_content "Documents" @@ -232,6 +234,9 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na end click_on submit_button + + expect(page).to have_content documentable_success_notice + documentable_redirected_to_resource_show_or_navigate_to expect(page).to have_content "Documents (#{documentable.class.max_documents_allowed})" diff --git a/spec/shared/system/nested_imageable.rb b/spec/shared/system/nested_imageable.rb index 1d6d1899e..f82b66abb 100644 --- a/spec/shared/system/nested_imageable.rb +++ b/spec/shared/system/nested_imageable.rb @@ -184,6 +184,9 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p expect(page).to have_selector ".loading-bar.complete" click_on submit_button + + expect(page).to have_content imageable_success_notice + imageable_redirected_to_resource_show_or_navigate_to(imageable) expect(page).to have_selector "figure img" From 2d693328dca71136c571dd34cfa6500ec202107a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 2 Jun 2022 18:40:56 +0200 Subject: [PATCH 2/5] Simplify checking map longitude and latitude We were doing a `mappable.map_location` call in an `expect` which might result in a database queries. Doing database queries in a test after the process running the browser has started might result in exceptions while running our test suite. --- spec/shared/system/mappable.rb | 4 +++- spec/support/common_actions/maps.rb | 9 --------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/spec/shared/system/mappable.rb b/spec/shared/system/mappable.rb index b1551a47a..68d1edde4 100644 --- a/spec/shared/system/mappable.rb +++ b/spec/shared/system/mappable.rb @@ -175,12 +175,14 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, describe "At #{mappable_edit_path}", if: mappable_edit_path.present? do scenario "Should edit map on #{mappable_factory_name} and contain default values" do + mappable.map_location.update!(latitude: 51.48, longitude: 0.0) do_login_for mappable.author, management: management visit send(mappable_edit_path, id: mappable.id) expect(page).to have_content "Navigate the map to the location and place the marker." - validate_latitude_longitude(mappable, mappable_factory_name) + expect(page).to have_field "#{mappable_factory_name}_map_location_attributes_latitude", type: :hidden, with: "51.48" + expect(page).to have_field "#{mappable_factory_name}_map_location_attributes_longitude", type: :hidden, with: "0.0" end scenario "Should edit default values from map on #{mappable_factory_name} edit page" do diff --git a/spec/support/common_actions/maps.rb b/spec/support/common_actions/maps.rb index 28be79eb1..1cc8ce233 100644 --- a/spec/support/common_actions/maps.rb +++ b/spec/support/common_actions/maps.rb @@ -13,15 +13,6 @@ module Maps end end - def validate_latitude_longitude(mappable, mappable_factory_name) - latitude_attribute = "##{mappable_factory_name}_map_location_attributes_latitude" - longitude_attribute = "##{mappable_factory_name}_map_location_attributes_longitude" - expect(find(latitude_attribute, visible: false).value).to eq "51.48" - expect(find(longitude_attribute, visible: false).value).to eq "0.0" - expect(mappable.map_location.latitude).to eq 51.48 - expect(mappable.map_location.longitude).to eq 0.0 - end - def fill_in_budget_investment_form fill_in_new_investment_title with: "Budget investment title" fill_in_ckeditor "Description", with: "Budget investment description" From 9a8536b1fcc0bf79001ec27f4b1f76f14a33a7ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 2 Jun 2022 18:44:46 +0200 Subject: [PATCH 3/5] Avoid simultaneous requests in moderation test We were clicking on the "Sign in" link right after clicking on the "Sign out" link, which might result in simultaneous requests and exceptions when running our test suite. So we're adding an expectation to make sure the first request has finished before starting the following one. --- spec/system/moderation/users_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/system/moderation/users_spec.rb b/spec/system/moderation/users_spec.rb index cf712de18..ce9eed9ec 100644 --- a/spec/system/moderation/users_spec.rb +++ b/spec/system/moderation/users_spec.rb @@ -38,7 +38,9 @@ describe "Moderate users" do expect(page).not_to have_content(comment3.body) - click_link("Sign out") + click_link "Sign out" + + expect(page).to have_content "You have been signed out successfully" visit root_path From 0dded3fa22c46b4b3eebd820426a33c0bc97ac4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 2 Jun 2022 18:59:13 +0200 Subject: [PATCH 4/5] Remove redundant expectations in polls tests Furthermore, using `Poll.all` results in a database query, and doing so after the process running the browser has started might result in failures when running our test suite. --- spec/system/admin/poll/polls_spec.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/spec/system/admin/poll/polls_spec.rb b/spec/system/admin/poll/polls_spec.rb index dc3b090aa..afeaaf8da 100644 --- a/spec/system/admin/poll/polls_spec.rb +++ b/spec/system/admin/poll/polls_spec.rb @@ -21,13 +21,6 @@ describe "Admin polls", :admin do expect(page).to have_content "List of polls" expect(page).to have_css ".poll", count: 3 - polls = Poll.all - polls.each do |poll| - within("#poll_#{poll.id}") do - expect(page).to have_content poll.name - end - end - expect(poll_3.name).to appear_before(poll_1.name) expect(poll_1.name).to appear_before(poll_2.name) expect(page).not_to have_content "There are no polls" From d91775b4aa704676bddd7a264be0a8e3f0572c4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 2 Jun 2022 19:03:11 +0200 Subject: [PATCH 5/5] Make database queries before starting the browser When we perform database queries in tests after the process running the browser has started, we sometimes get failures in our test suite due to both the tests and the browser accessing the database at the same time. --- spec/system/admin/poll/booths_spec.rb | 3 +-- spec/system/admin/poll/polls_spec.rb | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/system/admin/poll/booths_spec.rb b/spec/system/admin/poll/booths_spec.rb index eb6cead7e..a930bf9f6 100644 --- a/spec/system/admin/poll/booths_spec.rb +++ b/spec/system/admin/poll/booths_spec.rb @@ -13,7 +13,7 @@ describe "Admin booths", :admin do end scenario "Index" do - 3.times { create(:poll_booth) } + booths = 3.times.map { create(:poll_booth) } visit admin_root_path @@ -22,7 +22,6 @@ describe "Admin booths", :admin do click_link "Booths location" end - booths = Poll::Booth.all booths.each do |booth| within("#booth_#{booth.id}") do expect(page).to have_content booth.name diff --git a/spec/system/admin/poll/polls_spec.rb b/spec/system/admin/poll/polls_spec.rb index afeaaf8da..a677ca0cc 100644 --- a/spec/system/admin/poll/polls_spec.rb +++ b/spec/system/admin/poll/polls_spec.rb @@ -212,6 +212,7 @@ describe "Admin polls", :admin do booth.booth_assignments.each do |booth_assignment| 3.times { create(:poll_officer_assignment, booth_assignment: booth_assignment) } end + officers = Poll::Officer.all visit admin_poll_path(poll) @@ -219,7 +220,6 @@ describe "Admin polls", :admin do expect(page).to have_css ".officer", count: 3 - officers = Poll::Officer.all officers.each do |officer| within("#officer_#{officer.id}") do expect(page).to have_content officer.name