Use if instead of skip to skip tests

This way the tests won't appear as "pending" when running the test
suite, and so we get rid of a lot of noise in the test results. There
doesn't seem to be a way to call `skip` without the test being marked as
"pending".

Note that in the globalizable tests we need to build a factory before
deciding whether an atribute is required or not (particularly for the
milestone factory, since milestone attributes are required depending on
the presence of other attributes). This isn't possible before we're
inside the test, so we can't add an `if:` condition to the test. So
we're adding the condition inside the test instead. A minor
inconvenience of this method is the test still runs even when the
condition is `false`.

spec/models/concerns/globalizable.rb

@@ -47,15 +47,15 @@ shared_examples_for "globalizable" do |factory_name|
     end
 
     it "Does not create invalid translations in the database" do
-      skip("cannot have invalid translations") if required_fields.empty?
+      if required_fields.any?
+        record.update(translations_attributes: [{ locale: :fr }])
 
-      record.update(translations_attributes: [{ locale: :fr }])
+        expect(record.translations.map(&:locale)).to match_array %i[en es fr]
 
-      expect(record.translations.map(&:locale)).to match_array %i[en es fr]
+        record.reload
 
-      record.reload
+        expect(record.translations.map(&:locale)).to match_array %i[en es]
-
+      end
-      expect(record.translations.map(&:locale)).to match_array %i[en es]
     end
 
     it "Does not automatically add a translation for the current locale" do
@@ -84,17 +84,17 @@ shared_examples_for "globalizable" do |factory_name|
     end
 
     it "Does not save invalid translations" do
-      skip("cannot have invalid translations") if required_fields.empty?
+      if required_fields.any?
+        record.update(translations_attributes: [
+          { id: record.translations.find_by(locale: :es).id, attribute => "" }
+        ])
 
-      record.update(translations_attributes: [
+        I18n.with_locale(:es) { expect(record.send(attribute)).to eq "" }
-        { id: record.translations.find_by(locale: :es).id, attribute => "" }
-      ])
 
-      I18n.with_locale(:es) { expect(record.send(attribute)).to eq "" }
+        record.reload
 
-      record.reload
+        I18n.with_locale(:es) { expect(record.send(attribute)).to eq "En español" }
-
+      end
-      I18n.with_locale(:es) { expect(record.send(attribute)).to eq "En español" }
     end
 
     it "Does not automatically add a translation for the current locale" do
@@ -122,33 +122,33 @@ shared_examples_for "globalizable" do |factory_name|
     end
 
     it "Does not remove all translations" do
-      skip("cannot have invalid translations") if required_fields.empty?
+      if required_fields.any?
+        record.translations_attributes = [
+          { id: record.translations.find_by(locale: :en).id, _destroy: true },
+          { id: record.translations.find_by(locale: :es).id, _destroy: true }
+        ]
 
-      record.translations_attributes = [
+        expect(record).not_to be_valid
-        { id: record.translations.find_by(locale: :en).id, _destroy: true },
-        { id: record.translations.find_by(locale: :es).id, _destroy: true }
-      ]
 
-      expect(record).not_to be_valid
+        record.reload
 
-      record.reload
+        expect(record.translations.map(&:locale)).to match_array %i[en es]
-
+      end
-      expect(record.translations.map(&:locale)).to match_array %i[en es]
     end
 
     it "Does not remove translations when there's invalid data" do
-      skip("cannot have invalid translations") if required_fields.empty?
+      if required_fields.any?
+        record.translations_attributes = [
+          { id: record.translations.find_by(locale: :es).id, attribute => "" },
+          { id: record.translations.find_by(locale: :en).id, _destroy: true }
+        ]
 
-      record.translations_attributes = [
+        expect(record).not_to be_valid
-        { id: record.translations.find_by(locale: :es).id, attribute => "" },
-        { id: record.translations.find_by(locale: :en).id, _destroy: true }
-      ]
 
-      expect(record).not_to be_valid
+        record.reload
 
-      record.reload
+        expect(record.translations.map(&:locale)).to match_array %i[en es]
-
+      end
-      expect(record.translations.map(&:locale)).to match_array %i[en es]
     end
   end
 

spec/shared/system/flaggable.rb

@@ -91,9 +91,7 @@ shared_examples "flaggable" do |factory_name, admin: false|
     end
   end
 
-  scenario "Flagging a comment with a child does not update its children" do
+  scenario "Flagging a comment with a child does not update its children", if: factory_name =~ /comment/ do
-    skip "Only for comments" unless flaggable.is_a?(Comment)
-
     child_comment = create(:comment, commentable: flaggable.commentable, parent: flaggable)
 
     login_as(user)

spec/shared/system/mappable.rb

@@ -173,9 +173,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
     end
   end
 
-  describe "At #{mappable_edit_path}" do
+  describe "At #{mappable_edit_path}", if: mappable_edit_path.present? do
-    before { skip } if mappable_edit_path.blank?
-
     scenario "Should edit map on #{mappable_factory_name} and contain default values" do
       do_login_for mappable.author, management: management
 

spec/shared/system/nested_documentable.rb

@@ -201,10 +201,8 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na
       expect(page).to have_content documentable_success_notice
     end
 
-    scenario "Should show new document after successful creation with one uploaded file" do
+    scenario "Should show new document after successful creation with one uploaded file",
-      if documentable_factory_name == "dashboard_action"
+             unless: documentable_factory_name == "dashboard_action" do
-        skip("Not render Documents count on dashboard_actions")
-      end
       do_login_for user_to_login, management: management
       visit send(path, arguments)
       send(fill_resource_method_name) if fill_resource_method_name
