Allow environment variables to be passed in to the command #259

westlakem · 2013-09-12T22:07:56Z

Updated Gherkin runner, test runner, and cli.rb to accept the --environement-variable command. This will allow us to set shell environment variables as an option when we execute our test script.

Since tests get launched in a new shell, shell specific environment variables get lost. You can use the -en switch to add options to the existing options[:env]. by adding your key/value pair to the hash, it will be set in your test's environment.

…rom the command line

grosser · 2013-09-13T01:06:21Z

Readme.md

+    -m, --multiply-processes [FLOAT] 		use given number as a multiplier of processes to run
+    -s, --single [PATTERN]           		Run all matching files in the same process
+    -i, --isolate                    		Do not run any other tests in the group used by --single(-s)
+    -e, --exec [COMMAND]             		execute this code parallel and with ENV['TEST_ENV_NUM']


Hehe maybe --env for lazy people like me + no crazy indent changes :)

grosser · 2013-09-13T01:07:33Z

If setting an environment variable fixes it how why doesn't setting the variable outside work, is it reset when shelling out ?

grosser · 2013-09-13T01:09:15Z

lib/parallel_tests/gherkin/runner.rb

@@ -8,7 +8,7 @@ class Runner < ParallelTests::Test::Runner
      class << self
        def run_tests(test_files, process_number, num_processes, options)
          sanitized_test_files = test_files.map { |val| Shellwords.escape(val) }
-          options = options.merge(:env => {"AUTOTEST" => "1"}) if $stdout.tty? # display color when we are in a terminal
+          (options[:env] || {}).merge!("AUTOTEST" => "1") if $stdout.tty? # display color when we are in a terminal


won't work -> (options[:env] ||= {}) please also add a test :)

Mike,

I'm confused on how that won't work is there a scenario I'm missing that I didn't test for? Here is how I understood the code to work

The original code took a nil hash symbol (:env), and assigned it a hash ({"AUTOTEST" => "1"}).

at this point, options has some values in them, but nothing for env, hence:
options = options.merge(:env => {"AUTOTEST" => "1"}) if $stdout.tty?
takes the existing options hash and merges AUTOTEST => 1 into options[:env].

At this point, options[:env] has one value {AUTOTEST => 1}

With the change I made, if options[:env].nil? == true, we get the same result
options[:env].inspect = {AUTOTEST => 1}

If we use the --environment-variable '[option]' tag, AUTOTEST => 1 gets added to the existing env hash, much in the same way you did in lib/parallel_tests/test/runner.rb:53

So if we execute:parallel_cucumber -n 2 --environment-variable '_JAVA_OPTIONS=-Xmx2048m'
Then at this point options[:env].inspect = [{_JAVA_OPTIONS => -Xmx2048m} , {AUTOTEST => 1}]

I can send you some screen shots of the testing I did when I get to work tomorrow.

Also, I am used to Java unit testing but not ruby testing, hence why I did manual testing. It may take me some time to understand how to write unit tests for ruby code.

