Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Make specs for ruby_exe :env option more fine grained.

  • Loading branch information...
commit 5aff4a48cc278b4992f14681571d8be9ff9edd90 1 parent cf5d38b
@brixen brixen authored
Showing with 40 additions and 17 deletions.
  1. +9 −7 lib/mspec/helpers/ruby_exe.rb
  2. +31 −10 spec/helpers/ruby_exe_spec.rb
View
16 lib/mspec/helpers/ruby_exe.rb
@@ -25,13 +25,15 @@
# The ruby_exe helper also accepts an options hash with three
# keys: :options, :args and :env. For example:
#
-# ruby_exe('file.rb', :options => "-w", :args => "> file.txt", :env => { :foo => "bar" })
+# ruby_exe('file.rb', :options => "-w",
+# :args => "> file.txt",
+# :env => { :FOO => "bar" })
#
# will be executed as
#
# `#{RUBY_EXE} -w #{'file.rb'} > file.txt`
#
-# with access to ENV["foo"] with value "bar"
+# with access to ENV["FOO"] with value "bar".
#
# If +nil+ is passed for the first argument, the command line
# will be built only from the options hash.
@@ -126,18 +128,18 @@ def ruby_exe(code, opts = {})
body = "-e #{code}"
end
- env_pairs = {}
- env.each_pair do |key,val|
+ saved_env = {}
+ env.each do |key, value|
key = key.to_s
- env_pairs[key] = ENV[key]
- ENV[key] = val
+ saved_env[key] = ENV[key] if ENV.key? key
@jc00ke
jc00ke added a note

I was setting saved_env[key] to ENV[key] regardless of its existence in ENV as then it would be nil'd out in the ensure clause. With this change won't we get ENV k/v pairs hanging around?

@brixen Owner
brixen added a note

Yeah, we can clear the keys in env if they aren't in saved_env.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ ENV[key] = value
end
begin
cmd = [RUBY_EXE, ENV['RUBY_FLAGS'], opts[:options], body, opts[:args]]
`#{cmd.compact.join(' ')}`
ensure
- env_pairs.each_pair{ |key,val| ENV[key] = val }
+ saved_env.each { |key, value| ENV[key] = value }
end
end
end
View
41 spec/helpers/ruby_exe_spec.rb
@@ -177,15 +177,36 @@ class RubyExeSpecs
@script.ruby_exe nil, :options => "-c", :args => "> file.txt"
end
- it "executes with env but without code or file" do
- ENV.should_receive(:[]).with("RUBY_FLAGS").and_return("-w -Q")
- ENV.should_receive(:[]).with("baz").and_return("a")
- ENV.should_receive(:[]=).with("baz", "b")
- ENV.should_receive(:[]).with("foo")
- ENV.should_receive(:[]=).with("foo", "bar")
- ENV.should_receive(:[]=).with("baz", "a")
- ENV.should_receive(:[]=).with("foo", nil)
- @script.should_receive(:`).with("ruby_spec_exe -w -Q -c")
- @script.ruby_exe nil, :env => { "foo" => "bar", :baz => "b" }, :options => "-c"
+ describe "with :env option" do
+ before :each do
@jc00ke
jc00ke added a note

We can remove the :each as that's assumed.

@brixen Owner
brixen added a note

no, do not remove :each ever.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ @script.stub!(:`)
+ end
+
+ after :each do
+ ENV.delete "ABC"
+ end
+
+ it "preserves the values of existing ENV keys" do
+ ENV["ABC"] = "123"
+ ENV.should_receive(:[]).with("RUBY_FLAGS")
+ ENV.should_receive(:[]).with("ABC")
+ @script.ruby_exe nil, :env => { :ABC => "xyz" }
+ end
+
+ it "adds the :env entries to ENV" do
+ ENV.should_receive(:[]=).with("ABC", "xyz")
+ @script.ruby_exe nil, :env => { :ABC => "xyz" }
+ end
+
+ it "resets the values of existing ENV keys when an exception is raised" do
@jc00ke
jc00ke added a note

Won't ensure ensure that the values are reset regardless of an exception being raised?

@brixen Owner
brixen added a note

Of course, the point of doing it in an ensure is in the case of an exception.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ ENV["ABC"] = "123"
+ ENV.should_receive(:[]=).with("ABC", "xyz")
+ ENV.should_receive(:[]=).with("ABC", "123")
+
+ @script.should_receive(:`).and_raise(Exception)
+ lambda do
+ @script.ruby_exe nil, :env => { :ABC => "xyz" }
+ end.should raise_error(Exception)
+ end
end
end
@jc00ke

Won't ensure ensure that the values are reset regardless of an exception being raised?

@jc00ke

We can remove the :each as that's assumed.

@jc00ke

I was setting saved_env[key] to ENV[key] regardless of its existence in ENV as then it would be nil'd out in the ensure clause. With this change won't we get ENV k/v pairs hanging around?

@brixen

no, do not remove :each ever.

@brixen

Of course, the point of doing it in an ensure is in the case of an exception.

@brixen

Yeah, we can clear the keys in env if they aren't in saved_env.

Please sign in to comment.
Something went wrong with that request. Please try again.