From 6088334dbf32835ddaed02eae72dbfa6dd5fc055 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Tue, 13 Oct 2020 18:11:39 +0200
Subject: [PATCH] Remove redundant visibility matcher usages

By default, Capybara only finds visible elements, so adding the
`visible: true` option is usually redundant.

We were using it sometimes to make it an obvious contrast with another
test using `visible: false`. However, from the user's perspective, we
don't care whether the element has been removed from the DOM or has been
hidden, so we can just test that the visible selector can't be found.

Besides, using `visible: false` means the test will also pass if the
element is present and visible. However, we want the test to fail if the
element is visible. That's why a couple of JavaScript-dependant tests
were passing even when JavaScript was disabled.
---
 spec/shared/system/nested_documentable.rb          | 14 +++++++-------
 spec/shared/system/nested_imageable.rb             | 10 +++++-----
 spec/shared/system/relationable.rb                 |  8 +++++---
 spec/support/common_actions/votes.rb               |  8 +++++---
 spec/system/admin/settings_spec.rb                 |  2 +-
 spec/system/budgets/ballots_spec.rb                |  4 ++--
 spec/system/comments/budget_investments_spec.rb    |  6 +++---
 .../comments/budget_investments_valuation_spec.rb  |  4 ++--
 spec/system/comments/debates_spec.rb               |  6 +++---
 .../comments/legislation_annotations_spec.rb       |  6 +++---
 spec/system/comments/legislation_questions_spec.rb |  6 +++---
 spec/system/comments/polls_spec.rb                 |  6 +++---
 spec/system/comments/proposals_spec.rb             |  6 +++---
 spec/system/comments/topics_spec.rb                | 12 ++++++------
 spec/system/home_spec.rb                           |  6 +++---
 15 files changed, 54 insertions(+), 50 deletions(-)

diff --git a/spec/shared/system/nested_documentable.rb b/spec/shared/system/nested_documentable.rb
index a44096f06..a347b1436 100644
--- a/spec/shared/system/nested_documentable.rb
+++ b/spec/shared/system/nested_documentable.rb
@@ -22,7 +22,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na
       login_as user_to_login
       visit send(path, arguments)
 
-      expect(page).to have_css "#new_document_link", visible: true
+      expect(page).to have_css "#new_document_link"
     end
 
     scenario "Should not show new document link when
@@ -34,14 +34,14 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na
         click_link "Add new document"
       end
 
-      expect(page).to have_css "#new_document_link", visible: false
+      expect(page).not_to have_css "#new_document_link"
     end
 
     scenario "Should not show max documents warning when no documents added", :js do
       login_as user_to_login
       visit send(path, arguments)
 
-      expect(page).to have_css ".max-documents-notice", visible: false
+      expect(page).not_to have_css ".max-documents-notice"
     end
 
     scenario "Should show max documents warning when max documents allowed limit is reached", :js do
@@ -51,7 +51,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na
         documentable_attach_new_file(Rails.root.join("spec/fixtures/files/empty.pdf"))
       end
 
-      expect(page).to have_css ".max-documents-notice", visible: true
+      expect(page).to have_css ".max-documents-notice"
       expect(page).to have_content "Remove document"
     end
 
@@ -65,7 +65,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na
 
       all("a", text: "Cancel").last.click
 
-      expect(page).to have_css ".max-documents-notice", visible: false
+      expect(page).not_to have_css ".max-documents-notice"
     end
 
     scenario "Should update nested document file name after choosing a file", :js do
@@ -263,7 +263,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na
         login_as user_to_login
         visit send(path, arguments)
 
-        expect(page).to have_css "#new_document_link", visible: false
+        expect(page).not_to have_css "#new_document_link"
       end
 
       scenario "Should show add document button after destroy one document", :js do
@@ -275,7 +275,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na
           click_on "Remove document"
         end
 
-        expect(page).to have_css "#new_document_link", visible: true
+        expect(page).to have_css "#new_document_link"
       end
 
       scenario "Should remove nested field after remove document", :js do
diff --git a/spec/shared/system/nested_imageable.rb b/spec/shared/system/nested_imageable.rb
index 6977e3e81..fa8e1a546 100644
--- a/spec/shared/system/nested_imageable.rb
+++ b/spec/shared/system/nested_imageable.rb
@@ -17,16 +17,16 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p
       login_as user
       visit send(path, arguments)
 
