Removing optionparser in favor of Thor

Rubocop fixes

Adding tests and making failure expectations match reality for the hook command

Adding tests and making failure expectations match reality for the hook command

Implementing the hook properly. Adding tests.

bin/msync

@@ -3,6 +3,6 @@
 lib = File.expand_path('../../lib', __FILE__)
 $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 
-require 'modulesync'
+require 'modulesync/cli'
 
-ModuleSync.run(ARGV)
+ModuleSync::CLI::Base.start(ARGV)

features/cli.feature

@@ -0,0 +1,18 @@
+Feature: CLI
+  ModuleSync needs to have a robust command line interface
+
+  Scenario: When passing no arguments to the msync command
+    When I run `msync`
+    And the output should match /Commands:/
+
+  Scenario: When passing invalid arguments to the msync update command
+    When I run `msync update`
+    And the output should match /No value provided for required option/
+
+  Scenario: When passing invalid arguments to the msync hook command
+    When I run `msync hook`
+    And the output should match /Commands:/
+
+  Scenario: When running the help subcommand
+    When I run `msync help`
+    And the output should match /Commands:/

lib/modulesync.rb

@@ -9,6 +9,16 @@
 module ModuleSync
   include Constants
 
+  def self.config_defaults
+    {
+      :project_root         => 'modules/',
+      :git_base             => 'git@github.com:',
+      :managed_modules_conf => 'managed_modules.yml',
+      :configs              => '.',
+      :tag_pattern          => '%s',
+    }
+  end
+
   def self.local_file(config_path, file)
     "#{config_path}/#{MODULE_FILES_DIR}/#{file}"
   end
@@ -40,10 +50,9 @@ def self.managed_modules(path, filter)
     managed_modules
   end
 
-  def self.run(args)
-    cli = CLI.new
-    cli.parse_opts(args)
-    options = cli.options
+  def self.run(options)
+    options = config_defaults.merge(options)
+
     if options[:command] == 'update'
       defaults = Util.parse_config("#{options[:configs]}/#{CONF_FILE}")
 
@@ -87,7 +96,7 @@ def self.run(args)
         end
       end
     elsif options[:command] == 'hook'
-      Hook.hook(args[1], options)
+      Hook.hook(options[:hook], options)
     end
   end
 end

lib/modulesync/cli.rb

@@ -1,121 +1,60 @@
-require 'optparse'
+require 'thor'
+require 'modulesync'
 require 'modulesync/constants'
 require 'modulesync/util'
 
 module ModuleSync
   class CLI
-    include Constants
-
-    def defaults
-      {
-        :namespace            => 'puppetlabs',
-        :branch               => 'master',
-        :git_base             => 'git@github.com:',
-        :managed_modules_conf => 'managed_modules.yml',
-        :configs              => '.',
-        :tag_pattern          => '%s',
-        :project_root         => './modules'
-      }
-    end
-
-    def commands_available
-      %w(
-        update
-        hook
-      )
-    end
-
-    def fail(message)
-      puts @options[:help]
-      puts message
-      exit
-    end
-
-    def parse_opts(args)
-      @options = defaults
-      @options.merge!(Hash.transform_keys_to_symbols(Util.parse_config(MODULESYNC_CONF_FILE)))
-      @options[:command] = args[0] if commands_available.include?(args[0])
-      opt_parser = OptionParser.new do |opts|
-        opts.banner = 'Usage: msync update [-m <commit message>] [-c <directory> ] [--offline] [--noop] [--bump] [--changelog] [--tag] [--tag-pattern <tag_pattern>] [-p <project_root> [-n <namespace>] [-b <branch>] [-r <branch>] [-f <filter>] | hook activate|deactivate [-c <directory> ] [-n <namespace>] [-b <branch>]'
-        opts.on('-m', '--message <msg>',
-                'Commit message to apply to updated modules') do |msg|
-          @options[:message] = msg
-        end
-        opts.on('-n', '--namespace <url>',
-                'Remote github namespace (user or organization) to clone from and push to. Defaults to puppetlabs') do |namespace|
-          @options[:namespace] = namespace
-        end
-        opts.on('-c', '--configs <directory>',
-                'The local directory or remote repository to define the list of managed modules, the file templates, and the default values for template variables.') do |configs|
-          @options[:configs] = configs
-        end
-        opts.on('-b', '--branch <branch>',
-                'Branch name to make the changes in. Defaults to "master"') do |branch|
-          @options[:branch] = branch
-        end
-        opts.on('-p', '--project-root <path>',
-                'Path used by git to clone modules into. Defaults to "modules"') do |project_root|
-          @options[:project_root] = project_root
-        end
-        opts.on('-r', '--remote-branch <branch>',
-                'Remote branch name to push the changes to. Defaults to the branch name') do |branch|
-          @options[:remote_branch] = branch
-        end
-        opts.on('-f', '--filter <filter>',
-                'A regular expression to filter repositories to update.') do |filter|
-          @options[:filter] = filter
-        end
-        opts.on('--amend',
-                'Amend previous commit') do |_msg|
-          @options[:amend] = true
-        end
-        opts.on('--force',
-                'Force push amended commit') do |_msg|
-          @options[:force] = true
-        end
-        opts.on('--noop',
-                'No-op mode') do |_msg|
-          @options[:noop] = true
-        end
-        opts.on('--offline',
-                'Do not run git command. Helpful if you have existing repositories locally.') do |_msg|
-          @options[:offline] = true
-        end
-        opts.on('--bump',
-                'Bump module version to the next minor') do |_msg|
-          @options[:bump] = true
-        end
-        opts.on('--changelog',
-                'Update CHANGELOG.md if version was bumped') do |_msg|
-          @options[:changelog] = true
-        end
-        opts.on('--tag',
-                'Git tag with the current module version') do |_msg|
-          @options[:tag] = true
-        end
-        opts.on('--tag-pattern',
-                'The pattern to use when tagging releases.') do |pattern|
-          @options[:tag_pattern] = pattern
-        end
-        @options[:help] = opts.help
-      end.parse!
-
-      @options.fetch(:message) do
-        if @options[:command] == 'update' && ! @options[:noop] && ! @options[:amend] && ! @options[:offline]
-          fail('A commit message is required unless using noop or offline.')
-        end
+    class Hook < Thor
+      class_option :project_root, :aliases => '-c', :desc => 'Path used by git to clone modules into. Defaults to "modules"', :default => 'modules'
+
+      desc 'activate', 'Activate a git hook.'
+      def activate
+        config = { :command => 'hook' }.merge(options)
+        config[:hook] = 'activate'
+        ModuleSync.run(config)
       end
 
