From e9636412a78c3c2471f25edab79134ed7b8499c2 Mon Sep 17 00:00:00 2001 From: Andrew Crump Date: Tue, 13 Dec 2011 22:06:23 +0000 Subject: [PATCH] Scenarios for choosing rules to apply, refs #4. --- features/choose_rules_to_apply.feature | 25 +++++++++++++++ features/step_definitions/cookbook_steps.rb | 35 +++++++++++++++++++-- features/support/lint_helpers.rb | 8 ++--- lib/foodcritic/linter.rb | 8 ++--- lib/foodcritic/rules.rb | 6 ++-- 5 files changed, 67 insertions(+), 15 deletions(-) create mode 100644 features/choose_rules_to_apply.feature diff --git a/features/choose_rules_to_apply.feature b/features/choose_rules_to_apply.feature new file mode 100644 index 00000000..59119a95 --- /dev/null +++ b/features/choose_rules_to_apply.feature @@ -0,0 +1,25 @@ +Feature: Choose rules to apply + + In order to remove warnings that are not appropriate for my usage of Chef + As a developer + I want to be able to filter the rules applied based on tags associated with each rule + + Scenario Outline: Specified tags + Given a cookbook that matches rules + When I check the cookbook specifying tags + Then the warnings shown should be + + Examples: + | cookbook_matches | tag_arguments | warnings_shown | + | FC002,FC003,FC004 | | FC002,FC003,FC004 | + | FC002 | -t FC002 | FC002 | + | FC002,FC003,FC004 | --tags FC002 | FC002 | + | FC002,FC003,FC004 | --tags fc002 | | + | FC002,FC003,FC004 | --tags FC005 | | + | FC002,FC003,FC004 | --tags ~FC002 | FC003,FC004 | + | | --tags FC002 | | + | FC002,FC003,FC004 | --tags @FC002 | | + | FC002,FC003,FC004 | --tags style | FC002,FC004 | + | FC002,FC003,FC004 | --tags FC002 --tags FC003 | | + | FC002,FC003,FC004 | --tags style --tags services | FC004 | + | FC002,FC003,FC004 | --tags style,services | FC002,FC004 | \ No newline at end of file diff --git a/features/step_definitions/cookbook_steps.rb b/features/step_definitions/cookbook_steps.rb index 183ab249..04c0faa3 100644 --- a/features/step_definitions/cookbook_steps.rb +++ b/features/step_definitions/cookbook_steps.rb @@ -1,5 +1,5 @@ -When /^I check the cookbook$/ do - run_lint +When /^I check the cookbook(?: specifying tags(.*))?$/ do |tags| + run_lint(tags) end Then /^the (?:[a-z ]+) warning ([0-9]+) should be displayed(?: against the (metadata) file)?$/ do |code, file| @@ -462,4 +462,33 @@ puts matching_node.to_s end }.strip -end \ No newline at end of file +end + +Given /^a cookbook that matches rules (.*)$/ do |rules| + recipe = '' + rules.split(',').each do |rule| + if rule == 'FC002' + recipe += %q{ + directory "#{node['base_dir']}" do + action :create + end + } + elsif rule == 'FC003' + recipe += %Q{nodes = search(:node, "hostname:[* TO *]")\n} + elsif rule == 'FC004' + recipe += %q{ + execute "stop-jetty" do + command "/etc/init.d/jetty6 stop" + action :run + end + } + end + end + write_recipe(recipe.strip) +end + +Then /^the warnings shown should be (.*)$/ do |warnings| + warnings.split(',').each do |warning| + expect_warning(warning, :line => nil) + end +end diff --git a/features/support/lint_helpers.rb b/features/support/lint_helpers.rb index 05b08e5e..739a8d6d 100644 --- a/features/support/lint_helpers.rb +++ b/features/support/lint_helpers.rb @@ -10,8 +10,8 @@ def write_metadata(content) write_file 'cookbooks/example/metadata.rb', content end - def run_lint - run_simple(unescape('foodcritic cookbooks/example/'), false) + def run_lint(cmd_args) + run_simple(unescape("foodcritic #{cmd_args} cookbooks/example/"), false) end def expect_warning(code, options={}) @@ -38,9 +38,9 @@ def expect_warning(code, options={}) end if opt[:expect_warning] - assert_partial_output("#{code}: #{warning_text}: #{opt[:file]}:#{opt[:line]}\n", all_output) + assert_partial_output("#{code}: #{warning_text}: #{opt[:file]}:#{opt[:line]}#{"\n" if ! opt[:line].nil?}", all_output) else - assert_no_partial_output("#{code}: #{warning_text}: #{opt[:file]}:#{opt[:line]}\n", all_output) + assert_no_partial_output("#{code}: #{warning_text}: #{opt[:file]}:#{opt[:line]}#{"\n" if ! opt[:line].nil?}", all_output) end end diff --git a/lib/foodcritic/linter.rb b/lib/foodcritic/linter.rb index bdfa468b..d0a86063 100644 --- a/lib/foodcritic/linter.rb +++ b/lib/foodcritic/linter.rb @@ -24,11 +24,9 @@ def check(cookbook_path, options) tag_expr = Gherkin::TagExpression.new(options[:tags]) files_to_process(cookbook_path).each do |file| ast = read_file(file) - @rules.each do |rule| - if tag_expr.eval(rule.tags) - matches = rule.recipe.yield(ast, file) - matches.each{|match| warnings << Warning.new(rule, {:filename => file}.merge(match))} unless matches.nil? - end + @rules.select{|rule| tag_expr.eval(rule.tags)}.each do |rule| + matches = rule.recipe.yield(ast, file) + matches.each{|match| warnings << Warning.new(rule, {:filename => file}.merge(match))} unless matches.nil? end end Review.new(warnings) diff --git a/lib/foodcritic/rules.rb b/lib/foodcritic/rules.rb index 9260637c..15961f59 100644 --- a/lib/foodcritic/rules.rb +++ b/lib/foodcritic/rules.rb @@ -1,5 +1,5 @@ rule "FC002", "Avoid string interpolation where not required" do - tags %w{style} + tags %w{style strings} description "When setting a resource value avoid string interpolation where not required." recipe do |ast| ast.xpath(%q{//string_literal[count(descendant::string_embexpr) = 1 and @@ -16,7 +16,7 @@ end rule "FC004", "Use a service resource to start and stop services" do - tags %w{style} + tags %w{style services} description "Avoid use of execute to control services - use the service resource instead." recipe do |ast| find_resources(ast, 'execute').find_all do |cmd| @@ -43,7 +43,7 @@ end rule "FC006", "Mode should be quoted or fully specified when setting file permissions" do - tags %w{correctness} + tags %w{correctness files} description "Not quoting mode when setting permissions can lead to incorrect permissions being set." recipe do |ast| ast.xpath(%q{//ident[@value='mode']/parent::command/descendant::int[string-length(@value) < 4]/