Don't reload records in system tests

As mentioned in commits like a586ba806, a7664ad81, 006128da5, b41fbfa52
and c480cdd91, accessing the database after starting the browser with
the `visit` method sometimes results in database corruption and failing
tests on our CI due to the process running the test accessing the
database after the process running the browser has started.

For example, one of these tests has recently failed on our CI:

```
3) Users Create a level 3 user with email from scratch
   Failure/Error: expect(user.reload).to be_confirmed
     expected `#<User id: 2060, email: "pepe@gmail.com", created_at:
     "2025-03-12 19:51:03.688867000 +0100", updated_...d_debates: true,
     recommended_proposals: true, subscriptions_token: nil,
     registering_from_web: false>.confirmed?` to be truthy, got false
```

IMHO this is also a bad practice for system tests, since these tests
should be checking what users experience.

So we're modifying the tests to check the results of users interaction
from the point of view of the users. For example, instead of checking
that a user is now level 3 verified in the database, we're checking that
the user interface states that the user is level 3 verified.

Note we're adding an offset when editing the map marker by clicking on
`map-location` with `.click(x: 30, y: 30)`. This way we make sure that
both the latitude and longitude change from the original values; we used
to clicking in the middle (no offset), which didn't change the longitude
and changed the latitude just by coincidence.

Also note we aren't changing tests with the `:no_js` tag, since these
tests don't run a real browser in a separate process. In the future, we
should also change most of these tests so they don't access the database
and they use a real browser.
This commit is contained in:
Javi Martín
2025-03-12 15:23:11 +01:00
parent e29ad8b505
commit 1b8a079727
11 changed files with 98 additions and 84 deletions

View File

@@ -193,21 +193,35 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
end end
scenario "Should edit default values from map on #{mappable_factory_name} edit page" do scenario "Should edit default values from map on #{mappable_factory_name} edit page" do
original_longitude = map_location.longitude
original_latitude = map_location.latitude
do_login_for mappable.author, management: management do_login_for mappable.author, management: management
visit send(mappable_edit_path, id: mappable.id) visit send(mappable_edit_path, id: mappable.id)
find(".map-location").click find(".map-location").click(x: 30, y: 30)
new_latitude = find_field(
"#{mappable_factory_name}_map_location_attributes_latitude", type: :hidden
).value
new_longitude = find_field(
"#{mappable_factory_name}_map_location_attributes_longitude", type: :hidden
).value
click_button "Save changes" click_button "Save changes"
expect(page).not_to have_button "Save changes" expect(page).not_to have_button "Save changes"
mappable.reload expect(page).to have_css ".map-location"
expect(page).to have_css ".map-location[data-marker-latitude='#{new_latitude}']"
expect(page).to have_css(".map-location") expect(page).to have_css ".map-location[data-marker-longitude='#{new_longitude}']"
expect(page).not_to have_css ".map-location[data-marker-latitude='#{map_location.latitude}']" expect(page).not_to have_css ".map-location[data-marker-latitude='#{original_latitude}']"
expect(page).to have_css ".map-location[data-marker-latitude='#{mappable.map_location.latitude}']" expect(page).not_to have_css ".map-location[data-marker-longitude='#{original_longitude}']"
end end
scenario "Should edit mappable on #{mappable_factory_name} without change map" do scenario "Should edit mappable on #{mappable_factory_name} without change map" do
original_longitude = map_location.longitude
original_latitude = map_location.latitude
do_login_for mappable.author, management: management do_login_for mappable.author, management: management
visit send(mappable_edit_path, id: mappable.id) visit send(mappable_edit_path, id: mappable.id)
@@ -215,11 +229,9 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
click_button "Save changes" click_button "Save changes"
expect(page).not_to have_button "Save changes" expect(page).not_to have_button "Save changes"
mappable.reload expect(page).to have_css ".map-location"
expect(page).to have_css ".map-location[data-marker-latitude='#{original_latitude}']"
expect(page).to have_css(".map-location") expect(page).to have_css ".map-location[data-marker-longitude='#{original_longitude}']"
expect(page).to have_css ".map-location[data-marker-latitude='#{map_location.latitude}']"
expect(page).to have_css ".map-location[data-marker-latitude='#{mappable.map_location.latitude}']"
end end
scenario "Can not display map on #{mappable_factory_name} edit when remove map marker" do scenario "Can not display map on #{mappable_factory_name} edit when remove map marker" do

View File

@@ -22,15 +22,13 @@ module Polls
expect(Poll::Voter.count).to eq(1) expect(Poll::Voter.count).to eq(1)
end end
def confirm_phone(user = nil) def confirm_phone(code:)
user ||= User.last
fill_in "sms_phone", with: "611111111" fill_in "sms_phone", with: "611111111"
click_button "Send" click_button "Send"
expect(page).to have_content "Enter the confirmation code sent to you by text message" expect(page).to have_content "Enter the confirmation code sent to you by text message"
fill_in "sms_confirmation_code", with: user.reload.sms_confirmation_code fill_in "sms_confirmation_code", with: code
click_button "Send" click_button "Send"
expect(page).to have_content "Code correct" expect(page).to have_content "Code correct"

