Skip to content
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
1 change: 0 additions & 1 deletion lib/linux_admin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

require 'linux_admin/common'
require 'linux_admin/exceptions'
require 'linux_admin/command_result'
require 'linux_admin/package'
require 'linux_admin/rpm'
require 'linux_admin/deb'
Expand Down
14 changes: 0 additions & 14 deletions lib/linux_admin/command_result.rb

This file was deleted.

100 changes: 3 additions & 97 deletions lib/linux_admin/common.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
require 'shellwords'
require 'awesome_spawn'

class LinuxAdmin
module Common
Expand All @@ -7,105 +7,11 @@ def cmd(cmd)
end

def run(cmd, options = {})
params = options[:params] || options[:parameters]

launch_params = {}
launch_params[:chdir] = options[:chdir] if options[:chdir]

output = ""
error = ""
status = nil
command_line = build_cmd(cmd, params)

begin
output, error = launch(command_line, launch_params)
status = exitstatus
ensure
output ||= ""
error ||= ""
self.exitstatus = nil
end
rescue Errno::ENOENT => err
raise NoSuchFileError.new(err.message) if NoSuchFileError.detected?(err.message)
raise
else
CommandResult.new(command_line, output, error, status)
AwesomeSpawn.run(cmd, options)
end

def run!(cmd, options = {})
params = options[:params] || options[:parameters]
command_result = run(cmd, options)

if command_result.exit_status != 0
message = "#{cmd} exit code: #{command_result.exit_status}"
raise CommandResultError.new(message, command_result)
end

command_result
end

private

def sanitize(params)
return [] if params.blank?
params.collect do |k, v|
v = case v
when Array; v.collect {|i| i.to_s.shellescape}
when NilClass; v
else v.to_s.shellescape
end
[k, v]
end
end

def assemble_params(sanitized_params)
sanitized_params.collect do |pair|
pair_joiner = pair.first.to_s.end_with?("=") ? "" : " "
pair.flatten.compact.join(pair_joiner)
end.join(" ")
end

def build_cmd(cmd, params = nil)
return cmd.to_s if params.blank?
"#{cmd} #{assemble_params(sanitize(params))}"
end

# IO pipes have a maximum size of 64k before blocking,
# so we need to read and write synchronously.
# http://stackoverflow.com/questions/13829830/ruby-process-spawn-stdout-pipe-buffer-size-limit/13846146#13846146
THREAD_SYNC_KEY = "LinuxAdmin-exitstatus"

def launch(cmd, spawn_options = {})
out_r, out_w = IO.pipe
err_r, err_w = IO.pipe
pid = Kernel.spawn(cmd, {:err => err_w, :out => out_w}.merge(spawn_options))
wait_for_process(pid, out_w, err_w)
wait_for_pipes(out_r, err_r)
end

def wait_for_process(pid, out_w, err_w)
self.exitstatus = :not_done
Thread.new(Thread.current) do |parent_thread|
_, status = Process.wait2(pid)
out_w.close
err_w.close
parent_thread[THREAD_SYNC_KEY] = status.exitstatus
end
end

def wait_for_pipes(out_r, err_r)
out = out_r.read
err = err_r.read
sleep(0.1) while exitstatus == :not_done
return out, err
end

def exitstatus
Thread.current[THREAD_SYNC_KEY]
end

def exitstatus=(value)
Thread.current[THREAD_SYNC_KEY] = value
AwesomeSpawn.run!(cmd, options)
end
end
end
21 changes: 1 addition & 20 deletions lib/linux_admin/exceptions.rb
Original file line number Diff line number Diff line change
@@ -1,24 +1,5 @@
class CommandResultError < StandardError
attr_reader :result

def initialize(message, result)
super(message)
@result = result
end
end

class LinuxAdmin
class NoSuchFileError < Errno::ENOENT
def initialize(message)
super(message.split("No such file or directory -").last.split(" ").first)
end

def self.detected?(message)
message.start_with?("No such file or directory -")
end
end

class CredentialError < CommandResultError
class CredentialError < AwesomeSpawn::CommandResultError
def initialize(result)
super("Invalid username or password", result)
end
Expand Down
4 changes: 2 additions & 2 deletions lib/linux_admin/registration_system/subscription_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class LinuxAdmin
class SubscriptionManager < RegistrationSystem
def run!(cmd, options = {})
super(cmd, options)
rescue CommandResultError => err
rescue AwesomeSpawn::CommandResultError => err
raise CredentialError.new(err.result) if err.result.error.downcase.include?("invalid username or password")
raise
end
Expand Down Expand Up @@ -140,4 +140,4 @@ def proxy_params(options)
config
end
end
end
end
1 change: 1 addition & 0 deletions linux_admin.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,6 @@ registration, updates, etc.
spec.add_dependency "activesupport", "> 3.2"
spec.add_dependency "inifile", "~> 2.0.2"
spec.add_dependency "more_core_extensions", "~> 1.1.2"
spec.add_dependency "awesome_spawn", "~> 1.1.0"
spec.add_dependency "nokogiri"
end
13 changes: 0 additions & 13 deletions spec/command_result_spec.rb

This file was deleted.

151 changes: 7 additions & 144 deletions spec/common_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,9 @@ class TestClass
Object.send(:remove_const, :TestClass)
end

let(:params) do
{
"--user" => "bob",
"--pass" => "P@$sw0^& |<>/-+*d%",
"--db" => nil,
"--desc=" => "Some Description",
nil => ["pkg1", "some pkg"]
}
end

let (:modified_params) do
params.to_a + [123, 456].collect {|pool| ["--pool", pool]}
end

subject { TestClass }

context ".cmd" do
context "#cmd" do
it "looks up local command from id" do
d = double(LinuxAdmin::Distro)
d.class::COMMANDS = {:sh => '/bin/sh'}
Expand All @@ -36,136 +22,13 @@ class TestClass
end
end

