From 8fbe0e726506340fde42afd609fa5a1bd21a6e6f Mon Sep 17 00:00:00 2001
From: Nick Bertrand <github@nickbertrand.com>
Date: Thu, 11 Jul 2019 17:13:40 -0500
Subject: [PATCH 1/2] Move deploy options into subcommands that actually
 support them

---
 lib/r10k/cli/deploy.rb | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/lib/r10k/cli/deploy.rb b/lib/r10k/cli/deploy.rb
index fbde98488..4e65b922a 100644
--- a/lib/r10k/cli/deploy.rb
+++ b/lib/r10k/cli/deploy.rb
@@ -21,17 +21,6 @@ def self.command
 (https://puppetlabs.com/blog/git-workflow-and-puppet-environments/).
         DESCRIPTION
 
-        required nil, :cachedir, 'Specify a cachedir, overriding the value in config'
-        flag nil, :'no-force', 'Prevent the overwriting of local module modifications'
-        flag nil, :'generate-types', 'Run `puppet generate types` after updating an environment'
-        option nil, :'puppet-path', 'Path to puppet executable', argument: :required do |value, cmd|
-          unless File.executable? value
-            $stderr.puts "The specified puppet executable #{value} is not executable."
-            puts cmd.help
-            exit 1
-          end
-        end
-
         run do |opts, args, cmd|
           puts cmd.help(:verbose => opts[:verbose])
           exit 0
@@ -60,8 +49,18 @@ def self.command
 scheduled. On subsequent deployments, Puppetfile deployment will default to off.
           DESCRIPTION
 
-          flag :p, :puppetfile, 'Deploy modules from a puppetfile'
+          required nil, :cachedir, 'Specify a cachedir, overriding the value in config'
           required nil, :'default-branch-override', 'Specify a branchname to override the default branch in the puppetfile'
+          flag nil, :'generate-types', 'Run `puppet generate types` after updating an environment'
+          flag nil, :'no-force', 'Prevent the overwriting of local module modifications'
+          option nil, :'puppet-path', 'Path to puppet executable', argument: :required do |value, cmd|
+            unless File.executable? value
+              $stderr.puts "The specified puppet executable #{value} is not executable."
+              puts cmd.help
+              exit 1
+            end
+          end
+          flag :p, :puppetfile, 'Deploy modules from a puppetfile'
 
           runner R10K::Action::CriRunner.wrap(R10K::Action::Deploy::Environment)
         end
@@ -82,6 +81,15 @@ def self.command
           DESCRIPTION
 
           required :e, :environment, 'Update the modules in the given environment'
+          flag nil, :'generate-types', 'Run `puppet generate types` after updating an environment'
+          flag nil, :'no-force', 'Prevent the overwriting of local module modifications'
+          option nil, :'puppet-path', 'Path to puppet executable', argument: :required do |value, cmd|
+            unless File.executable? value
+              $stderr.puts "The specified puppet executable #{value} is not executable."
+              puts cmd.help
+              exit 1
+            end
+          end
 
           runner R10K::Action::CriRunner.wrap(R10K::Action::Deploy::Module)
         end

From 5515f6cb86acde337c828f4af40e794a7eaa7694 Mon Sep 17 00:00:00 2001
From: Nick Bertrand <github@nickbertrand.com>
Date: Thu, 11 Jul 2019 17:14:31 -0500
Subject: [PATCH 2/2] Add R10K::CLI spec tests

---
 spec/shared-contexts/cli.rb |  27 +++++
 spec/shared-examples/cli.rb | 182 +++++++++++++++++++++++++++++++++
 spec/spec_helper.rb         |   1 +
 spec/unit/cli_spec.rb       | 199 +++++++++++++++++++++++++++++++++++-
 4 files changed, 404 insertions(+), 5 deletions(-)
 create mode 100644 spec/shared-contexts/cli.rb
 create mode 100644 spec/shared-examples/cli.rb

diff --git a/spec/shared-contexts/cli.rb b/spec/shared-contexts/cli.rb
new file mode 100644
index 000000000..e9e072ca9
--- /dev/null
+++ b/spec/shared-contexts/cli.rb
@@ -0,0 +1,27 @@
+shared_context 'cli' do
+  def exit_with_code_and_message(r10k_args, code = 0, message = nil, out = 'stdout')
+    expect do
+      expect { r10k.run(r10k_args) }.to exit_with(code)
+    end.to output(message).send("to_#{out}".to_sym)
+  end
+
+  def filtered_extra_args
+    extra_args.reject { |arg| arg == '--' }
+  end
+
+  def string_to_module(str)
+    str.split('::').inject(Object) { |o, c| o.const_get c }
+  end
+
+  def setup_mock_run(command)
+    command.block = lambda do |opts, args, cmd|
+      [opts, args.to_a, cmd]
+    end
+  end
+
+  def setup_mock_runner(command, klass)
+    command.block = lambda do |opts, args, cmd|
+      klass.new(opts, args.to_a, cmd)
+    end
+  end
+end
diff --git a/spec/shared-examples/cli.rb b/spec/shared-examples/cli.rb
new file mode 100644
index 000000000..a57128595
--- /dev/null
+++ b/spec/shared-examples/cli.rb
@@ -0,0 +1,182 @@
+shared_examples 'missing argument' do |option, optinfo|
+  it 'prints error and exits 1' do
+    ["--#{option}", optinfo[:short]].compact.each do |opt|
+      exit_with_code_and_message(command_args + [opt] + extra_args, 1, %r{option requires an argument}, 'stderr')
+    end
+  end
+end
+
+shared_examples 'accepts option' do |option, optinfo, value|
+  it 'accepts the option' do
+    ["--#{option}", optinfo[:short]].compact.each do |opt|
+      expect(r10k.run(command_args + [opt, value].compact + extra_args)).to eq([{ option.to_sym => value || true }, filtered_extra_args, command])
+    end
+  end
+end
+
+shared_examples 'help option' do |option, optinfo|
+  context "when #{option} specified" do
+    it 'prints command help and exits 0' do
+      ["--#{option}", optinfo[:short]].compact.each do |help_arg|
+        expect(command).to receive(:help).and_call_original
+        exit_with_code_and_message(command_args + [help_arg] + extra_args, 0, %r{USAGE\s*r10k #{command_args.join(' ')}})
+      end
+    end
+  end
+end
+
+shared_examples 'Cri argument parsing' do |options, include_parent_examples|
+  context 'Cri argument parsing' do
+    let!(:prev_run) { command.block }
+
+    before(:each) do
+      setup_mock_run(command)
+    end
+
+    after(:each) do
+      command.block = prev_run
+    end
+
+    options.each do |option, optinfo|
+      if optinfo[:custom_cri_examples]
+        optinfo[:custom_cri_examples].each do |examples|
+          include_examples examples, option, optinfo
+        end
+        next
+      end
+
+      case optinfo[:type]
+      when :required
+        context "when #{option} specified without an argument" do
+          include_examples 'missing argument', option, optinfo
+        end
+
+        context "when #{option} specified with an argument" do
+          include_examples 'accepts option', option, optinfo, optinfo[:value]
+        end
+      when :optional
+        context "when #{option} specified without an argument" do
+          include_examples 'accepts option', option, optinfo, nil
+        end
+
+        context "when #{option} specified with an argument" do
+          include_examples 'accepts option', option, optinfo, optinfo[:value]
+        end
+      when :flag
+        context "when #{option} specified" do
+          include_examples 'accepts option', option, optinfo, nil
+        end
+      else
+        raise ArgumentError, "Unknown option type #{optinfo} for #{option}"
+      end
+    end
+
+    include_examples 'parent command examples' if include_parent_examples
+  end
+end
+
+shared_examples 'parent command examples' do
+  context 'with no arguments' do
+    it 'prints command help and exits 0' do
+      command.block = prev_run
+      expect(command).to receive(:help).and_call_original
+      exit_with_code_and_message(command_args, 0, %r{USAGE\s*r10k #{command_args.join(' ')}})
+    end
+  end
+
+  context 'with unknown argument' do
+    let(:command_args) { super() + ['invalid_arg'] }
+
+    it 'prints error to stderr and exits 1' do
+      exit_with_code_and_message(command_args, 1, %r{unknown command 'invalid_arg'}, 'stderr')
+    end
+  end
+end
+
+shared_examples 'Action argument parsing' do |options|
+  context 'Action argument parsing' do
+    def instance(opts = [])
+      r10k.run(command_args + opts.compact + extra_args).instance_variable_get(:@runner).instance
+    end
+
+    context 'CriRunner' do
+      it do
+        expect(r10k.run(command_args + extra_args)).to be_an_instance_of(R10K::Action::CriRunner)
+      end
+    end
+
+    context 'Runner' do
+      it do
+        expect(r10k.run(command_args + extra_args).instance_variable_get(:@runner)).to be_an_instance_of(R10K::Action::Runner)
+      end
+    end
+
+    context 'Action' do
+      it do
+        expect(instance).to be_an_instance_of(action_class)
+      end
+    end
+
+    options.each do |option, optinfo|
+      if optinfo[:custom_action_examples]
+        optinfo[:custom_action_examples].each do |examples|
+          include_examples examples, option, optinfo
+        end
+        next
+      end
+
+      context "when #{option} specified" do
+        let(:option_variable) do
+          "@#{option.to_s.sub(%r{^-*}, '')}".tr('-', '_').to_sym
+        end
+
+        if CRI_RUNNER_OPTIONS.include? option
+          it 'CriRunner processes the option' do
+            ["--#{option}", optinfo[:short]].compact.each do |opt|
+              expect(instance([opt, optinfo[:value]]).instance_variable_defined?(option_variable)).to be false
+            end
+          end
+        else
+          it 'has the option set' do
+            ["--#{option}", optinfo[:short]].compact.each do |opt|
+              expect(instance([opt, optinfo[:value]]).instance_variable_get(option_variable)).to eq(optinfo[:value] || true)
+            end
+          end
+        end
+        it 'processes extra args correctly' do
+          ["--#{option}", optinfo[:short]].compact.each do |opt|
+            expect(instance([opt, optinfo[:value]]).instance_variable_get(:@argv)).to eq(filtered_extra_args)
+          end
+        end
+      end
+    end
+
+    context 'when no options specified' do
+      it 'processes extra args correctly' do
+        expect(instance.instance_variable_get(:@argv)).to eq(filtered_extra_args)
+      end
+    end
+  end
+end
+
+shared_examples 'deploy examples' do
+  before(:each) do
+    allow(File).to receive(:executable?).with('/path/to/puppet').and_return('true')
+  end
+end
+
+shared_examples 'version examples' do
+  context 'with no arguments' do
+    it 'prints r10k version' do
+      exit_with_code_and_message(command_args, 0, "r10k #{R10K::VERSION}\n")
+    end
+  end
+
+  context 'with -v or --verbose' do
+    it 'prints verbose version info' do
+      ['-v', '--verbose'].each do |option|
+        exit_with_code_and_message(command_args + [option], 0, %r{r10k #{Regexp.quote(R10K::VERSION)}\n#{Regexp.quote(RUBY_DESCRIPTION)}})
+      end
+    end
+  end
+end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 7c7d85744..89d4969f7 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -18,6 +18,7 @@
 
 Dir.glob(File.expand_path('spec/shared-examples/**/*.rb', PROJECT_ROOT)).each { |file| require file }
 
+require 'shared-contexts/cli'
 require 'shared-contexts/git-fixtures'
 require 'matchers/exit_with'
 require 'matchers/match_realpath'
diff --git a/spec/unit/cli_spec.rb b/spec/unit/cli_spec.rb
index f3175bb41..5aff7971c 100644
--- a/spec/unit/cli_spec.rb
+++ b/spec/unit/cli_spec.rb
@@ -1,9 +1,198 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
+require 'r10k/cli'
+
+# Command options that are processed by R10K::Action::CriRunner
+CRI_RUNNER_OPTIONS = %i[color verbose].freeze
+
+COMMANDS = {
+  r10k: {
+    options: {
+      config: { type: :required, value: '/path/to/config', short: '-c' },
+      color: { type: :flag },
+      help: { type: :flag, short: '-h', custom_cri_examples: ['help option'], custom_action_examples: [] },
+      trace: { type: :flag, short: '-t' },
+      verbose: { type: :optional, value: 'debug', short: '-v' },
+    },
+    subcommands: {
+      deploy: {
+        options: {},
+        subcommands: {
+          display: {
+            options: {
+              detail: { type: :flag },
+              fetch: { type: :flag },
+              format: { type: :required, value: 'yaml' },
+              puppetfile: { type: :flag, short: '-p' },
+            },
+          },
+          environment: {
+            options: {
+              cachedir: { type: :required, value: '/path/to/cachedir' },
+              'default-branch-override': { type: :required, value: 'branch-override' },
+              'generate-types': { type: :flag },
+              'no-force': { type: :flag },
+              'puppet-path': { type: :required, value: '/path/to/puppet' },
+              puppetfile: { type: :flag, short: '-p' },
+            },
+            extra_args: {
+              'environments' => %w[environment1 environment2],
+            },
+            included_examples: ['deploy examples'],
+          },
+          module: {
+            options: {
+              environment: { type: :required, value: 'environment1', short: '-e' },
+              'generate-types': { type: :flag },
+              'no-force': { type: :flag },
+              'puppet-path': { type: :required, value: '/path/to/puppet' },
+            },
+            extra_args: {
+              'modules' => %w[module1 module2],
+            },
+            included_examples: ['deploy examples'],
+          },
+        },
+      },
+      help: {
+        options: {
+          verbose: { type: :flag, short: '-v' },
+        },
+        no_runner: true,
+        command: R10K::CLI.command.command_named('help'),
+      },
+      puppetfile: {
+        options: {},
+        subcommands: {
+          check: {
+            options: {
+              puppetfile: { type: :required, value: '/path/to/Puppetfile' },
+            },
+          },
+          install: {
+            options: {
+              moduledir: { type: :required, value: '/path/to/moduledir' },
+              puppetfile: { type: :required, value: '/path/to/Puppetfile' },
+              force: { type: :flag },
+            },
+          },
+          purge: {
+            options: {
+              moduledir: { type: :required, value: '/path/to/moduledir' },
+              puppetfile: { type: :required, value: '/path/to/Puppetfile' },
+            },
+          },
+        },
+      },
+      version: {
+        options: {},
+        no_runner: true,
+        included_examples: ['version examples'],
+      },
+    },
+  },
+}.freeze
+
+def add_commands(commands, parents = [])
+  commands.each do |cmd, cmdinfo|
+    args = parents + [cmd]
+    subargs = (parents + [cmd]).reject { |p| p == :r10k }
+    subargs_module_string = ([''] + subargs).map { |p| p.to_s.capitalize }.join('::')
+
+    # Collect options from parent commands
+    options = {}
+    args.each_index do |i|
+      keys = args.slice(0, i + 1).zip([:subcommands] * i).flatten.compact + [:options]
+      options.merge!(COMMANDS.dig(*keys))
+    end
+
+    context args.join(' ') do
+      let!(:command)     { cmdinfo[:command] || string_to_module("R10K::CLI#{subargs_module_string}").command }
+      let!(:prev_runner) { command.block }
+
+      let(:command_args) { subargs.map(&:to_s) }
+      let(:extra_args)   { [] }
+      let(:action_class) { string_to_module("R10K::Action#{subargs_module_string}") }
+
+      (cmdinfo[:included_examples] || []).each do |examples|
+        include_examples examples
+      end
+
+      if cmdinfo[:extra_args]
+        cmdinfo[:extra_args].each do |desc, extra_args|
+          context "without #{desc} specified" do
+            add_includes(cmdinfo, options)
+          end
+
+          context "with #{desc} specified" do
+            let(:extra_args) { ['--'] + extra_args }
+
+            add_includes(cmdinfo, options)
+          end
+        end
+      else
+        add_includes(cmdinfo, options)
+      end
+    end
+
+    add_commands(cmdinfo[:subcommands], parents + [cmd]) if cmdinfo[:subcommands]
+  end
+end
+
+def add_includes(cmdinfo, options)
+  include_examples 'Cri argument parsing', options, cmdinfo[:subcommands] ? true : false
+
+  return if cmdinfo[:subcommands] || cmdinfo[:no_runner]
+
+  before(:each) do
+    setup_mock_runner(command, R10K::Action::CriRunner.wrap(action_class))
+  end
+
+  after(:each) do
+    command.block = prev_runner
+  end
+
+  include_examples 'Action argument parsing', options
+end
+
+def check_command(command, parents = [])
+  command_key = parents.zip([:subcommands] * parents.size).flatten.compact + [command.name.to_sym]
+  subcommands = COMMANDS.dig(*command_key, :subcommands) || {}
+  options = COMMANDS.dig(*command_key, :options)
+
+  context "command #{(parents + [command.name]).join(' ')}" do
+    it 'command and option checks exist' do
+      expect(options).to be_an_instance_of(Hash), "No `#{command.name}: { options: {...} }` definition found in COMMANDS"
+      expect(command.option_definitions.map(&:long).sort).to eq(options.keys.map(&:to_s).sort)
+    end
+  end
+
+  command.commands.each do |cmd|
+    check_command(cmd, parents + [command.name.to_sym])
+  end
+end
+
+describe R10K::CLI do
+  subject(:r10k) { described_class.command }
+
+  include_context 'cli'
+
+  # r10k has many methods with `exit`, which cause rspec to immediately exit if not caught
+  # Tests should also not print to stdout or stderr unexpectedly
+  around(:each) do |example|
+    expect do
+      expect do
+        expect { example.run }.not_to output.to_stderr
+      end.not_to output.to_stdout
+    end.not_to raise_error
+  end
+
+  # Generate examples for each command defined in COMMANDS
+  add_commands(COMMANDS)
 
-RSpec.describe 'basic cli sanity check' do
-  it 'can load the R10K::CLI namespace' do
-    expect {
-      require 'r10k/cli'
-    }.not_to raise_exception
+  # Ensure each command and command option defined in r10k is defined in COMMANDS
+  context 'when checking all commands and options have checks' do
+    check_command(described_class.command)
   end
 end