View File

@@ -104,17 +104,21 @@ describe "Admin custom information texts", :admin do
end end
scenario "Remove a translation" do scenario "Remove a translation" do
featured = create(:i18n_content, key: "debates.index.featured_debates", create(:i18n_content, key: "debates.index.featured_debates",
value_en: "Custom featured", value_en: "Custom featured",
value_es: "Destacar personalizado") value_es: "Destacar personalizado")
page_title = create(:i18n_content, key: "debates.new.start_new", create(:i18n_content, key: "debates.new.start_new",
value_en: "Start a new debate", value_en: "Start a new custom debate",
value_es: "Empezar un debate") value_es: "Empezar un nuevo debate personalizado")
visit admin_site_customization_information_texts_path(tab: "debates") visit admin_site_customization_information_texts_path(tab: "debates")
select "Español", from: "Current language" select "Español", from: "Current language"
expect(page).to have_field "debates.index.featured_debates", with: "Destacar personalizado"
expect(page).to have_field "debates.new.start_new", with: "Empezar un nuevo debate personalizado"
click_link "Remove language" click_link "Remove language"
click_button "Save" click_button "Save"
@@ -124,20 +128,13 @@ describe "Admin custom information texts", :admin do
visit admin_site_customization_information_texts_path(tab: "debates") visit admin_site_customization_information_texts_path(tab: "debates")
expect(page).to have_field "debates.index.featured_debates" expect(page).to have_field "debates.index.featured_debates", with: "Custom featured"
expect(page).to have_field "debates.new.start_new", with: "Start a new custom debate"
select "English", from: "Current language" select "Español", from: "Add language"
expect(page).to have_content "Start a new debate" expect(page).to have_field "debates.index.featured_debates", with: "Destacar"
expect(page).to have_content "Custom featured" expect(page).to have_field "debates.new.start_new", with: "Empezar un debate"
featured.reload
page_title.reload
expect(page_title.value_es).to be nil
expect(featured.value_es).to be nil
expect(page_title.value_en).to eq "Start a new debate"
expect(featured.value_en).to eq "Custom featured"
end end
end end
end end

View File

@@ -26,7 +26,11 @@ describe "DocumentVerifications" do
expect(page).to have_content "already verified" expect(page).to have_content "already verified"
expect(user.reload).to be_level_three_verified visit management_document_verifications_path
fill_in "document_verification_document_number", with: user.document_number
click_button "Check document"
expect(page).to have_content "already verified"
end end
describe "Verifying througth Census" do describe "Verifying througth Census" do

View File

@@ -2,35 +2,46 @@ require "rails_helper"
describe "EmailVerifications" do describe "EmailVerifications" do
scenario "Verifying a level 1 user via email" do scenario "Verifying a level 1 user via email" do
login_as_manager
user = create(:user) user = create(:user)
visit management_document_verifications_path in_browser(:user) do
fill_in "document_verification_document_number", with: "12345678Z" login_as user
click_button "Check document" visit root_path
expect(page).to have_content "Please introduce the email used on the account" expect(page).to have_content "Debates"
end
fill_in "email_verification_email", with: user.email in_browser(:manager) do
click_button "Send verification email" login_as_manager
visit management_document_verifications_path
fill_in "document_verification_document_number", with: "12345678Z"
click_button "Check document"
expect(page).to have_content "In order to completely verify this user, " \ expect(page).to have_content "Please introduce the email used on the account"
"it is necessary that the user clicks on a link"
user.reload fill_in "email_verification_email", with: user.email
click_button "Send verification email"
login_as(user) expect(page).to have_content "In order to completely verify this user, " \
"it is necessary that the user clicks on a link"
end
sent_token = /.*email_verification_token=(.*)".*/.match(ActionMailer::Base.deliveries.last.body.to_s)[1] sent_token = /.*email_verification_token=(.*)".*/.match(ActionMailer::Base.deliveries.last.body.to_s)[1]
visit email_path(email_verification_token: sent_token)
expect(page).to have_content "You are a verified user" in_browser(:user) do
visit email_path(email_verification_token: sent_token)
expect(page).not_to have_link "Verify my account" expect(page).to have_content "You are a verified user"
expect(page).to have_content "Account verified" expect(page).to have_content "Account verified"
expect(page).not_to have_link "Verify my account"
end
expect(user.reload.document_number).to eq("12345678Z") in_browser(:manager) do
expect(user).to be_level_three_verified visit management_document_verifications_path
fill_in "document_verification_document_number", with: "12345678Z"
click_button "Check document"
expect(page).to have_content "This user account is already verified"
end
end end
end end

View File

@@ -37,9 +37,12 @@ describe "Users" do
click_button "Confirm" click_button "Confirm"
expect(user.reload).to be_confirmed
expect(page).to have_content "Your account has been confirmed." expect(page).to have_content "Your account has been confirmed."
visit account_path
expect(page).to have_field "Username", with: "pepe"
expect(page).to have_content "Account verified"
end end
scenario "Create a level 3 user without email from scratch" do scenario "Create a level 3 user without email from scratch" do

