From e49c32638d9903202f9ec540379e2c20ad35b72e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 8 Sep 2021 03:24:15 +0200 Subject: [PATCH] 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 | 64 ++++++++++----------- spec/shared/system/flaggable.rb | 4 +- spec/shared/system/mappable.rb | 4 +- spec/shared/system/nested_documentable.rb | 11 +--- spec/shared/system/nested_imageable.rb | 21 +++---- spec/shared/system/remotely_translatable.rb | 33 +++-------- spec/system/admin/poll/questions_spec.rb | 11 +--- 7 files changed, 57 insertions(+), 91 deletions(-) diff --git a/spec/models/concerns/globalizable.rb b/spec/models/concerns/globalizable.rb index 8b6555b87..54135137c 100644 --- a/spec/models/concerns/globalizable.rb +++ b/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] + expect(record.translations.map(&:locale)).to match_array %i[en es] + end 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: [ - { id: record.translations.find_by(locale: :es).id, attribute => "" } - ]) + I18n.with_locale(:es) { expect(record.send(attribute)).to eq "" } - 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" } + I18n.with_locale(:es) { expect(record.send(attribute)).to eq "En español" } + end 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 = [ - { 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 - expect(record).not_to be_valid + record.reload - record.reload - - expect(record.translations.map(&:locale)).to match_array %i[en es] + expect(record.translations.map(&:locale)).to match_array %i[en es] + end 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 = [ - { 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 - expect(record).not_to be_valid + record.reload - record.reload - - expect(record.translations.map(&:locale)).to match_array %i[en es] + expect(record.translations.map(&:locale)).to match_array %i[en es] + end end end diff --git a/spec/shared/system/flaggable.rb b/spec/shared/system/flaggable.rb index 3b5ad64ef..b40f3c480 100644 --- a/spec/shared/system/flaggable.rb +++ b/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 - skip "Only for comments" unless flaggable.is_a?(Comment) - + scenario "Flagging a comment with a child does not update its children", if: factory_name =~ /comment/ do child_comment = create(:comment, commentable: flaggable.commentable, parent: flaggable) login_as(user) diff --git a/spec/shared/system/mappable.rb b/spec/shared/system/mappable.rb index d73620697..b1551a47a 100644 --- a/spec/shared/system/mappable.rb +++ b/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 - before { skip } if mappable_edit_path.blank? - + describe "At #{mappable_edit_path}", if: mappable_edit_path.present? do scenario "Should edit map on #{mappable_factory_name} and contain default values" do do_login_for mappable.author, management: management diff --git a/spec/shared/system/nested_documentable.rb b/spec/shared/system/nested_documentable.rb index 46e6f9f83..dafc01796 100644 --- a/spec/shared/system/nested_documentable.rb +++ b/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 - if documentable_factory_name == "dashboard_action" - skip("Not render Documents count on dashboard_actions") - end + scenario "Should show new document after successful creation with one uploaded file", + unless: documentable_factory_name == "dashboard_action" do 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 - if documentable_factory_name == "dashboard_action" - skip("Not render Documents count on dashboard_actions") - end + maximum allowed uploaded files", unless: documentable_factory_name == "dashboard_action" do do_login_for user_to_login, management: management visit send(path, arguments) diff --git a/spec/shared/system/nested_imageable.rb b/spec/shared/system/nested_imageable.rb index 626f16174..cbe7a48b3 100644 --- a/spec/shared/system/nested_imageable.rb +++ b/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 - skip "Question answers behave differently" if imageable.is_a?(Poll::Question::Answer) + scenario "Render image preview after sending the form with validation errors", + 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 - if has_many_images - skip "no need to test, there are no attributes for the parent resource" - else - do_login_for user, management: management - visit send(path, arguments) + scenario "Should show successful notice when resource filled correctly without any nested images", + unless: has_many_images do + do_login_for user, management: management + visit send(path, arguments) - send(fill_resource_method_name) if fill_resource_method_name - click_on submit_button - expect(page).to have_content imageable_success_notice - end + send(fill_resource_method_name) if fill_resource_method_name + click_on submit_button + expect(page).to have_content imageable_success_notice end scenario "Should show successful notice when resource filled correctly and after valid file uploads" do diff --git a/spec/shared/system/remotely_translatable.rb b/spec/shared/system/remotely_translatable.rb index 0b49ac745..c38678e46 100644 --- a/spec/shared/system/remotely_translatable.rb +++ b/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 - skip("only index_path") if show_path?(path_name) + scenario "should not be present when there are no resources to translate", if: index_path?(path_name) do 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 - before do - if show_path?(path_name) || !commentable?(resource) - skip("only index_path") - end - end - + describe "should ignore missing translations on resource comments", + if: index_path?(path_name) && commentable?(factory_name) do 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 - before do - if index_path?(path_name) - skip("only show_path") - end - end - + describe "should evaluate missing translations on resource comments", if: show_path?(path_name) do 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 - before { skip("only debates index path") if path_name != "debates_path" } - + describe "should evaluate missing translations on featured_debates", if: path_name == "debates_path" do 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 - before { skip("only proposals index path") if path_name != "proposals_path" } - + describe "should evaluate missing translations on featured_proposals", + 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) - Comment::COMMENTABLE_TYPES.include?(resource.class.to_s) +def commentable?(factory_name) + Comment::COMMENTABLE_TYPES.include?(FactoryBot.factories[factory_name].build_class.to_s) end def generate_response(resource) diff --git a/spec/system/admin/poll/questions_spec.rb b/spec/system/admin/poll/questions_spec.rb index 6819d520d..f7062ea67 100644 --- a/spec/system/admin/poll/questions_spec.rb +++ b/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 - unless globalize_french_fallbacks.first == :es - skip("Spec only useful when French falls back to Spanish") - end - + scenario "uses fallback if name is not translated to current locale", + if: Globalize.fallbacks(:fr).reject { |locale| locale.match(/fr/) }.first == :es do 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