From 02c47f2f73ac2d5a33b9a6c971668a38397b4910 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 11 Oct 2018 19:10:42 +0200 Subject: Add a new QA::ElementWithPattern cop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This cop forbids the use of `element :foo, 'pattern'` and `element :bar, /pattern/` in QA files. Signed-off-by: Rémy Coutable --- rubocop/cop/qa/element_with_pattern.rb | 39 ++++++++++++++++++ rubocop/qa_helpers.rb | 11 ++++++ rubocop/rubocop.rb | 1 + spec/rubocop/cop/qa/element_with_pattern_spec.rb | 50 ++++++++++++++++++++++++ spec/rubocop/qa_helpers_spec.rb | 37 ++++++++++++++++++ 5 files changed, 138 insertions(+) create mode 100644 rubocop/cop/qa/element_with_pattern.rb create mode 100644 rubocop/qa_helpers.rb create mode 100644 spec/rubocop/cop/qa/element_with_pattern_spec.rb create mode 100644 spec/rubocop/qa_helpers_spec.rb diff --git a/rubocop/cop/qa/element_with_pattern.rb b/rubocop/cop/qa/element_with_pattern.rb new file mode 100644 index 00000000000..9d80946f1ba --- /dev/null +++ b/rubocop/cop/qa/element_with_pattern.rb @@ -0,0 +1,39 @@ +require_relative '../../qa_helpers' + +module RuboCop + module Cop + module QA + # This cop checks for the usage of factories in migration specs + # + # @example + # + # # bad + # let(:user) { create(:user) } + # + # # good + # let(:users) { table(:users) } + # let(:user) { users.create!(name: 'User 1', username: 'user1') } + class ElementWithPattern < RuboCop::Cop::Cop + include QAHelpers + + MESSAGE = "Don't use a pattern for element, create a corresponding `%s` instead.".freeze + + def on_send(node) + return unless in_qa_file?(node) + return unless method_name(node).to_s == 'element' + + element_name, pattern = node.arguments + return unless pattern + + add_offense(node, location: pattern.source_range, message: MESSAGE % "qa-#{element_name.value.to_s.tr('_', '-')}") + end + + private + + def method_name(node) + node.children[1] + end + end + end + end +end diff --git a/rubocop/qa_helpers.rb b/rubocop/qa_helpers.rb new file mode 100644 index 00000000000..f4adf7f4e9f --- /dev/null +++ b/rubocop/qa_helpers.rb @@ -0,0 +1,11 @@ +module RuboCop + # Module containing helper methods for writing QA cops. + module QAHelpers + # Returns true if the given node originated from the qa/ directory. + def in_qa_file?(node) + path = node.location.expression.source_buffer.name + + path.start_with?(File.join(Dir.pwd, 'qa')) + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 76d6037706e..6c9b8753c1a 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -29,6 +29,7 @@ require_relative 'cop/migration/update_large_table' require_relative 'cop/project_path_helper' require_relative 'cop/rspec/env_assignment' require_relative 'cop/rspec/factories_in_migration_specs' +require_relative 'cop/qa/element_with_pattern' require_relative 'cop/sidekiq_options_queue' require_relative 'cop/destroy_all' require_relative 'cop/ruby_interpolation_in_translation' diff --git a/spec/rubocop/cop/qa/element_with_pattern_spec.rb b/spec/rubocop/cop/qa/element_with_pattern_spec.rb new file mode 100644 index 00000000000..c5beb40f9fd --- /dev/null +++ b/spec/rubocop/cop/qa/element_with_pattern_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/qa/element_with_pattern' + +describe RuboCop::Cop::QA::ElementWithPattern do + include CopHelper + + let(:source_file) { 'qa/page.rb' } + + subject(:cop) { described_class.new } + + context 'in a QA file' do + before do + allow(cop).to receive(:in_qa_file?).and_return(true) + end + + it "registers an offense for elements with a pattern" do + expect_offense(<<-RUBY) + view 'app/views/shared/groups/_search_form.html.haml' do + element :groups_filter, 'search_field_tag :filter' + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't use a pattern for element, create a corresponding `qa-groups-filter` instead. + element :groups_filter_placeholder, /Search by name/ + ^^^^^^^^^^^^^^^^ Don't use a pattern for element, create a corresponding `qa-groups-filter-placeholder` instead. + end + RUBY + end + + it "does not register an offense for element without a pattern" do + expect_no_offenses(<<-RUBY) + view 'app/views/shared/groups/_search_form.html.haml' do + element :groups_filter + element :groups_filter_placeholder + end + RUBY + end + end + + context 'outside of a migration spec file' do + it "does not register an offense" do + expect_no_offenses(<<-RUBY) + describe 'foo' do + let(:user) { create(:user) } + end + RUBY + end + end +end diff --git a/spec/rubocop/qa_helpers_spec.rb b/spec/rubocop/qa_helpers_spec.rb new file mode 100644 index 00000000000..26e4c1ca6f0 --- /dev/null +++ b/spec/rubocop/qa_helpers_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'rubocop' +require_relative '../../rubocop/qa_helpers' + +describe RuboCop::QAHelpers do + def parse_source(source, path = 'foo.rb') + buffer = Parser::Source::Buffer.new(path) + buffer.source = source + + builder = RuboCop::AST::Builder.new + parser = Parser::CurrentRuby.new(builder) + + parser.parse(buffer) + end + + let(:cop) do + Class.new do + include RuboCop::QAHelpers + end.new + end + + describe '#in_qa_file?' do + it 'returns true for a node in the qa/ directory' do + node = parse_source('10', Rails.root.join('qa', 'qa', 'page', 'dashboard', 'groups.rb')) + + expect(cop.in_qa_file?(node)).to eq(true) + end + + it 'returns false for a node outside the qa/ directory' do + node = parse_source('10', Rails.root.join('app', 'foo', 'foo.rb')) + + expect(cop.in_qa_file?(node)).to eq(false) + end + end +end -- cgit v1.2.3