Merge pull request #334 from puppetlabs/cat-1991-skip_missing_dir_check

(CAT-1991) - Skip missing dirs invalid_dir method

Author: david22swan

.rubocop_todo.yml

@@ -138,10 +138,10 @@ RSpec/MultipleDescribes:
 RSpec/MultipleExpectations:
   Max: 15
 
-# Offense count: 107
+# Offense count: 109
 # Configuration parameters: AllowSubject.
 RSpec/MultipleMemoizedHelpers:
-  Max: 17
+  Max: 19
 
 # Offense count: 55
 # Configuration parameters: AllowedGroups.

lib/pwsh.rb

@@ -108,7 +108,7 @@ def initialize(cmd, args = [], options = {})
       @powershell_command = cmd
       @powershell_arguments = args
 
-      raise "Bad configuration for ENV['lib']=#{ENV['lib']} - invalid path" if Pwsh::Util.invalid_directories?(ENV['lib'])
+      warn "Bad configuration for ENV['lib']=#{ENV['lib']} - invalid path" if Pwsh::Util.invalid_directories?(ENV['lib'])
 
       if Pwsh::Util.on_windows?
         # Named pipes under Windows will automatically be mounted in \\.\pipe\...

lib/pwsh/util.rb

@@ -15,20 +15,16 @@ def on_windows?
       !!(host_os =~ /mswin|mingw/)
     end
 
-    # Verify paths specified are valid directories which exist.
-    #
-    # @return [Bool] true if any directories specified do not exist
+    # Verify paths specified are valid directories.
+    # Skips paths which do not exist.
+    # @return [Bool] true if any paths specified are not valid directories
     def invalid_directories?(path_collection)
-      invalid_paths = false
-
-      return invalid_paths if path_collection.nil? || path_collection.empty?
+      return false if path_collection.nil? || path_collection.empty?
 
-      paths = on_windows? ? path_collection.split(';') : path_collection.split(':')
-      paths.each do |path|
-        invalid_paths = true unless File.directory?(path) || path.empty?
-      end
+      delimiter = on_windows? ? ';' : ':'
+      paths = path_collection.split(delimiter)
 
-      invalid_paths
+      paths.any? { |path| !path.empty? && File.exist?(path) && !File.directory?(path) }
     end
 
     # Return a string or symbol converted to snake_case

spec/unit/pwsh/util_spec.rb

@@ -87,13 +87,15 @@
   end
 
   describe '.invalid_directories?' do
-    let(:valid_path_a)  { 'C:/some/folder' }
-    let(:valid_path_b)  { 'C:/another/folder' }
-    let(:valid_paths)   { 'C:/some/folder;C:/another/folder' }
-    let(:invalid_path)  { 'C:/invalid/path' }
-    let(:mixed_paths)   { 'C:/some/folder;C:/invalid/path;C:/another/folder' }
-    let(:empty_string)  { '' }
-    let(:empty_members) { 'C:/some/folder;;C:/another/folder' }
+    let(:valid_path_a)     { 'C:/some/folder' }
+    let(:valid_path_b)     { 'C:/another/folder' }
+    let(:valid_paths)      { 'C:/some/folder;C:/another/folder' }
+    let(:invalid_path)     { 'C:/invalid/path' }
+    let(:mixed_paths)      { 'C:/some/folder;C:/another/folder;C:/invalid/path' }
+    let(:empty_string)     { '' }
+    let(:file_path)        { 'C:/some/folder/file.txt' }
+    let(:non_existent_dir) { 'C:/some/dir/that/doesnt/exist' }
+    let(:empty_members)    { 'C:/some/folder;;C:/another/folder' }
 
     it 'returns false if passed nil' do
       expect(described_class.invalid_directories?(nil)).to be false
@@ -103,40 +105,70 @@
       expect(described_class.invalid_directories?('')).to be false
     end
 
+    it 'returns false if a file path is provided' do
+      expect(described_class.invalid_directories?(file_path)).to be false
+    end
+
     it 'returns false if one valid path is provided' do
       expect(described_class).to receive(:on_windows?).and_return(true)
+      expect(File).to receive(:exist?).with(valid_path_a).and_return(true)
       expect(File).to receive(:directory?).with(valid_path_a).and_return(true)
       expect(described_class.invalid_directories?(valid_path_a)).to be false
     end
 
     it 'returns false if a collection of valid paths is provided' do
       expect(described_class).to receive(:on_windows?).and_return(true)
       expect(File).to receive(:directory?).with(valid_path_a).and_return(true)
+      expect(File).to receive(:exist?).with(valid_path_a).and_return(true)
       expect(File).to receive(:directory?).with(valid_path_b).and_return(true)
+      expect(File).to receive(:exist?).with(valid_path_b).and_return(true)
       expect(described_class.invalid_directories?(valid_paths)).to be false
     end
 
     it 'returns true if there is only one path and it is invalid' do
       expect(described_class).to receive(:on_windows?).and_return(true)
+      expect(File).to receive(:exist?).with(invalid_path).and_return(true)
       expect(File).to receive(:directory?).with(invalid_path).and_return(false)
       expect(described_class.invalid_directories?(invalid_path)).to be true
     end
 
     it 'returns true if the collection has on valid and one invalid member' do
       expect(described_class).to receive(:on_windows?).and_return(true)
+      expect(File).to receive(:exist?).with(valid_path_a).and_return(true)
       expect(File).to receive(:directory?).with(valid_path_a).and_return(true)
+      expect(File).to receive(:exist?).with(valid_path_b).and_return(true)
       expect(File).to receive(:directory?).with(valid_path_b).and_return(true)
+      expect(File).to receive(:exist?).with(invalid_path).and_return(true)
       expect(File).to receive(:directory?).with(invalid_path).and_return(false)
       expect(described_class.invalid_directories?(mixed_paths)).to be true
     end
 
     it 'returns false if collection has empty members but other entries are valid' do
       expect(described_class).to receive(:on_windows?).and_return(true)
+      expect(File).to receive(:exist?).with(valid_path_a).and_return(true)
       expect(File).to receive(:directory?).with(valid_path_a).and_return(true)
+      expect(File).to receive(:exist?).with(valid_path_b).and_return(true)
       expect(File).to receive(:directory?).with(valid_path_b).and_return(true)
       allow(File).to receive(:directory?).with('')
       expect(described_class.invalid_directories?(empty_members)).to be false
     end
+
+    it 'returns true if a collection has valid members but also contains a file path' do
+      expect(described_class).to receive(:on_windows?).and_return(true)
+      expect(File).to receive(:exist?).with(valid_path_a).and_return(true)
+      expect(File).to receive(:directory?).with(valid_path_a).and_return(true)
+      expect(File).to receive(:exist?).with(file_path).and_return(true)
+      expect(File).to receive(:directory?).with(file_path).and_return(false)
+      expect(described_class.invalid_directories?("#{valid_path_a};#{file_path}")).to be true
+    end
+
+    it 'returns false if a collection has valid members but contains a non-existent dir path' do
+      expect(described_class).to receive(:on_windows?).and_return(true)
+      expect(File).to receive(:exist?).with(valid_path_a).and_return(true)
+      expect(File).to receive(:directory?).with(valid_path_a).and_return(true)
+      expect(File).to receive(:exist?).with(non_existent_dir).and_return(false)
+      expect(described_class.invalid_directories?("#{valid_path_a};#{non_existent_dir}")).to be false
+    end
   end
 
   describe '.snake_case' do