Skip to content
This repository has been archived by the owner on Sep 19, 2020. It is now read-only.

Commit

Permalink
Scenarios for choosing rules to apply, refs #4.
Browse files Browse the repository at this point in the history
  • Loading branch information
Andrew Crump committed Dec 13, 2011
1 parent 3836620 commit e963641
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 15 deletions.
25 changes: 25 additions & 0 deletions features/choose_rules_to_apply.feature
Original file line number Diff line number Diff line change
@@ -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 <cookbook_matches>
When I check the cookbook specifying tags <tag_arguments>
Then the warnings shown should be <warnings_shown>

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 |
35 changes: 32 additions & 3 deletions features/step_definitions/cookbook_steps.rb
Original file line number Diff line number Diff line change
@@ -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|
Expand Down Expand Up @@ -462,4 +462,33 @@
puts matching_node.to_s
end
}.strip
end
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
8 changes: 4 additions & 4 deletions features/support/lint_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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={})
Expand All @@ -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

Expand Down
8 changes: 3 additions & 5 deletions lib/foodcritic/linter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions lib/foodcritic/rules.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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|
Expand All @@ -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]/
Expand Down

0 comments on commit e963641

Please sign in to comment.