-      expect(page).to have_selector "#new_image_link", visible: true
+      expect(page).to have_selector "#new_image_link"
     end
 
-    scenario "Should hide new image link after adding one image" do
+    scenario "Should hide new image link after adding one image", :js do
       login_as user
       visit send(path, arguments)
 
       click_on "Add image"
 
-      expect(page).to have_selector "#new_image_link", visible: false
+      expect(page).not_to have_selector "#new_image_link"
     end
 
     scenario "Should update nested image file name after choosing any file", :js do
@@ -226,7 +226,7 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p
         login_as user
         visit send(path, arguments)
 
-        expect(page).to have_css "a#new_image_link", visible: false
+        expect(page).not_to have_css "a#new_image_link"
       end
 
       scenario "Should remove nested field after remove image", :js do
@@ -244,7 +244,7 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p
         visit send(path, arguments)
         click_on "Remove image"
 
-        expect(page).to have_css "a#new_image_link", visible: true
+        expect(page).to have_css "a#new_image_link"
       end
     end
   end
diff --git a/spec/shared/system/relationable.rb b/spec/shared/system/relationable.rb
index 094dc1873..bec05d1f2 100644
--- a/spec/shared/system/relationable.rb
+++ b/spec/shared/system/relationable.rb
@@ -23,13 +23,13 @@ shared_examples "relationable" do |relationable_model_name|
     expect(page).not_to have_css("#related-content-list")
   end
 
-  scenario "related contents can be added" do
+  scenario "related contents can be added", :js do
     login_as(user)
     visit relationable.url
 
-    expect(page).to have_selector("#related_content", visible: false)
+    expect(page).not_to have_selector("#related_content")
+
     click_on("Add related content")
-    expect(page).to have_selector("#related_content", visible: true)
 
     within("#related_content") do
       fill_in "url", with: "#{Setting["url"] + related1.url}"
@@ -46,6 +46,8 @@ shared_examples "relationable" do |relationable_model_name|
       expect(page).to have_content(relationable.title)
     end
 
+    click_on("Add related content")
+
     within("#related_content") do
       fill_in "url", with: "#{Setting["url"] + related2.url}"
       click_button "Add"
diff --git a/spec/support/common_actions/votes.rb b/spec/support/common_actions/votes.rb
index 2f917a154..37a9eb485 100644
--- a/spec/support/common_actions/votes.rb
+++ b/spec/support/common_actions/votes.rb
@@ -5,9 +5,11 @@ module Votes
   end
 
   def expect_message_you_need_to_sign_in_to_vote_comments
-    expect(page).to have_content "You must sign in or sign up to vote"
-    expect(page).to have_selector(".participation-allowed", visible: false)
-    expect(page).to have_selector(".participation-not-allowed", visible: true)
+    within(".participation-not-allowed") do
+      expect(page).to have_content "You must sign in or sign up to vote"
+    end
+
+    expect(page).not_to have_selector(".participation-allowed")
   end
 
   def expect_message_to_many_anonymous_votes
diff --git a/spec/system/admin/settings_spec.rb b/spec/system/admin/settings_spec.rb
index d9f418d05..8218c76ca 100644
--- a/spec/system/admin/settings_spec.rb
+++ b/spec/system/admin/settings_spec.rb
@@ -50,7 +50,7 @@ describe "Admin settings" do
 
       find("#map-tab").click
 
-      expect(page).to have_css("#admin-map.leaflet-container", visible: true)
+      expect(page).to have_css("#admin-map.leaflet-container")
     end
   end
 
diff --git a/spec/system/budgets/ballots_spec.rb b/spec/system/budgets/ballots_spec.rb
index 6e8648cb5..3db1a4cdf 100644
--- a/spec/system/budgets/ballots_spec.rb
+++ b/spec/system/budgets/ballots_spec.rb
@@ -690,11 +690,11 @@ describe "Ballots" do
         find(".in-favor a").click
 
         expect(page).not_to have_content "Remove"
-        expect(page).to have_selector(".participation-not-allowed", visible: false)
+        expect(page).not_to have_selector(".participation-not-allowed")
 
         hover_over_ballot
 
-        expect(page).to have_selector(".participation-not-allowed", visible: true)
+        expect(page).to have_selector(".participation-not-allowed")
         expect(page).to have_selector(".in-favor a", obscured: true)
       end
     end