-      @options.fetch(:command) do
-        fail('A command is required.')
+      desc 'deactivate', 'Deactivate a git hook.'
+      def deactivate
+        config = { :command => 'hook' }.merge(options)
+        config[:hook] = 'deactivate'
+        ModuleSync.run(config)
       end
+    end
 
-      if @options[:command] == 'hook' &&
-         (!args.include?('activate') && !args.include?('deactivate'))
-        fail('You must activate or deactivate the hook.')
+    class Base < Thor
+      include Constants
+
+      class_option :project_root, :aliases => '-c', :desc => 'Path used by git to clone modules into. Defaults to "modules"', :default => 'modules'
+
+      desc 'update', 'Update the modules in managed_modules.yml'
+      option :message, :aliases => '-m', :desc => 'Commit message to apply to updated modules. Required unless running in noop mode.'
+      option :namespace, :aliases => '-n', :desc => 'Remote github namespace (user or organization) to clone from and push to. Defaults to puppetlabs', :default => 'puppetlabs'
+      option :configs, :aliases => '-c', :desc => 'The local directory or remote repository to define the list of managed modules, the file templates, and the default values for template variables.'
+      option :branch, :aliases => '-b', :desc => 'Branch name to make the changes in. Defaults to master.', :default => 'master'
+      option :remote_branch, :aliases => '-r', :desc => 'Remote branch name to push the changes to. Defaults to the branch name.'
+      option :filter, :aliases => '-f', :desc => 'A regular expression to filter repositories to update.'
+      option :amend, :type => :boolean, :desc => 'Amend previous commit', :default => false
+      option :force, :type => :boolean, :desc => 'Force push amended commit', :default => false
+      option :noop, :type => :boolean, :desc => 'No-op mode', :default => false
+      option :offline, :type => :boolean, :desc => 'Do not run any Git commands. Allows the user to manage Git outside of ModuleSync.', :default => false
+      option :bump, :type => :boolean, :desc => 'Bump module version to the next minor', :default => false
+      option :changelog, :type => :boolean, :desc => 'Update CHANGELOG.md if version was bumped', :default => false
+      option :tag, :type => :boolean, :desc => 'Git tag with the current module version', :defalut => false
+      option :tag_pattern, :desc => 'The pattern to use when tagging releases.'
+
+      def update
+        config = { :command => 'update' }.merge(options)
+        config.merge!(Util.parse_config(MODULESYNC_CONF_FILE))
+        config = Util.symbolize_keys(config)
+        fail Thor::Error, 'No value provided for required option "--message"' unless config[:noop] || config[:message] || config[:offline]
+        config[:git_opts] = { 'amend' => config[:amend], 'force' => config[:force] }
+        ModuleSync.run(config)
       end
-    end
 
-    attr_reader :options
+      desc 'hook', 'Activate or deactivate a git hook.'
+      subcommand 'hook', ModuleSync::CLI::Hook
+    end
   end
 end

lib/modulesync/util.rb

@@ -2,6 +2,10 @@
 
 module ModuleSync
   module Util
+    def self.symbolize_keys(hash)
+      hash.inject({}) { |memo, (k, v)| memo[k.to_sym] = v; memo }
+    end
+
     def self.parse_config(config_file)
       if File.exist?(config_file)
         YAML.load_file(config_file) || {}
@@ -12,12 +16,3 @@ def self.parse_config(config_file)
     end
   end
 end
-
-class Hash
-  # take keys of hash and transform those to a symbols
-  def self.transform_keys_to_symbols(value)
-    return value unless value.is_a?(Hash)
-    hash = value.inject({}) { |memo, (k, v)| memo[k.to_sym] = Hash.transform_keys_to_symbols(v); memo }
-    hash
-  end
-end

modulesync.gemspec

@@ -22,6 +22,7 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'aruba'
   spec.add_development_dependency 'rubocop'
 
+  spec.add_runtime_dependency 'thor'
   spec.add_runtime_dependency 'git', '~>1.2'
   spec.add_runtime_dependency 'puppet-blacksmith', '~>3.0'
 end