Skip to content

Fix Pathname support #43

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 28, 2013
Merged

Fix Pathname support #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a37a37b
Add example confirming Pathnames can be used as param values.
jrafanie Aug 23, 2013
cef1230
Add support for Pathname objects in commands and param options.
jrafanie Aug 26, 2013
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
4 changes: 2 additions & 2 deletions lib/linux_admin/common.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ def sanitize(params)

def assemble_params(sanitized_params)
sanitized_params.collect do |pair|
pair_joiner = pair.first.try(:end_with?, "=") ? "" : " "
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 if params.blank?
return cmd.to_s if params.blank?
"#{cmd} #{assemble_params(sanitize(params))}"
end

Expand Down
26 changes: 24 additions & 2 deletions spec/common_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class TestClass
end

shared_examples_for "run" do
context "with params" do
context "paramater and command handling" do
before do
subject.stub(:exitstatus => 0)
end
Expand All @@ -49,7 +49,19 @@ class TestClass

it "sanitizes fixnum array params" do
subject.should_receive(:launch).once.with("true 1", {})
subject.send(run_method, "true", :params => { nil => [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})
Copy link
Member

Choose a reason for hiding this comment

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

Is this backwards? The key part of a key/value pair is the command line option itself (the "--whatever") Does it make sense to have a path name on the key side?

Copy link
Member Author

Choose a reason for hiding this comment

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

@Fryguy This example is contrived but I could see you running:

LinuxAdmin.run("/bin/some_script.sh", :params => {"/usr/bin/ruby" => nil})

If you wanted to use a Pathname instead of a String and it's not coming from user input, then I can imagine wanting to do this.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Fryguy what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

@jrafanie yeah, I think it's an ok test, although it is contrived, like you said. Basically, we are showing we can support anything as a key parameter.

end

it "as empty hash" do
Expand All @@ -69,6 +81,16 @@ class TestClass
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 => "..")
Expand Down