diff --git a/spec/system/comments/budget_investments_spec.rb b/spec/system/comments/budget_investments_spec.rb
index 667d4a970..cffed554f 100644
--- a/spec/system/comments/budget_investments_spec.rb
+++ b/spec/system/comments/budget_investments_spec.rb
@@ -241,7 +241,7 @@ describe "Commenting Budget::Investments" do
       expect(page).to have_content "It will be done next week."
     end
 
-    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
   end
 
   scenario "Reply update parent comment responses count", :js do
@@ -357,7 +357,7 @@ describe "Commenting Budget::Investments" do
         expect(page).to have_css "img.moderator-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
 
     scenario "can not comment as an administrator" do
@@ -456,7 +456,7 @@ describe "Commenting Budget::Investments" do
           expect(page).to have_css "img.admin-avatar"
         end
 
-        expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+        expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
         expect(page).to have_css "div.is-admin"
       end
 
diff --git a/spec/system/comments/budget_investments_valuation_spec.rb b/spec/system/comments/budget_investments_valuation_spec.rb
index 87792acbd..cbc2e5813 100644
--- a/spec/system/comments/budget_investments_valuation_spec.rb
+++ b/spec/system/comments/budget_investments_valuation_spec.rb
@@ -210,7 +210,7 @@ describe "Internal valuation comments on Budget::Investments" do
         expect(page).to have_content "It will be done next week."
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
 
       visit budget_investment_path(investment.budget, investment)
       expect(page).not_to have_content("It will be done next week.")
@@ -328,7 +328,7 @@ describe "Internal valuation comments on Budget::Investments" do
         expect(page).to have_css "img.admin-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
   end
 
diff --git a/spec/system/comments/debates_spec.rb b/spec/system/comments/debates_spec.rb
index 48da9e79f..f05ac20fb 100644
--- a/spec/system/comments/debates_spec.rb
+++ b/spec/system/comments/debates_spec.rb
@@ -254,7 +254,7 @@ describe "Commenting debates" do
       expect(page).to have_content "It will be done next week."
     end
 
-    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
   end
 
   scenario "Reply to reply", :js do
@@ -408,7 +408,7 @@ describe "Commenting debates" do
         expect(page).to have_css "img.moderator-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
 
     scenario "can not comment as an administrator" do
@@ -464,7 +464,7 @@ describe "Commenting debates" do
         expect(page).to have_css "img.admin-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
 
     scenario "can not comment as a moderator" do
diff --git a/spec/system/comments/legislation_annotations_spec.rb b/spec/system/comments/legislation_annotations_spec.rb
index b6560f743..ad19b738c 100644
--- a/spec/system/comments/legislation_annotations_spec.rb
+++ b/spec/system/comments/legislation_annotations_spec.rb
@@ -277,7 +277,7 @@ describe "Commenting legislation questions" do
       expect(page).to have_content "It will be done next week."
     end
 
-    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
   end
 
   scenario "Reply update parent comment responses count", :js do
@@ -430,7 +430,7 @@ describe "Commenting legislation questions" do
         expect(page).to have_css "img.moderator-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
 
     scenario "can not comment as an administrator" do
@@ -493,7 +493,7 @@ describe "Commenting legislation questions" do
         expect(page).to have_css "img.admin-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
 
     scenario "can not comment as a moderator" do
diff --git a/spec/system/comments/legislation_questions_spec.rb b/spec/system/comments/legislation_questions_spec.rb
index 7ddf1e56d..ebb58a67f 100644
--- a/spec/system/comments/legislation_questions_spec.rb
+++ b/spec/system/comments/legislation_questions_spec.rb
@@ -258,7 +258,7 @@ describe "Commenting legislation questions" do
       expect(page).to have_content "It will be done next week."
     end
 
-    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
   end
 
   scenario "Reply update parent comment responses count", :js do
@@ -387,7 +387,7 @@ describe "Commenting legislation questions" do
         expect(page).to have_css "img.moderator-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
 
     scenario "can not comment as an administrator" do
@@ -443,7 +443,7 @@ describe "Commenting legislation questions" do
         expect(page).to have_css "img.admin-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
 
     scenario "can not comment as a moderator" do
