Re-enable the full Windows test matrix and fix platform failures

.github/workflows/tests.yml

@@ -8,7 +8,7 @@ on:
 jobs:
   rspec:
     name: rspec (${{ matrix.ruby-version }}, ${{ matrix.os }})
-    timeout-minutes: 15
+    timeout-minutes: ${{ matrix.os == 'windows' && 30 || 15 }}
     runs-on: ${{ matrix.runner || format('{0}-latest', matrix.os) }}
 
     strategy:
@@ -25,10 +25,7 @@ jobs:
           - "4.0"
         os:
           - ubuntu
-          # At the moment of this commit various specs fail on Windows.
-          # Any contributor is welcome to fix them and enable the Windows build.
-          # Please see Issue #836 for more details.
-          # - windows
+          - windows
         include:
           - os: ubuntu
             runner: ubuntu-26.04

lib/overcommit/git_repo.rb

@@ -163,10 +163,10 @@ def store_merge_state
       end
 
       merge_mode_file = Overcommit::Utils.git_path('MERGE_MODE')
-      @merge_mode = File.open(merge_mode_file).read if File.exist?(merge_mode_file)
+      @merge_mode = File.read(merge_mode_file) if File.exist?(merge_mode_file)
 
       merge_msg_file = Overcommit::Utils.git_path('MERGE_MSG')
-      @merge_msg = File.open(merge_msg_file).read if File.exist?(merge_msg_file)
+      @merge_msg = File.read(merge_msg_file) if File.exist?(merge_msg_file)
     end
 
     # Store any relevant files that are present when repo is in the middle of a

lib/overcommit/utils.rb

@@ -166,17 +166,20 @@ def parent_command
         return if Process.ppid.zero?
 
         if OS.windows?
-          `wmic process where ProcessId=#{Process.ppid} get CommandLine /FORMAT:VALUE`.
-            strip.
-            slice(/(?<=CommandLine=).+/)
+          result = execute([
+            'powershell.exe', '-NoProfile', '-NonInteractive', '-Command',
+            "(Get-CimInstance Win32_Process -Filter 'ProcessId = #{Process.ppid}').CommandLine",
+          ])
+          command = result.stdout.strip
+          command if result.success? && !command.empty?
         elsif OS.cygwin?
           # Cygwin's `ps` command behaves differently than the traditional
           # Linux version, but a comparable `procps` is provided to compensate.
           `procps -ocommand= -p #{Process.ppid}`.chomp
         else
           `ps -ocommand= -p #{Process.ppid}`.chomp
         end
-      rescue Errno::EPERM, Errno::ENOENT
+      rescue Errno::EPERM, Errno::ENOENT, ChildProcess::LaunchError
         # Process information may not be available, such as inside sandboxed environments
         nil
       end

lib/overcommit/utils/file_utils.rb

@@ -1,70 +1,19 @@
 # frozen_string_literal: true
 
-require 'overcommit/os'
-require 'overcommit/subprocess'
-
 module Overcommit::Utils
   # Utility functions for file IO.
   module FileUtils
     class << self
-      # When the host OS is Windows, uses the `mklink` command to create an
-      # NTFS symbolic link from `new_name` to `old_name`. Otherwise delegates
-      # to `File.symlink`
       def symlink(old_name, new_name)
-        return File.symlink(old_name, new_name) unless Overcommit::OS.windows?
-
-        result = win32_mklink_cmd(old_name, new_name)
-        result.status
+        File.symlink(old_name, new_name)
       end
 
-      # When the host OS is Windows, uses the `dir` command to check whether
-      # `file_name` is an NTFS symbolic link. Otherwise delegates to
-      # `File.symlink`.
       def symlink?(file_name)
-        return File.symlink?(file_name) unless Overcommit::OS.windows?
-
-        result = win32_dir_cmd(file_name)
-        win32_symlink?(result.stdout)
+        File.symlink?(file_name)
       end
 
-      # When the host OS is Windows, uses the `dir` command to check whether
-      # `link_name` is an NTFS symbolic link. If so, it parses the target from
-      # the command output. Otherwise raises an `ArgumentError`. Delegates to
-      # `File.readlink` if the host OS is not Windows.
       def readlink(link_name)