@@ -223,10 +221,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na
     end
 
     scenario "Should show resource with new document after successful creation with
-              maximum allowed uploaded files" do
+              maximum allowed uploaded files", unless: documentable_factory_name == "dashboard_action" do
-      if documentable_factory_name == "dashboard_action"
-        skip("Not render Documents count on dashboard_actions")
-      end
       do_login_for user_to_login, management: management
       visit send(path, arguments)
 

spec/shared/system/nested_imageable.rb

@@ -128,8 +128,8 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p
       end
     end
 
-    scenario "Render image preview after sending the form with validation errors" do
+    scenario "Render image preview after sending the form with validation errors",
-      skip "Question answers behave differently" if imageable.is_a?(Poll::Question::Answer)
+             unless: imageable_factory_name == "poll_question_answer" do
       do_login_for user, management: management
       visit send(path, arguments)
 
@@ -154,17 +154,14 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p
       expect(page).not_to have_selector("#nested-image .image")
     end
 
-    scenario "Should show successful notice when resource filled correctly without any nested images" do
+    scenario "Should show successful notice when resource filled correctly without any nested images",
-      if has_many_images
+             unless: has_many_images do
-        skip "no need to test, there are no attributes for the parent resource"
+      do_login_for user, management: management
-      else
+      visit send(path, arguments)
-        do_login_for user, management: management
-        visit send(path, arguments)
 
-        send(fill_resource_method_name) if fill_resource_method_name
+      send(fill_resource_method_name) if fill_resource_method_name
-        click_on submit_button
+      click_on submit_button
-        expect(page).to have_content imageable_success_notice
+      expect(page).to have_content imageable_success_notice
-      end
     end
 
     scenario "Should show successful notice when resource filled correctly and after valid file uploads" do

spec/shared/system/remotely_translatable.rb

@@ -38,8 +38,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume
       expect(page).not_to have_button("Traducir página")
     end
 
-    scenario "should not be present when there are no resources to translate" do
+    scenario "should not be present when there are no resources to translate", if: index_path?(path_name) do
-      skip("only index_path") if show_path?(path_name)
       resource.destroy!
       visit path
 
@@ -60,13 +59,8 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume
       end
     end
 
-    describe "should ignore missing translations on resource comments" do
+    describe "should ignore missing translations on resource comments",
-      before do
+             if: index_path?(path_name) && commentable?(factory_name) do
-        if show_path?(path_name) || !commentable?(resource)
-          skip("only index_path")
-        end
-      end
-
       scenario "is not present when a resource translation exists but its comment has not tanslations" do
         add_translations(resource, :es)
         create(:comment, commentable: resource)
@@ -79,13 +73,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume
       end
     end
 
-    describe "should evaluate missing translations on resource comments" do
+    describe "should evaluate missing translations on resource comments", if: show_path?(path_name) do
-      before do
-        if index_path?(path_name)
-          skip("only show_path")
-        end
-      end
-
       scenario "display when exists resource translations but the comment does not have a translation" do
         add_translations(resource, :es)
         create(:comment, commentable: resource)
@@ -109,9 +97,7 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume
       end
     end
 
-    describe "should evaluate missing translations on featured_debates" do
+    describe "should evaluate missing translations on featured_debates", if: path_name == "debates_path" do
-      before { skip("only debates index path") if path_name != "debates_path" }
-
       scenario "display when exists featured_debates without tanslations" do
         add_translations(resource, :es)
         create_featured_debates
@@ -124,9 +110,8 @@ shared_examples "remotely_translatable" do |factory_name, path_name, path_argume
       end
     end
 
-    describe "should evaluate missing translations on featured_proposals" do
+    describe "should evaluate missing translations on featured_proposals",
-      before { skip("only proposals index path") if path_name != "proposals_path" }
+             if: path_name == "proposals_path" do
-
       scenario "display when exists featured_proposals without tanslations" do
         add_translations(resource, :es)
         create_featured_proposals
@@ -230,8 +215,8 @@ def show_path?(path)
   !index_path?(path)
 end
 
-def commentable?(resource)
+def commentable?(factory_name)
-  Comment::COMMENTABLE_TYPES.include?(resource.class.to_s)
+  Comment::COMMENTABLE_TYPES.include?(FactoryBot.factories[factory_name].build_class.to_s)
 end
 
 def generate_response(resource)

spec/system/admin/poll/questions_spec.rb

@@ -167,11 +167,8 @@ describe "Admin poll questions", :admin do
                                   options: ["Seleccionar votación", poll.name_es])
     end
 
-    scenario "uses fallback if name is not translated to current locale" do
+    scenario "uses fallback if name is not translated to current locale",
-      unless globalize_french_fallbacks.first == :es
+             if: Globalize.fallbacks(:fr).reject { |locale| locale.match(/fr/) }.first == :es do
-        skip("Spec only useful when French falls back to Spanish")
-      end
-
       poll = create(:poll, name_en: "Name in English", name_es: "Nombre en Español")
       proposal = create(:proposal)
 
@@ -186,8 +183,4 @@ describe "Admin poll questions", :admin do
                                   options: ["Sélectionner un vote", poll.name_es])
     end
   end
-
-  def globalize_french_fallbacks
-    Globalize.fallbacks(:fr).reject { |locale| locale.match(/fr/) }
-  end
 end