From f93c85bd2eca8e1beac721b5779229acacc0d08e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 13 Mar 2025 00:44:55 +0100 Subject: [PATCH] Don't check the database during 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. IMHO this is also a bad practice for system tests, since these tests should be checking what users experience. So, just like we did a few commits ago with tests that reloaded records, we're modifying the tests to check the results of user interactions from the point of view of the users. 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. Finally, note that one of the tests we're changing in the shared `notifiable_in_app` file did not check the database content, but we're also changing it for consistency. --- spec/shared/system/notifiable_in_app.rb | 12 ++++------ spec/system/account_spec.rb | 4 +++- spec/system/documents_spec.rb | 2 +- spec/system/management/users_spec.rb | 31 +++++++++++++++---------- spec/system/officing_spec.rb | 15 ++++-------- 5 files changed, 32 insertions(+), 32 deletions(-) diff --git a/spec/shared/system/notifiable_in_app.rb b/spec/shared/system/notifiable_in_app.rb index 8f8af11fd..d8a59c86e 100644 --- a/spec/shared/system/notifiable_in_app.rb +++ b/spec/shared/system/notifiable_in_app.rb @@ -22,8 +22,7 @@ shared_examples "notifiable in-app" do |factory_name| click_link "You have a new notification" expect(page).to have_css ".notification", count: 1 - expect(page).to have_content "Someone commented on" - expect(page).to have_xpath "//a[@href='#{notification_path(notification)}']" + expect(page).to have_link text: "Someone commented on", href: notification_path(notification) end scenario "Multiple users commented on my notifiable" do @@ -44,8 +43,7 @@ shared_examples "notifiable in-app" do |factory_name| visit notifications_path expect(page).to have_css ".notification", count: 1 - expect(page).to have_content "There are 3 new comments on" - expect(page).to have_xpath "//a[@href='#{notification_path(Notification.last)}']" + expect(page).to have_link text: "There are 3 new comments on" end scenario "A user replied to my comment" do @@ -58,8 +56,7 @@ shared_examples "notifiable in-app" do |factory_name| visit notifications_path expect(page).to have_css ".notification", count: 1 - expect(page).to have_content "Someone replied to your comment on" - expect(page).to have_xpath "//a[@href='#{notification_path(Notification.last)}']" + expect(page).to have_link text: "Someone replied to your comment on" end scenario "Multiple replies to my comment" do @@ -85,8 +82,7 @@ shared_examples "notifiable in-app" do |factory_name| visit notifications_path expect(page).to have_css ".notification", count: 1 - expect(page).to have_content "There are 3 new replies to your comment on" - expect(page).to have_xpath "//a[@href='#{notification_path(Notification.last)}']" + expect(page).to have_link text: "There are 3 new replies to your comment on" end scenario "Author commented on his own notifiable" do diff --git a/spec/system/account_spec.rb b/spec/system/account_spec.rb index 9bea31abe..aa12b6def 100644 --- a/spec/system/account_spec.rb +++ b/spec/system/account_spec.rb @@ -194,7 +194,9 @@ describe "Account" do user.update!(username: "Admin") administrators = [create(:administrator, user: user), create(:administrator, user: create(:user, username: "Other admin"))] + other_user = administrators.last.user budget = create(:budget, administrators: administrators) + visit admin_budget_budget_investments_path(budget) expect(page).to have_select options: ["All administrators", "Admin", "Other admin"] @@ -206,7 +208,7 @@ describe "Account" do expect(page).to have_content "Goodbye! Your account has been cancelled. We hope to see you again soon." - login_as(administrators.last.user) + login_as(other_user) visit admin_budget_budget_investments_path(budget) expect(page).to have_select options: ["All administrators", "Other admin"] diff --git a/spec/system/documents_spec.rb b/spec/system/documents_spec.rb index 2b3ff51cb..b1abac580 100644 --- a/spec/system/documents_spec.rb +++ b/spec/system/documents_spec.rb @@ -15,7 +15,7 @@ describe "Documents" do expect(page).to have_content "Proposal created successfully" - io = URI.parse("#{app_host}#{polymorphic_path(Document.last.attachment)}").open + io = URI.parse(find_link(text: "PDF")[:href]).open reader = PDF::Reader.new(io) expect(reader.info[:Keywords]).not_to eq "Test Metadata" diff --git a/spec/system/management/users_spec.rb b/spec/system/management/users_spec.rb index 4a21eb072..2da45d536 100644 --- a/spec/system/management/users_spec.rb +++ b/spec/system/management/users_spec.rb @@ -4,7 +4,7 @@ describe "Users" do scenario "Create a level 3 user with email from scratch" do login_as_manager visit management_document_verifications_path - fill_in "document_verification_document_number", with: "12345678Z" + fill_in "Document number", with: "12345678Z" click_button "Check document" expect(page).to have_content "Please introduce the email used on the account" @@ -20,12 +20,11 @@ describe "Users" do expect(page).to have_content "We have sent an email" expect(page).not_to have_content "Autogenerated password is" - user = User.find_by(email: "pepe@gmail.com") + visit management_document_verifications_path + fill_in "Document number", with: "12345678Z" + click_button "Check document" - expect(user).to be_level_three_verified - expect(user).to be_residence_verified - expect(user).not_to be_confirmed - expect(user.date_of_birth).to have_content Date.new(1980, 12, 31) + expect(page).to have_content "This user account is already verified" sent_token = /.*confirmation_token=(.*)".*/.match(ActionMailer::Base.deliveries.last.body.to_s)[1] visit user_confirmation_path(confirmation_token: sent_token) @@ -62,14 +61,22 @@ describe "Users" do click_button "Create user" expect(page).not_to have_content "We have sent an email" - expect(page).to have_content "Autogenerated password is" + generated_password = find("p", text: "Autogenerated password is").find("b").text - user = User.find_by(username: "Kelly Sue") + visit management_document_verifications_path + fill_in "Document number", with: "12345678Z" + click_button "Check document" - expect(user).to be_level_three_verified - expect(user).to be_residence_verified - expect(user).to be_confirmed - expect(user.date_of_birth).to have_content Date.new(1980, 12, 31) + expect(page).to have_content "This user account is already verified" + + logout + login_through_form_with("Kelly Sue", password: generated_password) + + expect(page).to have_content "You have been signed in successfully." + + visit account_path + + expect(page).to have_content "Account verified" end scenario "Delete a level 2 user account from document verification page" do diff --git a/spec/system/officing_spec.rb b/spec/system/officing_spec.rb index ba054901b..f78c9684f 100644 --- a/spec/system/officing_spec.rb +++ b/spec/system/officing_spec.rb @@ -134,11 +134,6 @@ describe "Poll Officing" do click_button "Confirm vote" expect(page).to have_content "Vote introduced!" - expect(Poll::Voter.where(document_number: "12345678Z", - poll_id: poll, - origin: "booth", - officer_id: officer1) - .count).to be(1) end in_browser(:two) do @@ -147,11 +142,11 @@ describe "Poll Officing" do click_button "Confirm vote" expect(page).to have_content "Vote introduced!" - expect(Poll::Voter.where(document_number: "12345678Y", - poll_id: poll, - origin: "booth", - officer_id: officer2) - .count).to be(1) + + visit new_officing_residence_path + officing_verify_residence(document_number: "12345678Z") + + expect(page).to have_content "Has already participated in this poll" end end end