Skip to content

Commit c5101bd

Browse files
authored
Re-enable the full Windows test matrix and fix platform failures (#911)
Windows CI has been disabled since #836. Restore the full test matrix on Windows for every supported Ruby (2.6–4.0), alongside Linux, and remove the Windows and AppVeyor exclusions from the specs. Give the slower Windows jobs 30 minutes to complete the integration suite while retaining the 15-minute Linux limit. Both platforms now run the same 1,679 examples; no examples are removed or newly skipped. The restored matrix exposed two production failures: - Reading merge state with `File.open(...).read` leaves handles open until garbage collection, preventing Windows from deleting `MERGE_MODE`. Use `File.read` so the handles close immediately. Also close readers and temporary files in the Gerrit specs, and retain commit-message fixtures until each example finishes, to prevent Windows finalizer errors. - Parent-command detection depends on WMIC, which is unavailable on current Windows runners. Query `Win32_Process` through PowerShell's `Get-CimInstance`, preserving the `nil` fallback when the command cannot launch or the parent has exited. Use Ruby's native symlink APIs on Windows instead of invoking `mklink` and parsing `dir` output. This supports directory links, raises on creation failures, and lets the restored specs exercise real symlinks with `core.symlinks=true`. Ten additional examples cover files, directories, missing targets, spaces, and creation errors. Repair the parallelism regression spec: its previous built-in hook ignored the configured sleep command. An ad hoc serial hook now actually sleeps and writes a marker; the spec requires a successful commit and verifies the marker. Allow 30 seconds for Git and interpreter startup on slower Windows runners instead of the flaky five-second deadline. Two additional examples cover unavailable Windows process information. Validation: - Local Ruby 3.4.7: full suite with 1,679 examples and zero failures; the strengthened parallelism example and all 52 commit-message context examples also pass independently. - [Tests](https://github.com/sds/overcommit/actions/runs/41940143707): all 16 Linux/Windows jobs pass, each with 1,679 examples, zero failures, and zero pending examples. All Windows logs are free of tempfile finalizer errors. - [Lint](https://github.com/sds/overcommit/actions/runs/41940143568), coverage finalization, and all 16 Coveralls statuses pass. All 34 PR checks are green on `03e86dc5`. Refs #836 and #910.
1 parent 51a541d commit c5101bd

15 files changed

Lines changed: 253 additions & 235 deletions

File tree

‎.github/workflows/tests.yml‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ on:
88
jobs:
99
rspec:
1010
name: rspec (${{ matrix.ruby-version }}, ${{ matrix.os }})
11-
timeout-minutes: 15
11+
timeout-minutes: ${{ matrix.os == 'windows' && 30 || 15 }}
1212
runs-on: ${{ matrix.runner || format('{0}-latest', matrix.os) }}
1313

1414
strategy:
@@ -25,10 +25,7 @@ jobs:
2525
- "4.0"
2626
os:
2727
- ubuntu
28-
# At the moment of this commit various specs fail on Windows.
29-
# Any contributor is welcome to fix them and enable the Windows build.
30-
# Please see Issue #836 for more details.
31-
# - windows
28+
- windows
3229
include:
3330
- os: ubuntu
3431
runner: ubuntu-26.04

‎lib/overcommit/git_repo.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,10 +163,10 @@ def store_merge_state
163163
end
164164

165165
merge_mode_file = Overcommit::Utils.git_path('MERGE_MODE')
166-
@merge_mode = File.open(merge_mode_file).read if File.exist?(merge_mode_file)
166+
@merge_mode = File.read(merge_mode_file) if File.exist?(merge_mode_file)
167167

168168
merge_msg_file = Overcommit::Utils.git_path('MERGE_MSG')
169-
@merge_msg = File.open(merge_msg_file).read if File.exist?(merge_msg_file)
169+
@merge_msg = File.read(merge_msg_file) if File.exist?(merge_msg_file)
170170
end
171171

172172
# Store any relevant files that are present when repo is in the middle of a

‎lib/overcommit/utils.rb‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -166,17 +166,20 @@ def parent_command
166166
return if Process.ppid.zero?
167167

168168
if OS.windows?
169-
`wmic process where ProcessId=#{Process.ppid} get CommandLine /FORMAT:VALUE`.
170-
strip.
171-
slice(/(?<=CommandLine=).+/)
169+
result = execute([
170+
'powershell.exe', '-NoProfile', '-NonInteractive', '-Command',
171+
"(Get-CimInstance Win32_Process -Filter 'ProcessId = #{Process.ppid}').CommandLine",
172+
])
173+
command = result.stdout.strip
174+
command if result.success? && !command.empty?
172175
elsif OS.cygwin?
173176
# Cygwin's `ps` command behaves differently than the traditional
174177
# Linux version, but a comparable `procps` is provided to compensate.
175178
`procps -ocommand= -p #{Process.ppid}`.chomp
176179
else
177180
`ps -ocommand= -p #{Process.ppid}`.chomp
178181
end
179-
rescue Errno::EPERM, Errno::ENOENT
182+
rescue Errno::EPERM, Errno::ENOENT, ChildProcess::LaunchError
180183
# Process information may not be available, such as inside sandboxed environments
181184
nil
182185
end

‎lib/overcommit/utils/file_utils.rb‎

Lines changed: 3 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,70 +1,19 @@
11
# frozen_string_literal: true
22

3-
require 'overcommit/os'
4-
require 'overcommit/subprocess'
5-
63
module Overcommit::Utils
74
# Utility functions for file IO.
85
module FileUtils
96
class << self
10-
# When the host OS is Windows, uses the `mklink` command to create an
11-
# NTFS symbolic link from `new_name` to `old_name`. Otherwise delegates
12-
# to `File.symlink`
137
def symlink(old_name, new_name)
14-
return File.symlink(old_name, new_name) unless Overcommit::OS.windows?
15-
16-
result = win32_mklink_cmd(old_name, new_name)
17-
result.status
8+
File.symlink(old_name, new_name)
189
end
1910

20-
# When the host OS is Windows, uses the `dir` command to check whether
21-
# `file_name` is an NTFS symbolic link. Otherwise delegates to
22-
# `File.symlink`.
2311
def symlink?(file_name)
24-
return File.symlink?(file_name) unless Overcommit::OS.windows?
25-
26-
result = win32_dir_cmd(file_name)
27-
win32_symlink?(result.stdout)
12+
File.symlink?(file_name)
2813
end
2914

30-
# When the host OS is Windows, uses the `dir` command to check whether
31-
# `link_name` is an NTFS symbolic link. If so, it parses the target from
32-
# the command output. Otherwise raises an `ArgumentError`. Delegates to
33-
# `File.readlink` if the host OS is not Windows.
3415
def readlink(link_name)
35-
return File.readlink(link_name) unless Overcommit::OS.windows?
36-
37-
result = win32_dir_cmd(link_name)
38-
39-
unless win32_symlink?(result.stdout)
40-
raise ArgumentError, "#{link_name} is not a symlink"
41-
end
42-
43-
# Extract symlink target from output, which looks like:
44-
# 11/13/2012 12:53 AM <SYMLINK> mysymlink [C:\Windows\Temp\somefile.txt]
45-
result.stdout[/\[(.+)\]/, 1]
46-
end
47-
48-
private
49-
50-
def win32_dir_cmd(file_name)
51-
Overcommit::Subprocess.spawn(
52-
%W[cmd.exe /c dir #{win32_fix_pathsep(file_name)}]
53-
)
54-
end
55-
56-
def win32_mklink_cmd(old_name, new_name)
57-
Overcommit::Subprocess.spawn(
58-
%W[cmd.exe /c mklink #{win32_fix_pathsep(new_name)} #{win32_fix_pathsep(old_name)}]
59-
)
60-
end
61-
62-
def win32_fix_pathsep(path)
63-
path.tr(File::SEPARATOR, Overcommit::OS::SEPARATOR)
64-
end
65-
66-
def win32_symlink?(dir_output)
67-
!(dir_output =~ /<SYMLINK>/).nil?
16+
File.readlink(link_name)
6817
end
6918
end
7019
end

‎spec/integration/parallelize_spec.rb‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@
99
let(:config) { <<-YML }
1010
concurrency: 20
1111
CommitMsg:
12-
TrailingPeriod:
12+
SerialHook:
1313
enabled: true
1414
parallelize: false
15-
command: ['ruby', '-e', 'sleep 1']
15+
required_executable: ruby
16+
command: ['ruby', '-e', 'sleep 1; File.write("serial-hook-ran", "done")']
1617
TextWidth:
1718
enabled: true
1819
parallelize: true
@@ -27,14 +28,11 @@
2728
end
2829
end
2930

30-
# Test fails on Ruby 3.0 on Windows but nothing else. Would glady accept a pull
31-
# request that resolves.
32-
unless Overcommit::OS.windows? &&
33-
Overcommit::Utils::Version.new(RUBY_VERSION) >= '3' &&
34-
Overcommit::Utils::Version.new(RUBY_VERSION) < '3.1'
35-
it 'does not hang' do
36-
result = Timeout.timeout(5) { subject }
37-
result.stderr.should_not include 'No live threads left. Deadlock?'
38-
end
31+
it 'does not hang' do
32+
# Include Git and hook interpreter startup in the deadline on slower runners.
33+
result = Timeout.timeout(30) { subject }
34+
expect(result.status).to eq(0), result.stdout + result.stderr
35+
File.read('serial-hook-ran').should == 'done'
36+
result.stderr.should_not include 'No live threads left. Deadlock?'
3937
end
4038
end

‎spec/integration/resolving_cherry_pick_conflict_spec.rb‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,15 @@
3939
end
4040

4141
it 'exits with a non-zero status' do
42-
skip 'Skipping flakey test on AppVeyor Windows builds' if ENV['APPVEYOR']
4342
subject.status.should_not == 0
4443
end
4544

4645
it 'does not remove the CHERRY_PICK_HEAD file' do
47-
skip 'Skipping flakey test on AppVeyor Windows builds' if ENV['APPVEYOR']
4846
subject
4947
Dir['.git/*'].should include '.git/CHERRY_PICK_HEAD'
5048
end
5149

5250
it 'keeps the commit message from the cherry-picked commit' do
53-
skip 'Skipping flakey test on AppVeyor Windows builds' if ENV['APPVEYOR']
5451
subject
5552
File.read(File.join('.git', 'MERGE_MSG')).should include 'Add Branch 2 addition'
5653
end

‎spec/overcommit/hook/commit_msg/gerrit_change_id_spec.rb‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,18 @@
1414
context.stub(:commit_message_file).and_return(commit_msg_file.path)
1515
end
1616

17+
after do
18+
commit_msg_file.close!
19+
end
20+
1721
context 'when the commit message contains no Change-Id' do
1822
let(:commit_msg) { 'Add code to repo' }
1923

2024
it { should pass }
2125

2226
it 'adds a Change-Id to the commit message' do
2327
subject.run
24-
File.open(commit_msg_file.path, 'r').read.should =~ /Change-Id/
28+
File.read(commit_msg_file.path).should =~ /Change-Id/
2529
end
2630
end
2731

@@ -31,9 +35,9 @@
3135
it { should pass }
3236

3337
it 'does nothing' do
34-
before = File.open(commit_msg_file.path, 'r').read
38+
before = File.read(commit_msg_file.path)
3539
subject.run
36-
after = File.open(commit_msg_file.path, 'r').read
40+
after = File.read(commit_msg_file.path)
3741
before.should == after
3842
end
3943
end

‎spec/overcommit/hook/pre_commit/line_endings_spec.rb‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,9 @@
7070
end
7171
end
7272

73-
unless Overcommit::OS.windows?
74-
context 'when attempting to check a binary file' do
75-
let(:contents) { "\xFF\xD8\xFF\xE0\u0000\u0010JFIF" }
73+
context 'when attempting to check a binary file' do
74+
let(:contents) { "\xFF\xD8\xFF\xE0\u0000\u0010JFIF" }
7675

77-
it { should warn }
78-
end
76+
it { should warn }
7977
end
8078
end

‎spec/overcommit/hook_context/commit_msg_spec.rb‎

Lines changed: 56 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,16 @@
2424
]
2525
end
2626

27-
let(:commit_message_file) do
27+
let(:commit_message_tempfile) do
2828
Tempfile.new('commit-message').tap do |file|
2929
file.write(commit_msg.join("\n"))
30-
file.fsync
31-
end.path
30+
file.close
31+
end
32+
end
33+
let(:commit_message_file) { commit_message_tempfile.path }
34+
35+
after do
36+
commit_message_tempfile.close!
3237
end
3338

3439
describe '#commit_message' do
@@ -317,22 +322,19 @@
317322
end
318323
end
319324

320-
# Git cannot track Windows symlinks
321-
unless Overcommit::OS.windows?
322-
context 'when a broken symlink is staged' do
323-
around do |example|
324-
repo do
325-
Overcommit::Utils::FileUtils.symlink('non-existent-file', 'symlink')
326-
`git add symlink`
327-
example.run
328-
end
325+
context 'when a broken symlink is staged' do
326+
around do |example|
327+
repo do
328+
Overcommit::Utils::FileUtils.symlink('non-existent-file', 'symlink')
329+
`git add symlink`
330+
example.run
329331
end
332+
end
330333

331-
it 'does not attempt to update/restore the modification time of the file' do
332-
File.should_not_receive(:mtime)
333-
File.should_not_receive(:utime)
334-
subject
335-
end
334+
it 'does not attempt to update/restore the modification time of the file' do
335+
File.should_not_receive(:mtime)
336+
File.should_not_receive(:utime)
337+
subject
336338
end
337339
end
338340
end
@@ -657,54 +659,51 @@
657659
end
658660
end
659661

660-
# Git cannot track Windows symlinks
661-
unless Overcommit::OS.windows?
662-
context 'when changing a symlink to a directory during an amendment' do
663-
around do |example|
664-
repo do
665-
`git commit --allow-empty -m "Initial commit"`
666-
FileUtils.mkdir 'some-directory'
667-
symlink('some-directory', 'some-symlink')
668-
`git add some-symlink some-directory`
669-
`git commit -m "Add file"`
670-
`git rm some-symlink`
671-
FileUtils.mkdir 'some-symlink'
672-
touch File.join('some-symlink', 'another-file')
673-
`git add some-symlink`
674-
example.run
675-
end
662+
context 'when changing a symlink to a directory during an amendment' do
663+
around do |example|
664+
repo do
665+
`git commit --allow-empty -m "Initial commit"`
666+
FileUtils.mkdir 'some-directory'
667+
symlink('some-directory', 'some-symlink')
668+
`git add some-symlink some-directory`
669+
`git commit -m "Add file"`
670+
`git rm some-symlink`
671+
FileUtils.mkdir 'some-symlink'
672+
touch File.join('some-symlink', 'another-file')
673+
`git add some-symlink`
674+
example.run
676675
end
676+
end
677677

678-
before do
679-
context.stub(:amendment?).and_return(true)
680-
end
678+
before do
679+
context.stub(:amendment?).and_return(true)
680+
end
681681

682-
it 'does not include the directory in the list of modified files' do
683-
subject.should_not include File.expand_path('some-symlink')
684-
end
682+
it 'does not include the directory in the list of modified files' do
683+
subject.should_not include File.expand_path('some-symlink')
685684
end
685+
end
686686

687-
context 'when breaking a symlink during an amendment' do
688-
around do |example|
689-
repo do
690-
`git commit --allow-empty -m "Initial commit"`
691-
FileUtils.mkdir 'some-directory'
692-
touch File.join('some-directory', 'some-file')
693-
symlink('some-directory', 'some-symlink')
694-
`git add some-symlink some-directory`
695-
`git commit -m "Add file"`
696-
`git rm -rf some-directory`
697-
example.run
698-
end
687+
context 'when breaking a symlink during an amendment' do
688+
around do |example|
689+
repo do
690+
`git commit --allow-empty -m "Initial commit"`
691+
FileUtils.mkdir 'some-directory'
692+
touch File.join('some-directory', 'some-file')
693+
symlink('some-directory', 'some-symlink')
694+
`git add some-symlink some-directory`
695+
`git commit -m "Add file"`
696+
`git rm -rf some-directory`
697+
example.run
699698
end
699+
end
700700

701-
before do
702-
context.stub(:amendment?).and_return(true)
703-
end
701+
before do
702+
context.stub(:amendment?).and_return(true)
703+
end
704704

705-
it 'still includes the broken symlink in the list of modified files' do
706-
subject.should include File.expand_path('some-symlink')
707-
end
705+
it 'still includes the broken symlink in the list of modified files' do
706+
subject.should include File.expand_path('some-symlink')
708707
end
709708
end
710709
end

0 commit comments

Comments
 (0)