Permalink
Browse files

FC003: Don't warn when return is used, refs #92.

  • Loading branch information...
1 parent 9a28569 commit 89526159a41b4602213779dcaf42739e4fc1cb33 @acrmp acrmp committed Mar 19, 2013
@@ -45,3 +45,12 @@ Feature: Check for Chef Server
Given a cookbook with a single recipe that searches but checks first (alternation) to see if this is server
When I check the cookbook
Then the check for server warning 003 should not be displayed against the condition
+
+ Scenario Outline: Search checking for server (return)
+ Given a cookbook with a single recipe that searches but returns first (<format>) if search is not supported
+ When I check the cookbook
+ Then the check for server warning 003 should not be displayed against the search after the <format> conditional
+ Examples:
+ | format |
+ | oneline |
+ | multiline |
@@ -1085,6 +1085,22 @@ def in_tier?(*tier)
}
end
+Given /^a cookbook with a single recipe that searches but returns first \((oneline|multiline)\) if search is not supported$/ do |format|
+ if format == 'oneline'
+ write_recipe %q{
+ return Chef::Log.warn("This recipe uses search. Chef Solo does not support search.") if Chef::Config[:solo]
+ nodes = search(:node, "hostname:[* TO *] AND chef_environment:#{node.chef_environment}")
+ }
+ else
+ write_recipe %q{
+ if Chef::Config[:solo]
+ return Chef::Log.warn("This recipe uses search. Chef Solo does not support search.")
+ end
+ nodes = search(:node, "hostname:[* TO *] AND chef_environment:#{node.chef_environment}")
+ }
+ end
+end
+
Given 'a cookbook with a single recipe that searches without checking if this is server' do
write_recipe %q{nodes = search(:node, "hostname:[* TO *] AND chef_environment:#{node.chef_environment}")}
end
@@ -1480,6 +1496,11 @@ def search(bag_name, query=nil, sort=nil, start=0, rows=1000, &block)
expect_warning("FC003", :line => 5, :expect_warning => false)
end
+Then /^the check for server warning 003 should not be displayed against the search after the (.*) conditional$/ do |format|
+ line = format == 'oneline' ? 2 : 4
+ expect_warning("FC003", :line => line, :expect_warning => false)
+end
+
Then 'the check for server warning 003 should not be displayed given we have checked' do
expect_warning("FC003", :line => 4, :expect_warning => false)
end
View
@@ -36,14 +36,16 @@ def checks_for_chef_solo?(ast)
raise_unless_xpath!(ast)
# TODO: This expression is too loose, but also will fail to match other
# types of conditionals.
- ! ast.xpath(%q{//*[self::if or self::unless]/*[self::aref or
+ (! ast.xpath(%q{//*[self::if or self::unless]/*[self::aref or
child::aref or self::call]
[count(descendant::const[@value = 'Chef' or @value = 'Config']) = 2
and
( count(descendant::ident[@value='solo']) > 0
or count(descendant::tstring_content[@value='solo']) > 0
)
- ]}).empty?
+ ]}).empty?) ||
+ ast.xpath('//if_mod[return][aref/descendant::ident/@value="solo"]/aref/
+ const_path_ref/descendant::const').map{|c|c['value']} == %w{Chef Config}
end
# Is the [chef-solo-search library](https://github.com/edelight/chef-solo-search)
@@ -93,6 +93,7 @@ def parse_ast(str)
end
it "returns false if there is no reference to chef solo" do
ast.expect :xpath, [], [String]
+ ast.expect :xpath, [], [String]
@miketheman

miketheman Mar 22, 2013

Contributor

This line looks exactly like the one prior, what added test does this trigger?

@acrmp

acrmp Mar 22, 2013

Owner

This line was needed because we now execute a separate query against the AST in #checks_for_chef_solo? looking for use of return.

MiniTest will now raise an error if your mock is sent an unexpected message when using #expect.

Really if we care about the message enough to use #expect we should be asserting that the xpath query is what we expect (rather than just being any String as currently).

refute api.checks_for_chef_solo?(ast)
end
it "returns true if there is one reference to chef solo" do

0 comments on commit 8952615

Please sign in to comment.