Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace ChildProcess with Process.spawn #892

Merged
merged 13 commits into from
Feb 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,20 @@

appraise "cucumber_4" do
gem "cucumber", "~> 4.0"
gem "childprocess", "~> 2.0.0"
end

appraise "cucumber_5" do
gem "cucumber", "~> 5.0"
gem "childprocess", "~> 3.0.0"
end

appraise "cucumber_6" do
gem "cucumber", "~> 6.0"
gem "childprocess", "~> 4.0"
end

appraise "cucumber_7" do
gem "cucumber", "~> 7.0"
gem "childprocess", "~> 4.0"
end

appraise "cucumber_8" do
gem "cucumber", "~> 8.0"
gem "childprocess", "~> 4.0"
end
1 change: 0 additions & 1 deletion aruba.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ Gem::Specification.new do |spec|
}

spec.add_runtime_dependency "bundler", [">= 1.17", "< 3.0"]
spec.add_runtime_dependency "childprocess", [">= 2.0", "< 5.0"]
spec.add_runtime_dependency "contracts", [">= 0.16.0", "< 0.18.0"]
spec.add_runtime_dependency "cucumber", ">= 4.0", "< 9.0"
spec.add_runtime_dependency "rspec-expectations", "~> 3.4"
Expand Down
1 change: 0 additions & 1 deletion gemfiles/cucumber_4.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@
source "https://rubygems.org"

gem "cucumber", "~> 4.0"
gem "childprocess", "~> 2.0.0"

gemspec path: "../"
1 change: 0 additions & 1 deletion gemfiles/cucumber_5.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@
source "https://rubygems.org"

gem "cucumber", "~> 5.0"
gem "childprocess", "~> 3.0.0"

gemspec path: "../"
1 change: 0 additions & 1 deletion gemfiles/cucumber_6.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@
source "https://rubygems.org"

gem "cucumber", "~> 6.0"
gem "childprocess", "~> 4.0"

gemspec path: "../"
1 change: 0 additions & 1 deletion gemfiles/cucumber_7.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@
source "https://rubygems.org"

gem "cucumber", "~> 7.0"
gem "childprocess", "~> 4.0"

gemspec path: "../"
1 change: 0 additions & 1 deletion gemfiles/cucumber_8.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@
source "https://rubygems.org"

gem "cucumber", "~> 8.0"
gem "childprocess", "~> 4.0"

gemspec path: "../"
2 changes: 1 addition & 1 deletion lib/aruba/cucumber/command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
sleep 0.1
end
end
rescue ChildProcess::TimeoutError, Timeout::Error
rescue Timeout::Error
last_command_started.terminate
end

Expand Down
4 changes: 4 additions & 0 deletions lib/aruba/platforms/unix_platform.rb
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,10 @@ def which(program, path = ENV["PATH"])
def builtin_shell_commands
[]
end

def term_signal_supported?
true
end
end
end
end
19 changes: 1 addition & 18 deletions lib/aruba/platforms/windows_command_string.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ module Aruba
module Platforms
# This is a command which should be run
#
# This adds `cmd.exec` in front of commmand
#
# @private
class WindowsCommandString
def initialize(command, *arguments)
Expand All @@ -15,22 +13,7 @@ def initialize(command, *arguments)

# Convert to array
def to_a
[cmd_path, "/c", [escaped_command, *escaped_arguments].join(" ")]
end

private

