From 8386a07a7b6bd2c03ce102ae9c780747e4507569 Mon Sep 17 00:00:00 2001 From: Mantas Masalskis Date: Tue, 22 Oct 2019 11:40:35 +0200 Subject: [PATCH] Refactoring: Prefer RSpec matchers over selectors for expressive output and isolated scope. --- spec/support/capybara/kb_matchers.rb | 20 ------- spec/support/capybara/selectors.rb | 15 ------ .../knowledge_base_public/have_breadcrumb.rb | 52 ++++++++++++++++++ .../have_breadcrumb_item.rb | 53 +++++++++++++++++++ .../knowledge_base_public/have_editor_bar.rb | 16 ++++++ .../have_language_banner.rb | 16 ++++++ .../produce_search_result_for.rb | 30 +++++++++++ .../editor_search_spec.rb | 6 +-- .../knowledge_base_public/editor_spec.rb | 2 +- .../guest_search_spec.rb | 6 +-- .../knowledge_base_public/guest_spec.rb | 24 ++++----- 11 files changed, 186 insertions(+), 54 deletions(-) delete mode 100644 spec/support/capybara/kb_matchers.rb create mode 100644 spec/support/custom_matchers/system/knowledge_base_public/have_breadcrumb.rb create mode 100644 spec/support/custom_matchers/system/knowledge_base_public/have_breadcrumb_item.rb create mode 100644 spec/support/custom_matchers/system/knowledge_base_public/have_editor_bar.rb create mode 100644 spec/support/custom_matchers/system/knowledge_base_public/have_language_banner.rb create mode 100644 spec/support/custom_matchers/system/knowledge_base_public/produce_search_result_for.rb diff --git a/spec/support/capybara/kb_matchers.rb b/spec/support/capybara/kb_matchers.rb deleted file mode 100644 index 5e0730109..000000000 --- a/spec/support/capybara/kb_matchers.rb +++ /dev/null @@ -1,20 +0,0 @@ -RSpec::Matchers.define :allow_to_search_for do |expected| - match do |actual| - search_string = expected.translation.title - - actual.find('.js-search-input').fill_in with: search_string - actual.find('.search-results').has_text? search_string - end - - description do - "allows to search for \"#{expected.translation.title}\"" - end - - failure_message do - "could not search for \"#{expected.translation.title}\"" - end - - failure_message do - "\"#{expected.translation.title}\" showed up in search results" - end -end diff --git a/spec/support/capybara/selectors.rb b/spec/support/capybara/selectors.rb index e1ad1879d..cc55290b0 100644 --- a/spec/support/capybara/selectors.rb +++ b/spec/support/capybara/selectors.rb @@ -39,18 +39,3 @@ end Capybara.add_selector(:link_containing) do xpath { |text| ".//a//*[text()[contains(.,\"#{text}\")]]" } end - -# Knowledge Base -Capybara.add_selector(:knowledge_base_editor_bar) do - css { '.topbar' } -end - -Capybara.add_selector(:knowledge_base_language_banner) do - css { '.language-banner' } -end - -# don't use "knowledge_base" prefix because breadcrumbs -# could be available in other contexts (in the future) -Capybara.add_selector(:breadcrumb) do - css { '.breadcrumbs .breadcrumb span' } -end diff --git a/spec/support/custom_matchers/system/knowledge_base_public/have_breadcrumb.rb b/spec/support/custom_matchers/system/knowledge_base_public/have_breadcrumb.rb new file mode 100644 index 000000000..dec557170 --- /dev/null +++ b/spec/support/custom_matchers/system/knowledge_base_public/have_breadcrumb.rb @@ -0,0 +1,52 @@ +module KnowledgeBasePublicMatchers + module HaveBreadcrumb + extend RSpec::Matchers::DSL + + matcher :have_breadcrumb do + match { breadcrumb_found? && of_specified_length? } + + chain(:with, :length) + chain(:items) {} + + description do + if @length.present? + "have #{@length}-item breadcrumb" + else + 'have breadcrumb' + end + end + + failure_message do + if breadcrumb_found? && !of_specified_length? + "expected breadcrumb to contain #{@length} items (#{actual_length} found)" + else + 'expected to find breadcrumb, but none was found' + end + end + + failure_message_when_negated do + if breadcrumb_found? && @length.present? + "expected breadcrumb not to contain #{@length} items" + else + 'expected not to find breadcrumb, but did' + end + end + + def breadcrumb_found? + actual.has_css?('.breadcrumbs') + end + + def of_specified_length? + @length.nil? || @length == actual_length + end + + def actual_length + actual.all('.breadcrumbs .breadcrumb').length + end + end + end +end + +RSpec.configure do |config| + config.include KnowledgeBasePublicMatchers::HaveBreadcrumb, type: :system +end diff --git a/spec/support/custom_matchers/system/knowledge_base_public/have_breadcrumb_item.rb b/spec/support/custom_matchers/system/knowledge_base_public/have_breadcrumb_item.rb new file mode 100644 index 000000000..406c68a12 --- /dev/null +++ b/spec/support/custom_matchers/system/knowledge_base_public/have_breadcrumb_item.rb @@ -0,0 +1,53 @@ +module KnowledgeBasePublicMatchers + module HaveBreadcrumbItem + extend RSpec::Matchers::DSL + + matcher :have_breadcrumb_item do |expected| + match { breadcrumb_item_found? && at_specified_index? } + + chain(:at_index, :index) + + description do + if @index.present? + %(have "#{expected}" in breadcrumb at index #{@index}) + else + %(have "#{expected}" in breadcrumb) + end + end + + failure_message do + if breadcrumb_item_found? && !at_specified_index? + %(expected to find "#{expected}" at index #{@index} of breadcrumb (found at #{breadcrumb_item_index})) + else + %(expected to find "#{expected}" in breadcrumb, but did not) + end + end + + failure_message_when_negated do + if breadcrumb_item_found? && @index.present? + %(expected not to find "#{expected}" at index #{@index} of breadcrumb) + else + %(expected not to find "#{expected}" in breadcrumb, but did) + end + end + + def breadcrumb_item_found? + !breadcrumb_item_index.nil? + end + + def at_specified_index? + @index.nil? || @index == breadcrumb_item_index + end + + def breadcrumb_item_index + @breadcrumb_item_index ||= actual.all('.breadcrumbs .breadcrumb').index do |item| + item.find('span').text == expected + end + end + end + end +end + +RSpec.configure do |config| + config.include KnowledgeBasePublicMatchers::HaveBreadcrumbItem, type: :system +end diff --git a/spec/support/custom_matchers/system/knowledge_base_public/have_editor_bar.rb b/spec/support/custom_matchers/system/knowledge_base_public/have_editor_bar.rb new file mode 100644 index 000000000..aa15f263a --- /dev/null +++ b/spec/support/custom_matchers/system/knowledge_base_public/have_editor_bar.rb @@ -0,0 +1,16 @@ +module KnowledgeBasePublicMatchers + module HaveEditorBar + extend RSpec::Matchers::DSL + + matcher :have_editor_bar do + match { actual.has_css? '.topbar' } + description { 'display editor bar' } + failure_message { 'expected to find editor bar above header, but did not' } + failure_message_when_negated { 'expected not to find editor bar above header, but did' } + end + end +end + +RSpec.configure do |config| + config.include KnowledgeBasePublicMatchers::HaveEditorBar, type: :system +end diff --git a/spec/support/custom_matchers/system/knowledge_base_public/have_language_banner.rb b/spec/support/custom_matchers/system/knowledge_base_public/have_language_banner.rb new file mode 100644 index 000000000..aae9fbc98 --- /dev/null +++ b/spec/support/custom_matchers/system/knowledge_base_public/have_language_banner.rb @@ -0,0 +1,16 @@ +module KnowledgeBasePublicMatchers + module HaveLanguageBanner + extend RSpec::Matchers::DSL + + matcher :have_language_banner do + match { actual.has_css? '.language-banner' } + description { 'display language banner' } + failure_message { 'expected to find language banner, but did not' } + failure_message_when_negated { 'expected not to find language banner, but did' } + end + end +end + +RSpec.configure do |config| + config.include KnowledgeBasePublicMatchers::HaveLanguageBanner, type: :system +end diff --git a/spec/support/custom_matchers/system/knowledge_base_public/produce_search_result_for.rb b/spec/support/custom_matchers/system/knowledge_base_public/produce_search_result_for.rb new file mode 100644 index 000000000..130b31770 --- /dev/null +++ b/spec/support/custom_matchers/system/knowledge_base_public/produce_search_result_for.rb @@ -0,0 +1,30 @@ +module KnowledgeBasePublicMatchers + module ProduceSearchResultFor + extend RSpec::Matchers::DSL + + matcher :produce_search_result_for do |expected| + match do |actual| + search_string = expected.translation.title + + actual.find('.js-search-input').fill_in with: search_string + actual.find('.search-results').has_text? search_string + end + + description do + %(allows to search for "#{expected.translation.title}") + end + + failure_message do + %(could not search for "#{expected.translation.title}") + end + + failure_message do + %("#{expected.translation.title}" showed up in search results) + end + end + end +end + +RSpec.configure do |config| + config.include KnowledgeBasePublicMatchers::ProduceSearchResultFor, type: :system +end diff --git a/spec/system/knowledge_base_public/editor_search_spec.rb b/spec/system/knowledge_base_public/editor_search_spec.rb index 11d39e16f..3c4f09d89 100644 --- a/spec/system/knowledge_base_public/editor_search_spec.rb +++ b/spec/system/knowledge_base_public/editor_search_spec.rb @@ -17,14 +17,14 @@ RSpec.describe 'Public Knowledge Base for guest search', type: :system, authenti end it 'list published article' do - expect(page).to allow_to_search_for published_answer + expect(page).to produce_search_result_for published_answer end it 'list draft article' do - expect(page).to allow_to_search_for draft_answer + expect(page).to produce_search_result_for draft_answer end it 'list internal article' do - expect(page).to allow_to_search_for internal_answer + expect(page).to produce_search_result_for internal_answer end end diff --git a/spec/system/knowledge_base_public/editor_spec.rb b/spec/system/knowledge_base_public/editor_spec.rb index 3edc09f22..f28388216 100644 --- a/spec/system/knowledge_base_public/editor_spec.rb +++ b/spec/system/knowledge_base_public/editor_spec.rb @@ -10,7 +10,7 @@ RSpec.describe 'Public Knowledge Base for editor', type: :system, authenticated: context 'homepage' do before { visit help_no_locale_path } - it { expect(page).to have_selector(:knowledge_base_editor_bar) } + it { expect(page).to have_editor_bar } it 'expect to have edit button' do button = find '.topbar-btn' diff --git a/spec/system/knowledge_base_public/guest_search_spec.rb b/spec/system/knowledge_base_public/guest_search_spec.rb index fc1e3e290..09e3c7a9c 100644 --- a/spec/system/knowledge_base_public/guest_search_spec.rb +++ b/spec/system/knowledge_base_public/guest_search_spec.rb @@ -17,14 +17,14 @@ RSpec.describe 'Public Knowledge Base for guest search', type: :system, authenti end it 'list published article' do - expect(page).to allow_to_search_for published_answer + expect(page).to produce_search_result_for published_answer end it 'not list draft article' do - expect(page).not_to allow_to_search_for draft_answer + expect(page).not_to produce_search_result_for draft_answer end it 'not list internal article' do - expect(page).not_to allow_to_search_for internal_answer + expect(page).not_to produce_search_result_for internal_answer end end diff --git a/spec/system/knowledge_base_public/guest_spec.rb b/spec/system/knowledge_base_public/guest_spec.rb index f949e7838..c3368528d 100644 --- a/spec/system/knowledge_base_public/guest_spec.rb +++ b/spec/system/knowledge_base_public/guest_spec.rb @@ -12,8 +12,8 @@ RSpec.describe 'Public Knowledge Base for guest', type: :system, authenticated: it('is redirected to primary locale') { expect(page).to have_current_path help_root_path(primary_locale.system_locale.locale) } - it { expect(page).not_to have_selector(:breadcrumb) } - it { expect(page).not_to have_selector(:knowledge_base_editor_bar) } + it { expect(page).not_to have_breadcrumb } + it { expect(page).not_to have_editor_bar } it 'shows category' do within '.main' do @@ -31,7 +31,7 @@ RSpec.describe 'Public Knowledge Base for guest', type: :system, authenticated: context 'category' do before { visit help_category_path(primary_locale.system_locale.locale, category) } - it { expect(page).to have_selector(:breadcrumb) } + it { expect(page).to have_breadcrumb } it 'shows published answer' do within '.main' do @@ -52,9 +52,9 @@ RSpec.describe 'Public Knowledge Base for guest', type: :system, authenticated: end context 'breadcrumb' do - it { expect(page).to have_selector(:breadcrumb, count: 2) } - it { expect(page.all(:breadcrumb)[0]).to have_text(knowledge_base.translation.title) } - it { expect(page.all(:breadcrumb)[1]).to have_text(category.translation.title) } + it { expect(page).to have_breadcrumb.with(2).items } + it { expect(page).to have_breadcrumb_item(knowledge_base.translation.title).at_index(0) } + it { expect(page).to have_breadcrumb_item(category.translation.title).at_index(1) } end end @@ -62,17 +62,17 @@ RSpec.describe 'Public Knowledge Base for guest', type: :system, authenticated: before { visit help_answer_path(primary_locale.system_locale.locale, category, published_answer) } context 'breadcrumb' do - it { expect(page).to have_selector(:breadcrumb, count: 3) } - it { expect(page.all(:breadcrumb)[0]).to have_text(knowledge_base.translation.title) } - it { expect(page.all(:breadcrumb)[1]).to have_text(category.translation.title) } - it { expect(page.all(:breadcrumb)[2]).to have_text(published_answer.translation.title) } + it { expect(page).to have_breadcrumb.with(3).items } + it { expect(page).to have_breadcrumb_item(knowledge_base.translation.title).at_index(0) } + it { expect(page).to have_breadcrumb_item(category.translation.title).at_index(1) } + it { expect(page).to have_breadcrumb_item(published_answer.translation.title).at_index(2) } end end context 'wrong locale' do before { visit help_root_path(alternative_locale.system_locale.locale) } - it { expect(page).to have_selector(:knowledge_base_language_banner) } + it { expect(page).to have_language_banner } context 'switch to correct locale after clicking on language banner' do before do @@ -81,7 +81,7 @@ RSpec.describe 'Public Knowledge Base for guest', type: :system, authenticated: end end - it { expect(page).not_to have_selector(:knowledge_base_language_banner) } + it { expect(page).not_to have_language_banner } end end