-        return File.readlink(link_name) unless Overcommit::OS.windows?
-
-        result = win32_dir_cmd(link_name)
-
-        unless win32_symlink?(result.stdout)
-          raise ArgumentError, "#{link_name} is not a symlink"
-        end
-
-        # Extract symlink target from output, which looks like:
-        #   11/13/2012 12:53 AM <SYMLINK> mysymlink [C:\Windows\Temp\somefile.txt]
-        result.stdout[/\[(.+)\]/, 1]
-      end
-
-      private
-
-      def win32_dir_cmd(file_name)
-        Overcommit::Subprocess.spawn(
-          %W[cmd.exe /c dir #{win32_fix_pathsep(file_name)}]
-        )
-      end
-
-      def win32_mklink_cmd(old_name, new_name)
-        Overcommit::Subprocess.spawn(
-          %W[cmd.exe /c mklink #{win32_fix_pathsep(new_name)} #{win32_fix_pathsep(old_name)}]
-        )
-      end
-
-      def win32_fix_pathsep(path)
-        path.tr(File::SEPARATOR, Overcommit::OS::SEPARATOR)
-      end
-
-      def win32_symlink?(dir_output)
-        !(dir_output =~ /<SYMLINK>/).nil?
+        File.readlink(link_name)
       end
     end
   end

spec/integration/parallelize_spec.rb

@@ -9,10 +9,11 @@
   let(:config) { <<-YML }
     concurrency: 20
     CommitMsg:
-      TrailingPeriod:
+      SerialHook:
         enabled: true
         parallelize: false
-        command: ['ruby', '-e', 'sleep 1']
+        required_executable: ruby
+        command: ['ruby', '-e', 'sleep 1; File.write("serial-hook-ran", "done")']
       TextWidth:
         enabled: true
         parallelize: true
@@ -27,14 +28,11 @@
     end
   end
 
-  # Test fails on Ruby 3.0 on Windows but nothing else. Would glady accept a pull
-  # request that resolves.
-  unless Overcommit::OS.windows? &&
-    Overcommit::Utils::Version.new(RUBY_VERSION) >= '3' &&
-    Overcommit::Utils::Version.new(RUBY_VERSION) < '3.1'
-    it 'does not hang' do
-      result = Timeout.timeout(5) { subject }
-      result.stderr.should_not include 'No live threads left. Deadlock?'
-    end
+  it 'does not hang' do
+    # Include Git and hook interpreter startup in the deadline on slower runners.
+    result = Timeout.timeout(30) { subject }
+    expect(result.status).to eq(0), result.stdout + result.stderr
+    File.read('serial-hook-ran').should == 'done'
+    result.stderr.should_not include 'No live threads left. Deadlock?'
   end
 end

spec/integration/resolving_cherry_pick_conflict_spec.rb

@@ -39,18 +39,15 @@
   end
 
   it 'exits with a non-zero status' do
-    skip 'Skipping flakey test on AppVeyor Windows builds' if ENV['APPVEYOR']
     subject.status.should_not == 0
   end
 
   it 'does not remove the CHERRY_PICK_HEAD file' do
-    skip 'Skipping flakey test on AppVeyor Windows builds' if ENV['APPVEYOR']
     subject
     Dir['.git/*'].should include '.git/CHERRY_PICK_HEAD'
   end
 
   it 'keeps the commit message from the cherry-picked commit' do
-    skip 'Skipping flakey test on AppVeyor Windows builds' if ENV['APPVEYOR']
     subject
     File.read(File.join('.git', 'MERGE_MSG')).should include 'Add Branch 2 addition'
   end

spec/overcommit/hook/commit_msg/gerrit_change_id_spec.rb

@@ -14,14 +14,18 @@
     context.stub(:commit_message_file).and_return(commit_msg_file.path)
   end
 
+  after do
+    commit_msg_file.close!
+  end
+
   context 'when the commit message contains no Change-Id' do
     let(:commit_msg) { 'Add code to repo' }
 
     it { should pass }
 
     it 'adds a Change-Id to the commit message' do
       subject.run
-      File.open(commit_msg_file.path, 'r').read.should =~ /Change-Id/
+      File.read(commit_msg_file.path).should =~ /Change-Id/
     end
   end
 
@@ -31,9 +35,9 @@
     it { should pass }
 
     it 'does nothing' do
-      before = File.open(commit_msg_file.path, 'r').read
+      before = File.read(commit_msg_file.path)
       subject.run
-      after = File.open(commit_msg_file.path, 'r').read
+      after = File.read(commit_msg_file.path)
       before.should == after
     end
   end

spec/overcommit/hook/pre_commit/line_endings_spec.rb

@@ -70,11 +70,9 @@
     end
   end
 
-  unless Overcommit::OS.windows?
-    context 'when attempting to check a binary file' do
-      let(:contents) { "\xFF\xD8\xFF\xE0\u0000\u0010JFIF" }
+  context 'when attempting to check a binary file' do
+    let(:contents) { "\xFF\xD8\xFF\xE0\u0000\u0010JFIF" }
 
-      it { should warn }
-    end
+    it { should warn }
   end
 end

spec/overcommit/hook_context/commit_msg_spec.rb

@@ -24,11 +24,16 @@
     ]
   end
 
-  let(:commit_message_file) do
+  let(:commit_message_tempfile) do
     Tempfile.new('commit-message').tap do |file|
       file.write(commit_msg.join("\n"))
-      file.fsync
-    end.path
+      file.close
+    end
+  end
+  let(:commit_message_file) { commit_message_tempfile.path }
+
+  after do
+    commit_message_tempfile.close!
   end
 
   describe '#commit_message' do
@@ -317,22 +322,19 @@
       end
     end
 
-    # Git cannot track Windows symlinks
-    unless Overcommit::OS.windows?
-      context 'when a broken symlink is staged' do
-        around do |example|
-          repo do
-            Overcommit::Utils::FileUtils.symlink('non-existent-file', 'symlink')
-            `git add symlink`
-            example.run
-          end
+    context 'when a broken symlink is staged' do
+      around do |example|
+        repo do
+          Overcommit::Utils::FileUtils.symlink('non-existent-file', 'symlink')
+          `git add symlink`
+          example.run
         end
+      end
 
-        it 'does not attempt to update/restore the modification time of the file' do
-          File.should_not_receive(:mtime)
-          File.should_not_receive(:utime)
-          subject
-        end
+      it 'does not attempt to update/restore the modification time of the file' do
+        File.should_not_receive(:mtime)
+        File.should_not_receive(:utime)
+        subject
       end
     end
   end
@@ -657,54 +659,51 @@
       end
     end
 
-    # Git cannot track Windows symlinks
-    unless Overcommit::OS.windows?
-      context 'when changing a symlink to a directory during an amendment' do
-        around do |example|
-          repo do
-            `git commit --allow-empty -m "Initial commit"`
-            FileUtils.mkdir 'some-directory'
-            symlink('some-directory', 'some-symlink')
-            `git add some-symlink some-directory`
-            `git commit -m "Add file"`
-            `git rm some-symlink`
-            FileUtils.mkdir 'some-symlink'
-            touch File.join('some-symlink', 'another-file')
-            `git add some-symlink`
-            example.run
-          end
+    context 'when changing a symlink to a directory during an amendment' do
+      around do |example|
+        repo do
+          `git commit --allow-empty -m "Initial commit"`
+          FileUtils.mkdir 'some-directory'
+          symlink('some-directory', 'some-symlink')
+          `git add some-symlink some-directory`
+          `git commit -m "Add file"`
+          `git rm some-symlink`
+          FileUtils.mkdir 'some-symlink'
+          touch File.join('some-symlink', 'another-file')
+          `git add some-symlink`
+          example.run
         end
+      end
 
-        before do
-          context.stub(:amendment?).and_return(true)
-        end
+      before do
+        context.stub(:amendment?).and_return(true)
+      end
 
-        it 'does not include the directory in the list of modified files' do
-          subject.should_not include File.expand_path('some-symlink')
-        end
+      it 'does not include the directory in the list of modified files' do
+        subject.should_not include File.expand_path('some-symlink')
       end
+    end
 
-      context 'when breaking a symlink during an amendment' do
-        around do |example|
-          repo do
-            `git commit --allow-empty -m "Initial commit"`
-            FileUtils.mkdir 'some-directory'
-            touch File.join('some-directory', 'some-file')
-            symlink('some-directory', 'some-symlink')
-            `git add some-symlink some-directory`
-            `git commit -m "Add file"`
-            `git rm -rf some-directory`
-            example.run
-          end
+    context 'when breaking a symlink during an amendment' do
+      around do |example|
+        repo do
+          `git commit --allow-empty -m "Initial commit"`
+          FileUtils.mkdir 'some-directory'
+          touch File.join('some-directory', 'some-file')
+          symlink('some-directory', 'some-symlink')
+          `git add some-symlink some-directory`
+          `git commit -m "Add file"`
+          `git rm -rf some-directory`
+          example.run
         end
+      end
 
-        before do
-          context.stub(:amendment?).and_return(true)
-        end
+      before do
+        context.stub(:amendment?).and_return(true)
+      end
 
-        it 'still includes the broken symlink in the list of modified files' do
-          subject.should include File.expand_path('some-symlink')
-        end
+      it 'still includes the broken symlink in the list of modified files' do
+        subject.should include File.expand_path('some-symlink')
       end
     end
   end