def escaped_arguments
@arguments.map { |arg| arg.gsub(/"/, '"""') }
.map { |arg| / /.match?(arg) ? "\"#{arg}\"" : arg }
end

def escaped_command
@command.gsub(/ /, '""" """')
end

def cmd_path
Aruba.platform.which("cmd.exe")
[@command, *@arguments]
end
end
end
Expand Down
4 changes: 4 additions & 0 deletions lib/aruba/platforms/windows_platform.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ def which(program, path = ENV["PATH"])
def builtin_shell_commands
%w(cd dir echo exit set type)
end

def term_signal_supported?
false
end
end
end
end
111 changes: 92 additions & 19 deletions lib/aruba/processes/spawn_process.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
require "childprocess"
require "tempfile"
require "shellwords"

Expand All @@ -10,6 +9,85 @@
module Aruba
# Platforms
module Processes
# Wrapper around Process.spawn that broadly follows the ChildProcess interface
# @private
class ProcessRunner
def initialize(command_array)
@command_array = command_array
@exit_status = nil
end

attr_accessor :stdout, :stderr, :cwd, :environment
attr_reader :command_array, :pid

def start
@stdin_r, @stdin_w = IO.pipe
@pid = Process.spawn(environment, *command_array,
unsetenv_others: true,
in: @stdin_r,
out: stdout.fileno,
err: stderr.fileno,
close_others: true,
chdir: cwd)
end

def stdin
@stdin_w
end

def stop
return if @exit_status

if Aruba.platform.term_signal_supported?
send_signal "TERM"
return if poll_for_exit(3)
end

send_signal "KILL"
wait
end

def wait
_, status = Process.waitpid2 @pid
@exit_status = status
end

def exited?
return true if @exit_status

pid, status = Process.waitpid2 @pid, Process::WNOHANG | Process::WUNTRACED

if pid
@exit_status = status
return true
end

false
end

def poll_for_exit(exit_timeout)
start = Time.now
wait_until = start + exit_timeout
loop do
return true if exited?
break if Time.now >= wait_until

sleep 0.1
end
false
end

def exit_code
@exit_status&.exitstatus
end

private

def send_signal(signal)
Process.kill signal, @pid
end
end

# Spawn a process for command
#
# `SpawnProcess` is not meant for direct use - `SpawnProcess.new` - by
Expand Down Expand Up @@ -74,7 +152,8 @@ def start

@started = true

@process = ChildProcess.build(*command_string.to_a)
@process = ProcessRunner.new(command_string.to_a)

@stdout_file = Tempfile.new("aruba-stdout-")
@stderr_file = Tempfile.new("aruba-stderr-")

Expand All @@ -85,21 +164,19 @@ def start
@stderr_file.set_encoding("ASCII-8BIT")

@exit_status = nil
@duplex = true

before_run

@process.io.stdout = @stdout_file
@process.io.stderr = @stderr_file
@process.duplex = @duplex
@process.cwd = @working_directory
@process.stdout = @stdout_file
@process.stderr = @stderr_file
@process.cwd = @working_directory

@process.environment.update(environment)
@process.environment = environment

begin
@process.start
sleep startup_wait_time
rescue ChildProcess::LaunchError => e
rescue SystemCallError => e
raise LaunchError, "It tried to start #{commandline}. " + e.message
end

Expand Down Expand Up @@ -129,7 +206,7 @@ def stdout(opts = {})
return @stdout_cache if stopped?

wait_for_io opts.fetch(:wait_for_io, io_wait_timeout) do
@process.io.stdout.flush
@process.stdout.flush
open(@stdout_file.path).read
end
end
Expand All @@ -148,16 +225,16 @@ def stderr(opts = {})
return @stderr_cache if stopped?

wait_for_io opts.fetch(:wait_for_io, io_wait_timeout) do
@process.io.stderr.flush
@process.stderr.flush
open(@stderr_file.path).read
end
end

def write(input)
return if stopped?

@process.io.stdin.write(input)
@process.io.stdin.flush
@process.stdin.write(input)
@process.stdin.flush

self
end
Expand All @@ -166,18 +243,14 @@ def write(input)
def close_io(name)
return if stopped?

@process.io.public_send(name.to_sym).close
@process.public_send(name.to_sym).close
end

# Stop command
def stop(*)
return @exit_status if stopped?

begin
@process.poll_for_exit(@exit_timeout)
rescue ChildProcess::TimeoutError
@timed_out = true
end
@process.poll_for_exit(@exit_timeout) or @timed_out = true

terminate
end
Expand Down
11 changes: 3 additions & 8 deletions spec/aruba/platforms/windows_command_string_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,19 @@

RSpec.describe Aruba::Platforms::WindowsCommandString do
let(:command_string) { described_class.new(base_command, *arguments) }
let(:cmd_path) { 'C:\Some Path\cmd.exe' }
let(:arguments) { [] }

before do
allow(Aruba.platform).to receive(:which).with("cmd.exe").and_return(cmd_path)
end

describe "#to_a" do
context "with a command with a path" do
let(:base_command) { "C:\\Foo\\Bar" }

it { expect(command_string.to_a).to eq [cmd_path, "/c", base_command] }
it { expect(command_string.to_a).to eq [base_command] }
end

context "with a command with a path containing spaces" do
let(:base_command) { "C:\\Foo Bar\\Baz" }

it { expect(command_string.to_a).to eq [cmd_path, "/c", 'C:\Foo""" """Bar\Baz'] }
it { expect(command_string.to_a).to eq [base_command] }
end

context "with a command and arguments" do
Expand All @@ -28,7 +23,7 @@

it {
expect(command_string.to_a)
.to eq [cmd_path, "/c", "#{base_command} -w \"baz quux\""]
.to eq [base_command, *arguments]
}
end
end
Expand Down
Loading