Fix exit status code on failures

features/cli.feature

@@ -4,18 +4,22 @@ Feature: CLI
   Scenario: When passing no arguments to the msync command
     When I run `msync`
     And the output should match /Commands:/
+    Then the exit status should be 1
 
   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/
+    Then the exit status should be 1
 
   Scenario: When passing invalid arguments to the msync hook command
     When I run `msync hook`
     And the output should match /Commands:/
+    Then the exit status should be 1
 
-  Scenario: When running the help subcommand
+  Scenario: When running the help command
     When I run `msync help`
     And the output should match /Commands:/
+    Then the exit status should be 0
 
   Scenario: When overriding a setting from the config file on the command line
     Given a puppet module "puppet-test" from "fakenamespace"

features/update.feature

@@ -430,7 +430,8 @@ Feature: update
 
   Scenario: Pulling a module that already exists in the modules directory
     Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
-    When I run `msync update`
+    And a directory named "moduleroot"
+    When I run `msync update --message "First update run"`
     Then the exit status should be 0
     And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"
     Given a file named "config_defaults.yml" with:
@@ -455,10 +456,10 @@ Feature: update
       """
     And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"
 
-  Scenario: When running update with no changes
+  Scenario: When running update without changes
     Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
     And a directory named "moduleroot"
-    When I run `msync update`
+    When I run `msync update --message "Running without changes"`
     Then the exit status should be 0
     And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"
 

features/update/bad_context.feature

@@ -0,0 +1,26 @@
+Feature: Run `msync update` without a good context
+
+  Scenario: Run `msync update` without any module
+    Given a directory named "moduleroot"
+    When I run `msync update --message "In a bad context"`
+    Then the exit status should be 1
+    And the stderr should contain:
+      """
+      No modules found
+      """
+
+  Scenario: Run `msync update` without the "moduleroot" directory
+    Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
+    When I run `msync update --message "In a bad context"`
+    Then the exit status should be 1
+    And the stderr should contain "moduleroot"
+
+  Scenario: Run `msync update` without commit message
+    Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
+    And a directory named "moduleroot"
+    When I run `msync update`
+    Then the exit status should be 1
+    And the stderr should contain:
+      """
+      No value provided for required option "--message"
+      """

lib/modulesync.rb

@@ -39,10 +39,10 @@ def self.find_template_files(local_template_dir)
           .collect { |p| p.chomp('.erb') }
           .to_a
     else
-      $stdout.puts "#{local_template_dir} does not exist." \
+      $stderr.puts "#{local_template_dir} does not exist." \
         ' Check that you are working in your module configs directory or' \
         ' that you have passed in the correct directory with -c.'
-      exit
+      exit 1
     end
   end
 
@@ -53,9 +53,9 @@ def self.relative_names(file_list, path)
   def self.managed_modules(config_file, filter, negative_filter)
     managed_modules = Util.parse_config(config_file)
     if managed_modules.empty?
-      $stdout.puts "No modules found in #{config_file}." \
+      $stderr.puts "No modules found in #{config_file}." \
         ' Check that you specified the right :configs directory and :managed_modules_conf file.'
-      exit
+      exit 1
     end
     managed_modules.select! { |m| m =~ Regexp.new(filter) } unless filter.nil?
     managed_modules.reject! { |m| m =~ Regexp.new(negative_filter) } unless negative_filter.nil?

lib/modulesync/cli.rb

@@ -1,10 +1,12 @@
 require 'thor'
+
 require 'modulesync'
+require 'modulesync/cli/thor'
 require 'modulesync/constants'
 require 'modulesync/util'
 
 module ModuleSync
-  class CLI
+  module CLI
     def self.defaults
       @defaults ||= Util.symbolize_keys(Util.parse_config(Constants::MODULESYNC_CONF_FILE))
     end

lib/modulesync/cli/thor.rb

@@ -0,0 +1,24 @@
+require 'thor'
+require 'modulesync/cli'
+
+module ModuleSync
+  module CLI
+    # Workaround some, still unfixed, Thor behaviors
+    #
+    # This class extends ::Thor class to
+    # - exit with status code sets to `1` on Thor failure (e.g. missing required option)
+    # - exit with status code sets to `1` when user calls `msync` (or a subcommand) without required arguments
+    class Thor < ::Thor
+      desc '_invalid_command_call', 'Invalid command', hide: true
+      def _invalid_command_call
+        self.class.new.help
+        exit 1
+      end
+      default_task :_invalid_command_call
+
+      def self.exit_on_failure?
+        true
+      end
+    end
+  end
+end