diff --git a/spec/system/comments/polls_spec.rb b/spec/system/comments/polls_spec.rb
index 1df15f3e2..bcbbd02a1 100644
--- a/spec/system/comments/polls_spec.rb
+++ b/spec/system/comments/polls_spec.rb
@@ -237,7 +237,7 @@ describe "Commenting polls" do
       expect(page).to have_content "It will be done next week."
     end
 
-    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
   end
 
   scenario "Reply update parent comment responses count", :js do
@@ -357,7 +357,7 @@ describe "Commenting polls" do
         expect(page).to have_css "img.moderator-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
 
     scenario "can not comment as an administrator" do
@@ -419,7 +419,7 @@ describe "Commenting polls" do
         expect(page).to have_css "img.admin-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
 
     scenario "can not comment as a moderator" do
diff --git a/spec/system/comments/proposals_spec.rb b/spec/system/comments/proposals_spec.rb
index 89fa11b08..4a1177e43 100644
--- a/spec/system/comments/proposals_spec.rb
+++ b/spec/system/comments/proposals_spec.rb
@@ -237,7 +237,7 @@ describe "Commenting proposals" do
       expect(page).to have_content "It will be done next week."
     end
 
-    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
   end
 
   scenario "Reply update parent comment responses count", :js do
@@ -353,7 +353,7 @@ describe "Commenting proposals" do
         expect(page).to have_css "img.moderator-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
 
     scenario "can not comment as an administrator" do
@@ -409,7 +409,7 @@ describe "Commenting proposals" do
         expect(page).to have_css "img.admin-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
 
     scenario "can not comment as a moderator" do
diff --git a/spec/system/comments/topics_spec.rb b/spec/system/comments/topics_spec.rb
index c7537814c..94e2e481a 100644
--- a/spec/system/comments/topics_spec.rb
+++ b/spec/system/comments/topics_spec.rb
@@ -263,7 +263,7 @@ describe "Commenting topics from proposals" do
       expect(page).to have_content "It will be done next week."
     end
 
-    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
   end
 
   scenario "Reply update parent comment responses count", :js do
@@ -393,7 +393,7 @@ describe "Commenting topics from proposals" do
         expect(page).to have_css "img.moderator-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
 
     scenario "can not comment as an administrator" do
@@ -455,7 +455,7 @@ describe "Commenting topics from proposals" do
         expect(page).to have_css "img.admin-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
 
     scenario "can not comment as a moderator" do
@@ -807,7 +807,7 @@ describe "Commenting topics from budget investments" do
       expect(page).to have_content "It will be done next week."
     end
 
-    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+    expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
   end
 
   scenario "Errors on reply", :js do
@@ -901,7 +901,7 @@ describe "Commenting topics from budget investments" do
         expect(page).to have_css "img.moderator-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
 
     scenario "can not comment as an administrator" do
@@ -963,7 +963,7 @@ describe "Commenting topics from budget investments" do
         expect(page).to have_css "img.admin-avatar"
       end
 
-      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}", visible: true)
+      expect(page).not_to have_selector("#js-comment-form-comment_#{comment.id}")
     end
 
     scenario "can not comment as a moderator" do
diff --git a/spec/system/home_spec.rb b/spec/system/home_spec.rb
index 90b12212f..027c94cba 100644
--- a/spec/system/home_spec.rb
+++ b/spec/system/home_spec.rb
@@ -115,7 +115,7 @@ describe "Home" do
       )
 
       visit root_path
-      expect(page).to have_xpath(ie_alert_box_xpath, visible: false)
+      expect(page).to have_xpath(ie_alert_box_xpath)
       expect(page.driver.request.cookies["ie_alert_closed"]).to be_nil
 
       # faking close button, since a normal find and click
@@ -123,13 +123,13 @@ describe "Home" do
       page.driver.browser.set_cookie("ie_alert_closed=true")
 
       visit root_path
-      expect(page).not_to have_xpath(ie_alert_box_xpath, visible: false)
+      expect(page).not_to have_xpath(ie_alert_box_xpath)
       expect(page.driver.request.cookies["ie_alert_closed"]).to eq("true")
     end
 
     scenario "non-IE visitors are not bothered with IE alerts", :page_driver do
       visit root_path
-      expect(page).not_to have_xpath(ie_alert_box_xpath, visible: false)
+      expect(page).not_to have_xpath(ie_alert_box_xpath)
       expect(page.driver.request.cookies["ie_alert_closed"]).to be_nil
     end