From: Michael Grosser notifications@github.com
To: grosser/parallel_tests parallel_tests@noreply.github.com
Cc: nightbeast westlake_m@Yahoo.com
Sent: Thursday, September 12, 2013 9:09 PM
Subject: Re: [parallel_tests] Allow environment variables to be passed in to the command (#259)

In lib/parallel_tests/gherkin/runner.rb:

@@ -8,7 +8,7 @@ class Runner < ParallelTests::Test::Runner
class << self
def run_tests(test_files, process_number, num_processes, options)
sanitized_test_files = test_files.map { |val| Shellwords.escape(val) }

options = options.merge(:env => {"AUTOTEST" => "1"}) if $stdout.tty? # display color when we are in a terminal

(options[:env] || {}).merge!("AUTOTEST" => "1") if $stdout.tty? # display color when we are in a terminal
won't work -> (options[:env] ||= {}) please also add a test :)
—
Reply to this email directly or view it on GitHub.

current version:

2.0.0-p247 :001 > options = {} => {} 2.0.0-p247 :002 > (options[:env] || {}).merge!("AUTOTEST" => "1") => {"AUTOTEST"=>"1"} 2.0.0-p247 :003 > options => {}

fixed version:

2.0.0-p247 :001 > options = {} => {} 2.0.0-p247 :002 > (options[:env] ||= {}).merge!("AUTOTEST" => "1") => {"AUTOTEST"=>"1"} 2.0.0-p247 :003 > options => {:env=>{"AUTOTEST"=>"1"}}

Ah ok... so AUTOTEST => 1 is always being set with this code... I'll look into different ways of doing it then.

From: Michael Grosser notifications@github.com
To: grosser/parallel_tests parallel_tests@noreply.github.com
Cc: nightbeast westlake_m@Yahoo.com
Sent: Thursday, September 12, 2013 11:40 PM
Subject: Re: [parallel_tests] Allow environment variables to be passed in to the command (#259)

In lib/parallel_tests/gherkin/runner.rb:

@@ -8,7 +8,7 @@ class Runner < ParallelTests::Test::Runner
class << self
def run_tests(test_files, process_number, num_processes, options)
sanitized_test_files = test_files.map { |val| Shellwords.escape(val) }

options = options.merge(:env => {"AUTOTEST" => "1"}) if $stdout.tty? # display color when we are in a terminal

(options[:env] || {}).merge!("AUTOTEST" => "1") if $stdout.tty? # display color when we are in a terminal
current version:
2.0.0-p247 :001 > options = {} => {}
2.0.0-p247 :002 > (options[:env] || {}).merge!("AUTOTEST" => "1") => {"AUTOTEST"=>"1"}
2.0.0-p247 :003 > options => {}
fixed version:
2.0.0-p247 :001 > options = {} => {}
2.0.0-p247 :002 > (options[:env] ||= {}).merge!("AUTOTEST" => "1") => {"AUTOTEST"=>"1"}
2.0.0-p247 :003 > options => {:env=>{"AUTOTEST"=>"1"}}
—
Reply to this email directly or view it on GitHub.

I already gave you the snippet that would work oO

(FYI your email client messes up github comments)

grosser · 2013-09-13T03:44:50Z

Test would be something like
https://github.com/grosser/parallel_tests/blob/master/spec/parallel_tests/test/runner_spec.rb#L384

    it "uses given env" do
      run_with_file("puts ENV['FOO']") do |path|
        result = call("ruby #{path}", 1, 4, {:env => {"FOO" => "XXX"}})
        result.should == {
          :stdout => "XXX\n",
          :exit_status => 0
        }
      end
    end

grosser · 2013-09-13T03:48:10Z

lib/parallel_tests/cli.rb

@@ -117,6 +117,7 @@ def parse_options!(argv)

        opts.on("-e", "--exec [COMMAND]", "execute this code parallel and with ENV['TEST_ENV_NUM']") { |path| options[:execute] = path }
        opts.on("-o", "--test-options '[OPTIONS]'", "execute test commands with those options") { |arg| options[:test_options] = arg }
+        opts.on("--environment-variables '[OPTION]'", "Add environment varaibles to tests") { |arg| options[:env] = {arg.split('=')[0] => arg.split('=')[1]}}


A more sophisticated splitting that also handles X="FOO = BAR" Y="BAR 'ZAR"

require 'shellwords' Hash[Shellwords.split("xxx=1 yyy=2").map {|x| x.split("=",2)}]

grosser · 2013-09-13T03:49:35Z

What would some example environment things be that you want to set ?

westlakem · 2013-09-13T11:06:41Z

So the execution is happening on a box that has other Java processes going on too, hence they will not let me change the system properties. If I could, I could set the environment variable in the profile, and it would be the default, but they don't want me changing the default for the risk of other Java processes not running correctly. Hence the only place I can set them is inside the shell. If each parallel task is spawned in a new shell, then the options from the parent shell do not get carried over. Being able to set environment variables in the children shells will fix my issue.

From: Michael Grosser notifications@github.com
To: grosser/parallel_tests parallel_tests@noreply.github.com
Cc: nightbeast westlake_m@Yahoo.com
Sent: Thursday, September 12, 2013 9:07 PM
Subject: Re: [parallel_tests] Allow environment variables to be passed in to the command (#259)

If setting an environment variable fixes it how why doesn't setting the variable outside work, is it reset when shelling out ?
—
Reply to this email directly or view it on GitHub.

westlakem · 2013-09-13T11:10:21Z

--env '_JAVA_OPTIONS=-Xms64m -Xmx2048m'

From: Michael Grosser notifications@github.com
To: grosser/parallel_tests parallel_tests@noreply.github.com
Cc: nightbeast westlake_m@Yahoo.com
Sent: Thursday, September 12, 2013 11:49 PM
Subject: Re: [parallel_tests] Allow environment variables to be passed in to the command (#259)

What would some example environment things be that you want to set ?
—
Reply to this email directly or view it on GitHub.

westlakem · 2013-09-13T14:14:38Z

Mike, I for the life of me cannot figure out why $stdout.tty? is being set to true. I did not modify anything dealing with $stdout. I even changed the code to

if $stdout.tty?
if options[:env].nil?
options.merge!(:env => {"AUTOTEST" => "1"})
else
options[:env].merge!("AUTOTEST" => "1")
end
end

and the result was the same... It might be my environement, can you check it with the new if / else layout?

grosser · 2013-09-14T01:16:06Z

Setting java options via the environment seems to work just fine, not sure why you still need these changes:

_JAVA_OPTIONS='-Xms64m -Xmx2048m' ruby -e "system('ruby -e \"puts ENV[:_JAVA_OPTIONS.to_s]\"')"
Picked up _JAVA_OPTIONS: -Xms64m -Xmx2048m
Picked up _JAVA_OPTIONS: -Xms64m -Xmx2048m
-Xms64m -Xmx2048m

westlakem · 2013-09-14T01:24:50Z

I need these changes because I can't set the global environment variables.

The _JAVA_OPTIONS that I can set (in the call to jruby) don't get added to the new shells that are spawned

From: Michael Grosser notifications@github.com
To: grosser/parallel_tests parallel_tests@noreply.github.com
Cc: nightbeast westlake_m@Yahoo.com
Sent: Friday, September 13, 2013 9:16 PM
Subject: Re: [parallel_tests] Allow environment variables to be passed in to the command (#259)

Setting java options via the environment seems to work just fine, not sure why you still need these changes:
_JAVA_OPTIONS='-Xms64m -Xmx2048m' ruby -e "system('ruby -e "puts ENV[:_JAVA_OPTIONS.to_s]"')"
Picked up _JAVA_OPTIONS: -Xms64m -Xmx2048m
Picked up _JAVA_OPTIONS: -Xms64m -Xmx2048m
-Xms64m -Xmx2048m
—
Reply to this email directly or view it on GitHub.

grosser · 2013-09-14T02:23:23Z

But you can set local env variables as I did in the example I pasted, and
as you can see they got copied into the sub-shells. I don't understand why
this does not work for you ?

On Fri, Sep 13, 2013 at 6:24 PM, nightbeast notifications@github.comwrote:

I need these changes because I can't set the global environment variables.

The _JAVA_OPTIONS that I can set (in the call to jruby) don't get added to
the new shells that are spawned

From: Michael Grosser notifications@github.com
To: grosser/parallel_tests parallel_tests@noreply.github.com
Cc: nightbeast westlake_m@Yahoo.com
Sent: Friday, September 13, 2013 9:16 PM
Subject: Re: [parallel_tests] Allow environment variables to be passed in
to the command (#259)

Setting java options via the environment seems to work just fine, not sure
why you still need these changes:
_JAVA_OPTIONS='-Xms64m -Xmx2048m' ruby -e "system('ruby -e "puts
ENV[:_JAVA_OPTIONS.to_s]"')"
Picked up _JAVA_OPTIONS: -Xms64m -Xmx2048m
Picked up _JAVA_OPTIONS: -Xms64m -Xmx2048m
-Xms64m -Xmx2048m
—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/259#issuecomment-24434182
.

westlakem · 2013-09-23T21:24:30Z

So... (I think) it all came down to permissions. Once I was finally able to get my proper access into the machine, It worked just fine.

_JAVA_OPTIONS=-Xms128m
jruby -S parallel_cucumber features/ABC -n 3 -o 'BROWSER_TYPE=chrome'
Picked up _JAVA_OPTIONS: -Xms128m
3 processes for 333 features, ~ 111 features per process
Picked up _JAVA_OPTIONS: -Xms128m
Picked up _JAVA_OPTIONS: -Xms128m
Picked up _JAVA_OPTIONS: -Xms128m

Closing the issue

grosser · 2013-09-23T21:31:58Z

Yeah! Glad it worked out :D

On Mon, Sep 23, 2013 at 2:24 PM, nightbeast notifications@github.comwrote:

So... (I think) it all came down to permissions. Once I was finally able
to get my proper access into the machine, It worked just fine.

_JAVA_OPTIONS=-Xms128m
jruby -S parallel_cucumber features/ABC -n 3 -o 'BROWSER_TYPE=chrome'
Picked up _JAVA_OPTIONS: -Xms128m
3 processes for 333 features, ~ 111 features per process
Picked up _JAVA_OPTIONS: -Xms128m
Picked up _JAVA_OPTIONS: -Xms128m
Picked up _JAVA_OPTIONS: -Xms128m

Closing the issue

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/259#issuecomment-24955964
.

nightbeast added 7 commits September 12, 2013 15:05

Updated readme.md to include the environment variables option

b8a04ce

Added tip on how to use new feature

fd4c2a2

Updated readme.md to better clarify the tip

52c8f9a

Updated with new syntax

7739fed

updated with correct environment-variable parser

dcac871

Updated gherkin runner to accept the inputted environment variables f…

14dab8f

…rom the command line

westlakem mentioned this pull request Sep 12, 2013

Jruby JVM options #256

Closed

grosser reviewed Sep 13, 2013
View reviewed changes

westlakem closed this Sep 23, 2013

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow environment variables to be passed in to the command #259

Allow environment variables to be passed in to the command #259

westlakem commented Sep 12, 2013

grosser Sep 13, 2013

grosser commented Sep 13, 2013

grosser Sep 13, 2013

westlakem Sep 13, 2013

grosser Sep 13, 2013

westlakem Sep 13, 2013

grosser Sep 14, 2013

grosser commented Sep 13, 2013

grosser Sep 13, 2013

grosser commented Sep 13, 2013

westlakem commented Sep 13, 2013

westlakem commented Sep 13, 2013

westlakem commented Sep 13, 2013

grosser commented Sep 14, 2013

westlakem commented Sep 14, 2013

grosser commented Sep 14, 2013

westlakem commented Sep 23, 2013

grosser commented Sep 23, 2013

Allow environment variables to be passed in to the command #259

Allow environment variables to be passed in to the command #259

Conversation

westlakem commented Sep 12, 2013

grosser Sep 13, 2013

Choose a reason for hiding this comment

grosser commented Sep 13, 2013

grosser Sep 13, 2013

Choose a reason for hiding this comment

westlakem Sep 13, 2013

Choose a reason for hiding this comment

grosser Sep 13, 2013

Choose a reason for hiding this comment

westlakem Sep 13, 2013

Choose a reason for hiding this comment

grosser Sep 14, 2013

Choose a reason for hiding this comment

grosser commented Sep 13, 2013

grosser Sep 13, 2013

Choose a reason for hiding this comment

grosser commented Sep 13, 2013

westlakem commented Sep 13, 2013

westlakem commented Sep 13, 2013

westlakem commented Sep 13, 2013

grosser commented Sep 14, 2013

westlakem commented Sep 14, 2013

grosser commented Sep 14, 2013

westlakem commented Sep 23, 2013

grosser commented Sep 23, 2013