shared_examples_for "run" do
context "paramater and command handling" do
before do
subject.stub(:exitstatus => 0)
end

it "sanitizes crazy params" do
subject.should_receive(:launch).once.with("true --user bob --pass P@\\$sw0\\^\\&\\ \\|\\<\\>/-\\+\\*d\\% --db --desc=Some\\ Description pkg1 some\\ pkg --pool 123 --pool 456", {})
subject.send(run_method, "true", :params => modified_params)
end

it "sanitizes fixnum array params" do
subject.should_receive(:launch).once.with("true 1", {})
subject.send(run_method, "true", :params => {nil => [1]})
end

it "sanitizes Pathname option value" do
require 'pathname'
subject.should_receive(:launch).once.with("true /usr/bin/ruby", {})
subject.send(run_method, "true", :params => {nil => [Pathname.new("/usr/bin/ruby")]})
end

it "sanitizes Pathname option" do
require 'pathname'
subject.should_receive(:launch).once.with("true /usr/bin/ruby", {})
subject.send(run_method, "true", :params => {Pathname.new("/usr/bin/ruby") => nil})
end

it "as empty hash" do
subject.should_receive(:launch).once.with("true", {})
subject.send(run_method, "true", :params => {})
end

it "as nil" do
subject.should_receive(:launch).once.with("true", {})
subject.send(run_method, "true", :params => nil)
end

it "won't modify caller params" do
orig_params = params.dup
subject.stub(:launch)
subject.send(run_method, "true", :params => params)
expect(orig_params).to eq(params)
end

it "Pathname command" do
subject.should_receive(:launch).once.with("/usr/bin/ruby", {})
subject.send(run_method, Pathname.new("/usr/bin/ruby"), {})
end

it "Pathname command with params" do
subject.should_receive(:launch).once.with("/usr/bin/ruby -v", {})
subject.send(run_method, Pathname.new("/usr/bin/ruby"), :params => {"-v" => nil})
end

it "supports spawn's chdir option" do
subject.should_receive(:launch).once.with("true", {:chdir => ".."})
subject.send(run_method, "true", :chdir => "..")
end
end

context "with real execution" do
before do
Kernel.stub(:spawn).and_call_original
end

it "command ok exit ok" do
expect(subject.send(run_method, "true")).to be_kind_of CommandResult
end

it "command ok exit bad" do
if run_method == "run!"
error = nil

# raise_error with do/end block notation is broken in rspec-expectations 2.14.x
# and has been fixed in master but not yet released.
# See: https://github.com/rspec/rspec-expectations/commit/b0df827f4c12870aa4df2f20a817a8b01721a6af
expect {subject.send(run_method, "false")}.to raise_error {|e| error = e }
expect(error).to be_kind_of CommandResultError
expect(error.result).to be_kind_of CommandResult
else
expect {subject.send(run_method, "false")}.to_not raise_error
end
end

it "command bad" do
expect {subject.send(run_method, "XXXXX --user=bob")}.to raise_error(LinuxAdmin::NoSuchFileError, "No such file or directory - XXXXX")
end

context "#exit_status" do
it "command ok exit ok" do
expect(subject.send(run_method, "true").exit_status).to eq(0)
end

it "command ok exit bad" do
expect(subject.send(run_method, "false").exit_status).to eq(1) if run_method == "run"
end
end

context "#output" do
it "command ok exit ok" do
expect(subject.send(run_method, "echo \"Hello World\"").output).to eq("Hello World\n")
end

it "command ok exit bad" do
expect(subject.send(run_method, "echo 'bad' && false").output).to eq("bad\n") if run_method == "run"
end
end

context "#error" do
it "command ok exit ok" do
expect(subject.send(run_method, "echo \"Hello World\" >&2").error).to eq("Hello World\n")
end

it "command ok exit bad" do
expect(subject.send(run_method, "echo 'bad' >&2 && false").error).to eq("bad\n") if run_method == "run"
end
end
end
it "#run" do
AwesomeSpawn.should_receive(:run).with("echo", nil => "test")
subject.run("echo", nil => "test")
end

context ".run" do
include_examples "run" do
let(:run_method) {"run"}
end
end

context ".run!" do
include_examples "run" do
let(:run_method) {"run!"}
end
it "#run!" do
AwesomeSpawn.should_receive(:run!).with("echo", nil => "test")
subject.run!("echo", nil => "test")
end
end
4 changes: 2 additions & 2 deletions spec/deb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@
Supported: 9m
Task: kubuntu-desktop, kubuntu-full, kubuntu-active, kubuntu-active-desktop, kubuntu-active-full, kubuntu-active, edubuntu-desktop-gnome, ubuntustudio-font-meta
EOS
described_class.should_receive(:run).
described_class.should_receive(:run!).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fryguy Why did you change from :run to :run! ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the old specs were relying on an implementation detail of the old run code, specifically that run! called run. That is no longer the right assumption, so the tests were failing. I changed them to properly expect the method they are calling.

with(described_class::APT_CACHE_CMD, :params => ["show", "ruby"]).
and_return(CommandResult.new("", data, "", 0))
and_return(double(:output => data))
metadata = described_class.info("ruby")
metadata['package'].should == 'ruby'
metadata['priority'].should == 'optional'
Expand Down
3 changes: 1 addition & 2 deletions spec/disk_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@

it "returns [] on non-zero parted rc" do
disk = LinuxAdmin::Disk.new :path => '/dev/hda'
disk.stub(:exitstatus => 1)
disk.stub(:launch)
disk.should_receive(:run).and_return(double(:output => "", :exit_status => 1))
disk.partitions.should == []
end

Expand Down
Loading