View File

@@ -189,6 +189,7 @@ describe "Voter" do
end end
scenario "Voting in poll and then verifiying account" do scenario "Voting in poll and then verifiying account" do
allow_any_instance_of(Verification::Sms).to receive(:generate_confirmation_code).and_return("1357")
user = create(:user) user = create(:user)
login_through_form_as_officer(officer.user) login_through_form_as_officer(officer.user)
@@ -200,7 +201,7 @@ describe "Voter" do
click_link "Verify my account" click_link "Verify my account"
verify_residence verify_residence
confirm_phone(user) confirm_phone(code: "1357")
visit poll_path(poll) visit poll_path(poll)

View File

@@ -15,10 +15,9 @@ describe "Verify Letter" do
"we will send it to the address featuring in the data " \ "we will send it to the address featuring in the data " \
"we have on file." "we have on file."
user.reload visit verification_path
expect(user.letter_requested_at).to be expect(page).to have_field "Code you received in letter"
expect(user.letter_verification_code).to be
end end
scenario "Deny access unless verified residence" do scenario "Deny access unless verified residence" do

View File

@@ -2,6 +2,7 @@ require "rails_helper"
describe "Level three verification" do describe "Level three verification" do
scenario "Verification with residency and verified sms" do scenario "Verification with residency and verified sms" do
allow_any_instance_of(Verification::Sms).to receive(:generate_confirmation_code).and_return("2015")
create(:geozone) create(:geozone)
user = create(:user) user = create(:user)
@@ -23,8 +24,7 @@ describe "Level three verification" do
expect(page).to have_content "Security code confirmation" expect(page).to have_content "Security code confirmation"
user = user.reload fill_in "sms_confirmation_code", with: "2015"
fill_in "sms_confirmation_code", with: user.sms_confirmation_code
click_button "Send" click_button "Send"
expect(page).to have_content "Code correct. Your account is now verified" expect(page).to have_content "Code correct. Your account is now verified"
@@ -65,6 +65,7 @@ describe "Level three verification" do
end end
scenario "Verification with residency and sms and letter" do scenario "Verification with residency and sms and letter" do
allow_any_instance_of(Verification::Sms).to receive(:generate_confirmation_code).and_return("1234")
create(:geozone) create(:geozone)
user = create(:user) user = create(:user)
login_as(user) login_as(user)
@@ -73,17 +74,7 @@ describe "Level three verification" do
click_link "Verify my account" click_link "Verify my account"
verify_residence verify_residence
confirm_phone(code: "1234")
fill_in "sms_phone", with: "611111111"
click_button "Send"
expect(page).to have_content "Security code confirmation"
user = user.reload
fill_in "sms_confirmation_code", with: user.sms_confirmation_code
click_button "Send"
expect(page).to have_content "Code correct"
click_link "Send me a letter with the code" click_link "Send me a letter with the code"

View File

@@ -40,11 +40,16 @@ describe "Residence" do
scenario "When trying to verify a deregistered account old votes are reassigned" do scenario "When trying to verify a deregistered account old votes are reassigned" do
erased_user = create(:user, document_number: "12345678Z", document_type: "1", erased_at: Time.current) erased_user = create(:user, document_number: "12345678Z", document_type: "1", erased_at: Time.current)
vote = create(:vote, voter: erased_user)
new_user = create(:user) new_user = create(:user)
debate = create(:debate, title: "Improve everything")
create(:vote, voter: erased_user, votable: debate)
login_as(new_user) login_as(new_user)
visit debate_path(debate)
expect(page).to have_css "button[aria-pressed='false']", exact_text: "I agree"
visit account_path visit account_path
click_link "Verify my account" click_link "Verify my account"
@@ -58,9 +63,9 @@ describe "Residence" do
expect(page).to have_content "Residence verified" expect(page).to have_content "Residence verified"
expect(vote.reload.voter).to eq(new_user) visit debate_path(debate)
expect(erased_user.reload.document_number).to be_blank
expect(new_user.reload.document_number).to eq("12345678Z") expect(page).to have_css "button[aria-pressed='true']", exact_text: "I agree"
end end
scenario "Error on verify" do scenario "Error on verify" do

View File

@@ -2,19 +2,12 @@ require "rails_helper"
describe "SMS Verification" do describe "SMS Verification" do
scenario "Verify" do scenario "Verify" do
allow_any_instance_of(Verification::Sms).to receive(:generate_confirmation_code).and_return("5678")
user = create(:user, residence_verified_at: Time.current) user = create(:user, residence_verified_at: Time.current)
login_as(user) login_as(user)
visit new_sms_path visit new_sms_path
confirm_phone(code: "5678")
fill_in "sms_phone", with: "611111111"
click_button "Send"
expect(page).to have_content "Security code confirmation"
user = user.reload
fill_in "sms_confirmation_code", with: user.sms_confirmation_code
click_button "Send"
expect(page).to have_content "Code correct" expect(page).to have_content "Code correct"
end end