(CAT-1991) - Skip missing dirs invalid_dir method

Prior to this commit, the invalid_directories? method in pwsh::util
would return true if a directory did not exist. We found this to cause
issues with updating packages which modifiy the LIB env, as missing
entries during install/uninstall/upgrade would raise an error and exit.

Now, we alter the check to skip entries that do not exist, and only
flag for an invalid directory if the path supplied is not a directory.

Author: jordanbreen28

.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/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