Merge pull request #118 from ripienaar/117

(#117) improve resolving user configs

lib/mcollective/util.rb

@@ -156,19 +156,23 @@ def self.choria_windows_prefix
       File.join(Dir::COMMON_APPDATA, "ChoriaIO", "choria")
     end
 
-    def self.mcollective_config_paths_for_user
+    def self.config_paths_for_user
       config_paths = []
 
-      begin
-        # File.expand_path will raise if HOME isn't set, catch it
-        user_path = File.expand_path("~/.mcollective")
-        config_paths << user_path
-      rescue Exception # rubocop:disable Lint/RescueException, Lint/SuppressedException
+      ["~/.choriarc", "~/.mcollective"].each do |f|
+        begin
+          # File.expand_path will raise if HOME isn't set, catch it
+          config_paths << File.expand_path(f)
+        rescue ArgumentError # rubocop:disable Lint/SuppressedException
+        end
       end
 
       if windows?
+        config_paths << File.join(choria_windows_prefix, "etc", "client.conf")
         config_paths << File.join(windows_prefix, "etc", "client.cfg")
       else
+        config_paths << "/etc/choria/client.conf"
+        config_paths << "/usr/local/etc/choria/client.conf"
         config_paths << "/etc/puppetlabs/mcollective/client.cfg"
         config_paths << "/etc/mcollective/client.cfg"
         config_paths << "/usr/local/etc/mcollective/client.cfg"
@@ -177,42 +181,30 @@ def self.mcollective_config_paths_for_user
       config_paths
     end
 
-    def self.choria_config_paths_for_user
-      config_paths = []
-
-      begin
-        # File.expand_path will raise if HOME isn't set, catch it
-        user_path = File.expand_path("~/.choriarc")
-        config_paths << user_path
-      rescue Exception # rubocop:disable Lint/RescueException, Lint/SuppressedException
-      end
-
-      if windows?
-        config_paths << File.join(choria_windows_prefix, "etc", "client.conf")
-      else
-        config_paths << "/etc/choria/client.conf"
-        config_paths << "/usr/local/etc/choria/client.conf"
-      end
-
-      config_paths
-    end
-
     # Picks the default user config file, priorities are first Choria ones then old MCollective ones
     #
     # In roughly this order, first to exist is used:
     #
     # - ~/.choriarc
-    # - APPData/ChoriaIO/choria/etc/client.conf on windows
-    # - /etc/choria/client.conf then
-    # - /usr/local/etc/choria/client.conf on unix
     # - ~/.mcollective
-    # - APPData/PuppetLabs/mcollective/etc/client.cfg on windows
+    #
+    # On Unix:
+    #
+    # - /etc/choria/client.conf
+    # - /usr/local/etc/choria/client.conf
     # - /etc/puppetlabs/mcollective/client.cfg
     # - /etc/mcollective/client.cfg
     # - /usr/local/etc/mcollective/client.cfg
+    #
+    # On Windows:
+    #
+    # - APPData/ChoriaIO/choria/etc/client.conf on windows
+    # - APPData/PuppetLabs/mcollective/etc/client.cfg on windows
     def self.config_file_for_user
-      config_paths = choria_config_paths_for_user + mcollective_config_paths_for_user
+      config_paths = config_paths_for_user
+
       found = config_paths.find_index { |file| File.readable?(file) } || 0
+
       config_paths[found]
     end
 

spec/unit/mcollective/util_spec.rb

@@ -188,12 +188,15 @@ module MCollective
       end
     end
 
-    describe "#mcollective_config_paths_for_user" do
+    describe "#config_paths_for_user" do
       it "should support unix" do
         Util.stubs(:windows?).returns(false)
 
-        expect(Util.mcollective_config_paths_for_user).to eq([
+        expect(Util.config_paths_for_user).to eq([
+          File.expand_path("~/.choriarc"),
           File.expand_path("~/.mcollective"),
+          "/etc/choria/client.conf",
+          "/usr/local/etc/choria/client.conf",
           "/etc/puppetlabs/mcollective/client.cfg",
           "/etc/mcollective/client.cfg",
           "/usr/local/etc/mcollective/client.cfg"
@@ -203,44 +206,22 @@ module MCollective
       if Util.windows?
         it "should support windows" do
           Util.stubs(:windows?).returns(true)
-          Util.stubs(:choria_windows_prefix).returns("C:/temp")
-
-          expect(Util.choria_config_paths_for_user).to eq([
-            File.expand_path("~/.mcollective"),
-            File.join("c:/temp", "etc", "client.cfg")
-          ])
-        end
-      end
-    end
-
-    describe "#choria_config_paths_for_user" do
-      it "should support unix" do
-        Util.stubs(:windows?).returns(false)
-
-        expect(Util.choria_config_paths_for_user).to eq([
-          File.expand_path("~/.choriarc"),
-          "/etc/choria/client.conf",
-          "/usr/local/etc/choria/client.conf"
-        ])
-      end
-
-      if Util.windows?
-        it "should support windows" do
-          Util.stubs(:windows?).returns(true)
-          Util.stubs(:choria_windows_prefix).returns("C:/temp")
+          Util.stubs(:choria_windows_prefix).returns("C:/temp/choria")
+          Util.stubs(:windows_prefix).returns("C:/temp/mcollective")
 
           expect(Util.choria_config_paths_for_user).to eq([
             File.expand_path("~/.choriarc"),
-            File.join("c:/temp", "etc", "client.conf")
+            File.expand_path("~/.mcollective"),
+            File.join("c:/", "temp", "choria", "etc", "client.conf"),
+            File.join("c:/", "temp", "mcollective", "etc", "client.cfg")
           ])
         end
       end
     end
 
     describe '#config_file_for_user' do
       it "should pick the first path that exist" do
-        Util.expects(:choria_config_paths_for_user).returns(["choria1", "choria2"])
-        Util.expects(:mcollective_config_paths_for_user).returns(["mcollective1", "mcollective2"])
+        Util.expects(:config_paths_for_user).returns(["choria1", "choria2", "mcollective1", "mcollective2"])
 
         File.expects(:readable?).with("choria1").returns(false)
         File.expects(:readable?).with("choria2").returns(false)