Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add support for specifying multiple paths #34

Closed
wants to merge 6 commits into from

2 participants

Chris Griego Andrew Crump
Chris Griego

This has multiple use cases:

  1. Allow a single foodcritic run to check both a cookbooks and a site-cookbooks folders.
  2. Check multiple specific cookbooks at once, so someone could not check community cookbooks but can check their custom cookbooks when all are in the same folder.
  3. In guard-foodcritic, multiple individual files can change in the same tick and all of those changed files need to be run. This is much easier to run all at once like guard-rspec would.
Andrew Crump
Owner

Hi Chris,

This looks super useful. I'm suffering the effects of a cold at the moment so I'll merge it this weekend when I have a clearer head if you can wait for then. Sorry for the delay.

Cheers,

Andrew.

Andrew Crump
Owner

Hi Chris,

Closing - I've squashed and merged these commits.

Thanks,

Andrew.

Andrew Crump acrmp closed this
Chris Griego

Thanks for the release! With your help I was able to release 1.0.0 of guard-foodcritic.
https://rubygems.org/gems/guard-foodcritic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
5 features/command_line_help.feature
View
@@ -9,11 +9,6 @@ Feature: Command line help
When I run it on the command line with no arguments
Then the simple usage text should be displayed along with a non-zero exit code
- Scenario: Too many arguments
- Given I have installed the lint tool
- When I run it on the command line with too many arguments
- Then the simple usage text should be displayed along with a non-zero exit code
-
Scenario: Non-existent cookbook directory
Given I have installed the lint tool
When I run it on the command line specifying a cookbook that does not exist
11 features/multiple_paths.feature
View
@@ -0,0 +1,11 @@
+Feature: Multiple paths
+
+ In order to avoid needing to run foodcritic multiple times
+ As a developer
+ I want to lint multiple paths at once
+
+ Scenario: Linting multiple individual cookbooks
+ Given a cookbook with a single recipe that reads node attributes via symbols only
+ And another cookbook with a single recipe that reads node attributes via strings only
+ When I check both cookbooks
+ Then the node access warning 001 should be displayed
8 features/step_definitions/cookbook_steps.rb
View
@@ -790,6 +790,10 @@ def search(bag_name, query=nil, sort=nil, start=0, rows=1000, &block)
run_lint(options + ["cookbooks/#{whole_tree.nil? ? 'example' : ''}"])
end
+When 'I check both cookbooks' do
+ run_lint(["cookbooks/another_example", "cookbooks/example"])
+end
+
When 'I check the cookbook without specifying a Chef version' do
run_lint(['-I', 'rules/test.rb', 'cookbooks/example'])
end
@@ -845,10 +849,6 @@ def search(bag_name, query=nil, sort=nil, start=0, rows=1000, &block)
run_lint(['-v'])
end
-When 'I run it on the command line with too many arguments' do
- run_lint(['example', 'example'])
-end
-
Then 'a warning for the custom rule should be displayed' do
expect_output('BAR001: Use symbols in preference to strings to access node attributes: cookbooks/example/recipes/default.rb:1')
end
2  features/support/command_helpers.rb
View
@@ -109,7 +109,7 @@ def expect_usage_option(short_switch, long_switch, description)
#
# @param [Boolean] is_exit_zero The exit code to check for.
def usage_displayed(is_exit_zero)
- expect_output 'foodcritic [cookbook_path]'
+ expect_output 'foodcritic [cookbook_paths]'
expect_usage_option('c', 'chef-version VERSION', 'Only check against rules valid for this version of Chef.')
expect_usage_option('f', 'epic-fail TAGS', 'Fail the build if any of the specified tags are matched.')
expect_usage_option('r', '[no-]repl', 'Drop into a REPL for interactive rule editing.')
18 lib/foodcritic/command_line.rb
View
@@ -11,7 +11,7 @@ def initialize(args)
@original_args = args.dup
@options = {:fail_tags => [], :tags => [], :include_rules => []}
@parser = OptionParser.new do |opts|
- opts.banner = 'foodcritic [cookbook_path]'
+ opts.banner = 'foodcritic [cookbook_paths]'
opts.on("-r", "--[no-]repl",
"Drop into a REPL for interactive rule editing.") do |r|
options[:repl] = r
@@ -76,18 +76,18 @@ def version
"foodcritic #{FoodCritic::VERSION}"
end
- # If the cookbook path provided is valid
+ # If the cookbook paths provided are valid
#
- # @return [Boolean] True if the path exists.
- def valid_path?
- @args.length == 1 and File.exists?(@args[0])
+ # @return [Boolean] True if the paths exist.
+ def valid_paths?
+ @args.any? && @args.all? {|path| File.exists?(path) }
end
- # The cookbook path
+ # The cookbook paths
#
- # @return [String] Path to the cookbook(s) being checked.
- def cookbook_path
- @args[0]
+ # @return [Array<String>] Path(s) to the cookbook(s) being checked.
+ def cookbook_paths
+ @args
end
# Is the search grammar specified valid?
8 lib/foodcritic/domain.rb
View
@@ -20,16 +20,16 @@ def initialize(rule, match={})
# The collected warnings (if any) raised against a cookbook tree.
class Review
- attr_reader :cookbook_path, :warnings
+ attr_reader :cookbook_paths, :warnings
# Create a new review
#
- # @param [String] cookbook_path The path this review was performed against
+ # @param [String] cookbook_paths The path this review was performed against
# @param [Array] warnings The warnings raised in this review
# @param [Boolean] is_failed Have warnings been raised that mean this
# should be considered failed?
- def initialize(cookbook_path, warnings, is_failed)
- @cookbook_path = cookbook_path
+ def initialize(cookbook_paths, warnings, is_failed)
+ @cookbook_paths = cookbook_paths
@warnings = warnings
@is_failed = is_failed
end
43 lib/foodcritic/linter.rb
View
@@ -25,8 +25,8 @@ def self.check(cmd_line)
return [cmd_line.version, 0] if cmd_line.show_version?
if ! cmd_line.valid_grammar?
[cmd_line.help, 4]
- elsif cmd_line.valid_path?
- review = FoodCritic::Linter.new.check(cmd_line.cookbook_path,
+ elsif cmd_line.valid_paths?
+ review = FoodCritic::Linter.new.check(cmd_line.cookbook_paths,
cmd_line.options)
[review, review.failed? ? 3 : 0]
else
@@ -42,16 +42,16 @@ def initialize
# Review the cookbooks at the provided path, identifying potential
# improvements.
#
- # @param [String] cookbook_path The file path to an individual cookbook
- # directory
+ # @param [String] cookbook_paths The file path(s) to an individual
+ # cookbook(s) being checked
# @param [Hash] options Options to apply to the linting
# @option options [Array] tags The tags to filter rules based on
# @option options [Array] fail_tags The tags to fail the build on
# @return [FoodCritic::Review] A review of your cookbooks, with any
# warnings issued.
- def check(cookbook_path, options)
- raise ArgumentError, "Cookbook path is required" if cookbook_path.nil?
- @last_cookbook_path, @last_options = cookbook_path, options
+ def check(cookbook_paths, options)
+ raise ArgumentError, "Cookbook paths are required" if cookbook_paths.nil?
+ @last_cookbook_paths, @last_options = cookbook_paths, options
load_rules unless defined? @rules
warnings = []; last_dir = nil; matched_rule_tags = Set.new
@@ -59,7 +59,7 @@ def check(cookbook_path, options)
matching_tags?(options[:tags], rule.tags) and
applies_to_version?(rule, options[:chef_version] || DEFAULT_CHEF_VERSION)
end
- files_to_process(cookbook_path).each do |file|
+ files_to_process(cookbook_paths).each do |file|
cookbook_dir = Pathname.new(
File.join(File.dirname(file), '..')).cleanpath
ast = read_ast(file)
@@ -82,7 +82,7 @@ def check(cookbook_path, options)
last_dir = cookbook_dir
end
- @review = Review.new(cookbook_path, warnings,
+ @review = Review.new(cookbook_paths, warnings,
should_fail_build?(options[:fail_tags], matched_rule_tags))
binding.pry if options[:repl]
@@ -92,7 +92,7 @@ def check(cookbook_path, options)
# Convenience method to repeat the last check. Intended to be used from the
# REPL.
def recheck
- check(@last_cookbook_path, @last_options)
+ check(@last_cookbook_paths, @last_options)
end
# Load the rules from the (fairly unnecessary) DSL.
@@ -146,14 +146,23 @@ def matches(match_method, *params)
# Return the files within a cookbook tree that we are interested in trying
# to match rules against.
#
- # @param [String] dir The cookbook directory
- # @return [Array] The files underneath the provided directory to be
+ # @param [Array<String>] dirs The cookbook path(s)
+ # @return [Array] The files underneath the provided paths to be
# processed.
- def files_to_process(dir)
- return [dir] unless File.directory? dir
- cookbook_glob = '{attributes,providers,recipes,resources}/*.rb'
- Dir.glob(File.join(dir, cookbook_glob)) +
- Dir.glob(File.join(dir, "*/#{cookbook_glob}"))
+ def files_to_process(dirs)
+ files = []
+
+ dirs.each do |dir|
+ if File.directory? dir
+ cookbook_glob = '{attributes,providers,recipes,resources}/*.rb'
+ files += Dir.glob(File.join(dir, cookbook_glob)) +
+ Dir.glob(File.join(dir, "*/#{cookbook_glob}"))
+ else
+ files << dir
+ end
+ end
+
+ files
end
# Whether to fail the build.
2  lib/foodcritic/output.rb
View
@@ -47,7 +47,7 @@ def output(review)
else
next
end
- }, review.cookbook_path])
+ }] + review.cookbook_paths)
Rak.send(:remove_const, :VERSION) # Prevent duplicate VERSION warning
load Gem.bin_path('rak', 'rak') # Assumes Rubygems
end
38 spec/foodcritic/command_line_spec.rb
View
@@ -5,17 +5,47 @@
FoodCritic::CommandLine.new([]).wont_be_nil
end
- describe "#valid_path?" do
+ describe "#cookbook_paths" do
+ it "retuns an empty array if no paths are specified" do
+ FoodCritic::CommandLine.new([]).cookbook_paths.must_be_empty
+ end
+
+ it "returns a single item array for a specified directory" do
+ FoodCritic::CommandLine.new(["example"]).cookbook_paths.must_equal ["example"]
+ end
+
+ it "returns multiple items for multiple specified directories" do
+ FoodCritic::CommandLine.new(["example", "another_example"]).cookbook_paths.must_equal ["example", "another_example"]
+ end
+
+ it "ignores known arguments" do
+ FoodCritic::CommandLine.new(["example", "--context"]).cookbook_paths.must_equal ["example"]
+ end
+ end
+
+ describe "#valid_paths?" do
+ it "returns false if no paths are specified" do
+ refute FoodCritic::CommandLine.new([]).valid_paths?
+ end
+
it "returns true if the specified directory exists" do
- assert FoodCritic::CommandLine.new(["lib"]).valid_path?
+ assert FoodCritic::CommandLine.new(["lib"]).valid_paths?
end
it "returns false if the specified directory does not exist" do
- refute FoodCritic::CommandLine.new(["lib2"]).valid_path?
+ refute FoodCritic::CommandLine.new(["lib2"]).valid_paths?
end
it "returns true if the specified file exists" do
- assert FoodCritic::CommandLine.new(["lib/foodcritic.rb"]).valid_path?
+ assert FoodCritic::CommandLine.new(["lib/foodcritic.rb"]).valid_paths?
+ end
+
+ it "returns true if both specified paths exist" do
+ assert FoodCritic::CommandLine.new(["lib", "lib/foodcritic.rb"]).valid_paths?
+ end
+
+ it "returns false if any on the specified paths do not exist" do
+ refute FoodCritic::CommandLine.new(["lib", "lib2"]).valid_paths?
end
end
end
Something went wrong with that request. Please try again.