From 8b70f5703cd6645de8ce7b28754c3399dd9119df Mon Sep 17 00:00:00 2001 From: Mantas Masalskis Date: Wed, 26 Aug 2020 14:31:33 +0200 Subject: [PATCH] Maintenance: Cop to replace `.not_to have_` to `.to have_no_` --- .rubocop/cop/zammad/have_no_over_not_to.rb | 62 +++++++++++++++++++ .rubocop/rubocop_zammad.rb | 1 + spec/system/channels/email_spec.rb | 2 +- .../knowledge_base_public/guest_spec.rb | 8 +-- .../knowledge_base_public/menu_items_spec.rb | 4 +- spec/system/login/message_spec.rb | 2 +- spec/system/profile/devices_spec.rb | 2 +- spec/system/ticket/create_spec.rb | 18 +++--- spec/system/ticket/update_spec.rb | 4 +- spec/system/ticket/zoom_spec.rb | 10 +-- 10 files changed, 88 insertions(+), 25 deletions(-) create mode 100644 .rubocop/cop/zammad/have_no_over_not_to.rb diff --git a/.rubocop/cop/zammad/have_no_over_not_to.rb b/.rubocop/cop/zammad/have_no_over_not_to.rb new file mode 100644 index 000000000..c409867ff --- /dev/null +++ b/.rubocop/cop/zammad/have_no_over_not_to.rb @@ -0,0 +1,62 @@ +require 'capybara/rspec/matchers' + +module RuboCop + module Cop + module Zammad + # This cop is used to identify usages of `have_*` RSpec matcher together with `not_to`. + # `have_no_*` with `to` is preferred + # + # @example + # # bad + # expect(page).not_to have_css('#elem') + # + # # good + # expect(page).to have_no_css('#elem') + class HaveNoOverNotTo < Base + extend AutoCorrector + + MSG = 'Prefer `.to %s` over `.not_to %s`.'.freeze + + def_node_matcher :on_have_not_no, '(send (send nil? :expect ...) :not_to (send nil? #matcher_to_replace? ...))' + + def matcher_to_replace?(matcher_name) + have_not_no?(matcher_name) && capybara_matcher?(matcher_name) + end + + def have_not_no?(matcher_name) + matcher_name.match?(/^have_(?!no)/) + end + + def capybara_matcher?(matcher_name) + Capybara::RSpecMatchers.instance_methods.include?(matcher_name) + end + + def on_send(node) + on_have_not_no(node) do + add_offense(node, message: message(node)) do |corrector| + corrector.replace(original_matcher_node(node).loc.selector, alternative_matcher(node)) + corrector.replace(node.loc.selector, 'to') + end + end + end + + def original_matcher_node(node) + node.children[2] + end + + def original_matcher(node) + original_matcher_node(node).method_name + end + + def alternative_matcher(node) + original_matcher(node).to_s.sub 'have_', 'have_no_' + end + + def message(node) + message = format(MSG, replacement: alternative_matcher(node), original: original_matcher(node)) + end + end + end + end +end + diff --git a/.rubocop/rubocop_zammad.rb b/.rubocop/rubocop_zammad.rb index fdf882167..630678c27 100644 --- a/.rubocop/rubocop_zammad.rb +++ b/.rubocop/rubocop_zammad.rb @@ -1,3 +1,4 @@ require_relative 'cop/zammad/exists_condition' +require_relative 'cop/zammad/have_no_over_not_to' require_relative 'cop/zammad/no_to_sym_on_string' require_relative 'cop/zammad/prefer_negated_if_over_unless' diff --git a/spec/system/channels/email_spec.rb b/spec/system/channels/email_spec.rb index 7d85d0a25..7ea680f12 100644 --- a/spec/system/channels/email_spec.rb +++ b/spec/system/channels/email_spec.rb @@ -16,7 +16,7 @@ RSpec.describe 'Manage > Channels > Email', type: :system do expect(page).to have_css('#c-account h3', text: 'Inbound') expect(page).to have_css('#c-account h3', text: 'Outbound') - expect(page).not_to have_css('.js-editInbound, .js-editOutbound', text: 'Edit') + expect(page).to have_no_css('.js-editInbound, .js-editOutbound', text: 'Edit') end end end diff --git a/spec/system/knowledge_base_public/guest_spec.rb b/spec/system/knowledge_base_public/guest_spec.rb index 11ba9845b..6922ba3b2 100644 --- a/spec/system/knowledge_base_public/guest_spec.rb +++ b/spec/system/knowledge_base_public/guest_spec.rb @@ -23,7 +23,7 @@ RSpec.describe 'Public Knowledge Base for guest', type: :system, authenticated_a it 'does not show answer' do within '.main' do - expect(page).not_to have_selector(:link_containing, published_answer.translation.title) + expect(page).to have_no_selector(:link_containing, published_answer.translation.title) end end end @@ -41,13 +41,13 @@ RSpec.describe 'Public Knowledge Base for guest', type: :system, authenticated_a it 'does not show draft answer' do within '.main' do - expect(page).not_to have_selector(:link_containing, draft_answer.translation.title) + expect(page).to have_no_selector(:link_containing, draft_answer.translation.title) end end it 'does not show internal answer' do within '.main' do - expect(page).not_to have_selector(:link_containing, internal_answer.translation.title) + expect(page).to have_no_selector(:link_containing, internal_answer.translation.title) end end @@ -93,7 +93,7 @@ RSpec.describe 'Public Knowledge Base for guest', type: :system, authenticated_a it { expect(page).to have_text(published_answer.translation_primary.title) } it { expect(page).to have_text('only available in these languages') } - it { expect(page).not_to have_selector('h1', text: published_answer.translation_primary.title) } + it { expect(page).to have_no_selector('h1', text: published_answer.translation_primary.title) } context 'follow primary locale' do before { click_on published_answer.translation_primary.title } diff --git a/spec/system/knowledge_base_public/menu_items_spec.rb b/spec/system/knowledge_base_public/menu_items_spec.rb index 6e9f17981..73670a0d1 100644 --- a/spec/system/knowledge_base_public/menu_items_spec.rb +++ b/spec/system/knowledge_base_public/menu_items_spec.rb @@ -19,7 +19,7 @@ RSpec.describe 'Public Knowledge Base menu items', type: :system, authenticated_ end it "doesn't show footer link in header" do - expect(page).not_to have_css('header .menu-item', text: menu_item_3.title) + expect(page).to have_no_css('header .menu-item', text: menu_item_3.title) end it 'shows footer public link' do @@ -27,7 +27,7 @@ RSpec.describe 'Public Knowledge Base menu items', type: :system, authenticated_ end it "doesn't show footer link of another locale" do - expect(page).not_to have_css('footer .menu-item', text: menu_item_4.title) + expect(page).to have_no_css('footer .menu-item', text: menu_item_4.title) end it 'shows public links in given order' do diff --git a/spec/system/login/message_spec.rb b/spec/system/login/message_spec.rb index 7b8661cb7..e504aa7c8 100644 --- a/spec/system/login/message_spec.rb +++ b/spec/system/login/message_spec.rb @@ -39,7 +39,7 @@ RSpec.describe 'Login Message', type: :system, authenticated_as: false do it 'does not show message' do open_login_page - expect(page).not_to have_text(message) + expect(page).to have_no_text(message) end it 'shows message on the go' do diff --git a/spec/system/profile/devices_spec.rb b/spec/system/profile/devices_spec.rb index 839120b64..bb0336247 100644 --- a/spec/system/profile/devices_spec.rb +++ b/spec/system/profile/devices_spec.rb @@ -13,6 +13,6 @@ RSpec.describe 'Profile > Devices', type: :system, authenticated_as: true do .click end - wait(5).until { expect(page).to have_no_css('td', text: device.name) } + expect(page).to have_no_css('td', text: device.name) end end diff --git a/spec/system/ticket/create_spec.rb b/spec/system/ticket/create_spec.rb index 70f773341..1378d1057 100644 --- a/spec/system/ticket/create_spec.rb +++ b/spec/system/ticket/create_spec.rb @@ -14,7 +14,7 @@ RSpec.describe 'Ticket Create', type: :system do visit 'ticket/create' use_template(template) - expect(page).not_to have_selector 'select[name="group_id"]' + expect(page).to have_no_selector 'select[name="group_id"]' end end @@ -38,16 +38,16 @@ RSpec.describe 'Ticket Create', type: :system do within(:active_content) do use_template(template) - expect(page).not_to have_css('div.js-securityEncrypt.btn--active', wait: 5) - expect(page).not_to have_css('div.js-securitySign.btn--active', wait: 5) + expect(page).to have_no_css('div.js-securityEncrypt.btn--active') + expect(page).to have_no_css('div.js-securitySign.btn--active') click '.js-submit' expect(page).to have_css('.ticket-article-item', count: 1) open_article_meta - expect(page).not_to have_css('span', text: 'Signed') - expect(page).not_to have_css('span', text: 'Encrypted') + expect(page).to have_no_css('span', text: 'Signed') + expect(page).to have_no_css('span', text: 'Encrypted') security_result = Ticket::Article.last.preferences['security'] expect(security_result['encryption']['success']).to be nil @@ -99,8 +99,8 @@ RSpec.describe 'Ticket Create', type: :system do open_article_meta - expect(page).not_to have_css('span', text: 'Signed') - expect(page).not_to have_css('span', text: 'Encrypted') + expect(page).to have_no_css('span', text: 'Signed') + expect(page).to have_no_css('span', text: 'Encrypted') security_result = Ticket::Article.last.preferences['security'] expect(security_result['encryption']['success']).to be nil @@ -128,7 +128,7 @@ RSpec.describe 'Ticket Create', type: :system do open_article_meta expect(page).to have_css('span', text: 'Signed') - expect(page).not_to have_css('span', text: 'Encrypted') + expect(page).to have_no_css('span', text: 'Encrypted') security_result = Ticket::Article.last.preferences['security'] expect(security_result['encryption']['success']).to be nil @@ -155,7 +155,7 @@ RSpec.describe 'Ticket Create', type: :system do open_article_meta - expect(page).not_to have_css('span', text: 'Signed') + expect(page).to have_no_css('span', text: 'Signed') expect(page).to have_css('span', text: 'Encrypted') security_result = Ticket::Article.last.preferences['security'] diff --git a/spec/system/ticket/update_spec.rb b/spec/system/ticket/update_spec.rb index f3d7016db..62f738403 100644 --- a/spec/system/ticket/update_spec.rb +++ b/spec/system/ticket/update_spec.rb @@ -105,7 +105,7 @@ RSpec.describe 'Ticket Update', type: :system do click('.js-openDropdownMacro') click(".js-dropdownActionMacro[data-id=\"#{macro.id}\"]") - expect(page).not_to have_css('.js-submitDropdown .js-submit[disabled]', wait: 2) + expect(page).to have_no_css('.js-submitDropdown .js-submit[disabled]') end expect(page).to have_field(attribute.name, with: attribute_value, visible: :hidden) @@ -150,7 +150,7 @@ RSpec.describe 'Ticket Update', type: :system do click('.js-openDropdownMacro') click(".js-dropdownActionMacro[data-id=\"#{macro.id}\"]") - expect(page).not_to have_css('.js-submitDropdown .js-submit[disabled]', wait: 2) + expect(page).to have_no_css('.js-submitDropdown .js-submit[disabled]') end expect(page).to have_selector('.content.active .article-content', text: 'test body') diff --git a/spec/system/ticket/zoom_spec.rb b/spec/system/ticket/zoom_spec.rb index 9a5814946..bdca01829 100644 --- a/spec/system/ticket/zoom_spec.rb +++ b/spec/system/ticket/zoom_spec.rb @@ -194,7 +194,7 @@ RSpec.describe 'Ticket zoom', type: :system do # wait till input box expands completely find('.attachmentPlaceholder-label').in_fixed_position - expect(page).not_to have_css('.attachmentPlaceholder-hint', wait: 0) + expect(page).to have_no_css('.attachmentPlaceholder-hint') find('.articleNewEdit-body').send_keys('Some reply') click '.js-submit' @@ -534,7 +534,7 @@ RSpec.describe 'Ticket zoom', type: :system do create(:smime_certificate, :with_private, fixture: system_email_address) visit "#ticket/zoom/#{ticket.id}" - expect(page).not_to have_css('.article-content', text: 'somebody with some text') + expect(page).to have_no_css('.article-content', text: 'somebody with some text') click '.js-securityRetryProcess' expect(page).to have_css('.article-content', text: 'somebody with some text') end @@ -911,9 +911,9 @@ RSpec.describe 'Ticket zoom', type: :system do refresh # refresh to have assets generated for ticket expect(page).to have_select('state_id', options: %w[new open closed]) - expect(page).not_to have_select('priority_id') - expect(page).not_to have_select('owner_id') - expect(page).not_to have_css('div.tabsSidebar-tab[data-tab=customer]') + expect(page).to have_no_select('priority_id') + expect(page).to have_no_select('owner_id') + expect(page).to have_no_css('div.tabsSidebar-tab[data-tab=customer]') end end