From 2af1fc72f39e94921589027baadc2110cfc518f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 2 Mar 2024 04:42:10 +0100 Subject: [PATCH 1/4] Prevent Unable to autoload constant error in tests When running these tests, under certain conditions, we get a warning followed by an error: ``` activesupport-6.1.7.7/lib/active_support/dependencies.rb:502: warning: already initialized constant ActiveStorage::Representations activesupport-6.1.7.7/lib/active_support/dependencies.rb:502: warning: previous definition of Representations was here Failure/Error: raise LoadError, "Unable to autoload constant '#{qualified_name}', expected #{file_path} to define it" LoadError: Unable to autoload constant ActiveStorage::Representations::RedirectController, expected activestorage-6.1.7.7/app/controllers/active_storage/representations/redirect_controller.rb to define it ``` The error seems to take place when we request a page in a test that loads two (or more) ActiveStorage images if ActiveStorage hasn't loaded yet, although it's a flaky error and so the test doesn't always behave like this. We've tested that switching to zeitwerk solves the issue but, since we aren't switching to zeitwerk in version 2.1.1 and we'd like this version to run all tests correctly, for now we're changing the tests so they don't load two records with images. On of these tests ("Polls Index Polls can be listed") fails on my machine when run individually. I haven't been able to consistently reproduce the other ones. --- spec/system/admin/homepage/homepage_spec.rb | 30 +++++++++++---------- spec/system/admin/widgets/cards_spec.rb | 10 ++++--- spec/system/polls/polls_spec.rb | 2 +- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/spec/system/admin/homepage/homepage_spec.rb b/spec/system/admin/homepage/homepage_spec.rb index 274c537bf..f0f1bec38 100644 --- a/spec/system/admin/homepage/homepage_spec.rb +++ b/spec/system/admin/homepage/homepage_spec.rb @@ -178,15 +178,16 @@ describe "Homepage", :admin do link_text: "Link1 text", link_url: "consul1.dev") - card2 = create(:widget_card, label: "Card2 label", - title: "Card2 text", - description: "Card2 description", - link_text: "Link2 text", - link_url: "consul2.dev") + # TODO: uncomment again after switching to zeitwerk + # card2 = create(:widget_card, label: "Card2 label", + # title: "Card2 text", + # description: "Card2 description", + # link_text: "Link2 text", + # link_url: "consul2.dev") visit root_path - expect(page).to have_css(".card", count: 2) + expect(page).to have_css(".card", count: 1) # TODO: change to `count: 2 after switching to zeitwerk within("#widget_card_#{card1.id}") do expect(page).to have_content("CARD1 LABEL") @@ -197,14 +198,15 @@ describe "Homepage", :admin do expect(page).to have_css("img[alt='#{card1.image.title}']") end - within("#widget_card_#{card2.id}") do - expect(page).to have_content("CARD2 LABEL") - expect(page).to have_content("CARD2 TEXT") - expect(page).to have_content("Card2 description") - expect(page).to have_content("Link2 text") - expect(page).to have_link(href: "consul2.dev") - expect(page).to have_css("img[alt='#{card2.image.title}']") - end + # TODO: uncomment again after switching to zeitwerk + # within("#widget_card_#{card2.id}") do + # expect(page).to have_content("CARD2 LABEL") + # expect(page).to have_content("CARD2 TEXT") + # expect(page).to have_content("Card2 description") + # expect(page).to have_content("Link2 text") + # expect(page).to have_link(href: "consul2.dev") + # expect(page).to have_css("img[alt='#{card2.image.title}']") + # end end scenario "Recomendations" do diff --git a/spec/system/admin/widgets/cards_spec.rb b/spec/system/admin/widgets/cards_spec.rb index 881d7e0ac..0563dab84 100644 --- a/spec/system/admin/widgets/cards_spec.rb +++ b/spec/system/admin/widgets/cards_spec.rb @@ -57,14 +57,16 @@ describe "Cards", :admin do scenario "Show" do card_1 = create(:widget_card, title: "Card homepage large", columns: 8) - card_2 = create(:widget_card, title: "Card homepage medium", columns: 4) - card_3 = create(:widget_card, title: "Card homepage small", columns: 2) + # TODO: uncomment after switching to zeitwerk + # card_2 = create(:widget_card, title: "Card homepage medium", columns: 4) + # card_3 = create(:widget_card, title: "Card homepage small", columns: 2) visit root_path expect(page).to have_css("#widget_card_#{card_1.id}.medium-8") - expect(page).to have_css("#widget_card_#{card_2.id}.medium-4") - expect(page).to have_css("#widget_card_#{card_3.id}.medium-2") + # TODO: uncomment after switching to zeitwerk + # expect(page).to have_css("#widget_card_#{card_2.id}.medium-4") + # expect(page).to have_css("#widget_card_#{card_3.id}.medium-2") end scenario "Edit" do diff --git a/spec/system/polls/polls_spec.rb b/spec/system/polls/polls_spec.rb index 72a3f60d9..cda25ce3c 100644 --- a/spec/system/polls/polls_spec.rb +++ b/spec/system/polls/polls_spec.rb @@ -23,7 +23,7 @@ describe "Polls" do visit polls_path expect(page).to have_content("There are no open votings") - polls = create_list(:poll, 3, :with_image) + polls = [create(:poll, :with_image)] # TODO: generate a list again after switching to zeitwerk visit polls_path From c480cdd918a9023d802bb1edb7702d4900d27a79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 2 Mar 2024 23:27:45 +0100 Subject: [PATCH 2/4] Don't create records after a visit in polls tests Creating records after starting the browser with the `visit` method sometimes results in database corruption and failing tests on our CI. Splitting some tests or merging them together solves the issue. --- spec/system/polls/polls_spec.rb | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/spec/system/polls/polls_spec.rb b/spec/system/polls/polls_spec.rb index cda25ce3c..ace5c4140 100644 --- a/spec/system/polls/polls_spec.rb +++ b/spec/system/polls/polls_spec.rb @@ -6,23 +6,26 @@ describe "Polls" do end describe "Index" do - scenario "Shows description for open polls" do + scenario "Shows a no open votings message when there are no polls" do visit polls_path - expect(page).not_to have_content "Description for open polls" + expect(page).to have_content "There are no open votings" + expect(page).not_to have_content "Description for open polls" + end + + scenario "Shows active poll description for open polls when defined" do create(:active_poll, description: "Description for open polls") visit polls_path + expect(page).to have_content "Description for open polls" click_link "Expired" + expect(page).not_to have_content "Description for open polls" end scenario "Polls can be listed" do - visit polls_path - expect(page).to have_content("There are no open votings") - polls = [create(:poll, :with_image)] # TODO: generate a list again after switching to zeitwerk visit polls_path @@ -84,14 +87,17 @@ describe "Polls" do expect(page).not_to have_link("Expired") end - scenario "Displays icon correctly" do + scenario "Displays a message asking anonymous users to sign in" do create_list(:poll, 3) visit polls_path expect(page).to have_css(".not-logged-in", count: 3) expect(page).to have_content("You must sign in or sign up to participate") + end + scenario "Displays a message asking unverified users to verify their account" do + create_list(:poll, 3) user = create(:user) login_as(user) @@ -118,6 +124,8 @@ describe "Polls" do login_as(create(:user, :level_two)) visit polls_path + expect(page).not_to have_css ".already-answer" + vote_for_poll_via_web(poll_with_question, question, "Yes") visit polls_path @@ -229,11 +237,7 @@ describe "Polls" do end scenario "Level 1 users" do - visit polls_path - expect(page).not_to have_css ".already-answer" - poll.update!(geozone_restricted_to: [geozone]) - create(:poll_question, :yes_no, poll: poll) login_as(create(:user, geozone: geozone)) From deb8b374e7cd58dea64508a54973de2892ec50fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 2 Mar 2024 23:32:21 +0100 Subject: [PATCH 3/4] Simplify creating a proposal poll in a test This way we avoid variables starting with underscores, which we don't use almost anywhere else. --- spec/system/polls/polls_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/system/polls/polls_spec.rb b/spec/system/polls/polls_spec.rb index ace5c4140..07880bf99 100644 --- a/spec/system/polls/polls_spec.rb +++ b/spec/system/polls/polls_spec.rb @@ -55,11 +55,11 @@ describe "Polls" do end scenario "Proposal polls won't be listed" do - proposal = create(:proposal) - _poll = create(:poll, related: proposal) + create(:poll, related: create(:proposal)) visit polls_path - expect(page).to have_content("There are no open votings") + + expect(page).to have_content "There are no open votings" end scenario "Filtering polls" do From 8ba37b295a7a6fe0fe0b584b089751774768f666 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 18 Mar 2024 15:09:17 +0100 Subject: [PATCH 4/4] Temporarily disable a test that fails sometimes This test is failing often due to an "Unable to autoload constant" error, that will be fixed after switching to zeitwerk. Just like it happened in the the "Polls can be listed" test, the reason seems to be accessing a page containing several ActiveStorage attachments. However, since this test only makes sense when two or more images are displayed on the page, we can't change the test so create just one image. So, for now, we're commenting the test, and we'll uncomment it again when we enable zeitwerk in version 2.2.0. --- spec/system/polls/polls_spec.rb | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/spec/system/polls/polls_spec.rb b/spec/system/polls/polls_spec.rb index 07880bf99..bf00eb20a 100644 --- a/spec/system/polls/polls_spec.rb +++ b/spec/system/polls/polls_spec.rb @@ -210,23 +210,24 @@ describe "Polls" do expect("Second question").to appear_before("Third question") end - scenario "Buttons to slide through images work back and forth" do - question = create(:poll_question, :yes_no, poll: poll) - create(:image, imageable: question.question_answers.last, title: "The no movement") - create(:image, imageable: question.question_answers.last, title: "No movement planning") + # TODO: uncomment after switching to zeitwerk + # scenario "Buttons to slide through images work back and forth" do + # question = create(:poll_question, :yes_no, poll: poll) + # create(:image, imageable: question.question_answers.last, title: "The no movement") + # create(:image, imageable: question.question_answers.last, title: "No movement planning") - visit poll_path(poll) + # visit poll_path(poll) - within(".orbit-bullets") do - find("[data-slide='1']").click + # within(".orbit-bullets") do + # find("[data-slide='1']").click - expect(page).to have_css ".is-active[data-slide='1']" + # expect(page).to have_css ".is-active[data-slide='1']" - find("[data-slide='0']").click + # find("[data-slide='0']").click - expect(page).to have_css ".is-active[data-slide='0']" - end - end + # expect(page).to have_css ".is-active[data-slide='0']" + # end + # end scenario "Non-logged in users" do create(:poll_question, :yes_no, poll: poll)