-
Notifications
You must be signed in to change notification settings - Fork 496
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
Set PARALLEL_TEST environment variable. #172
Conversation
Why not rely on TEST_ENV_NUMBER -> more generic -> if something needs multiple test envs it's something parallel -> use different command names |
I often set TEST_ENV_NUMBER to run different tests in different terminals manually. TEST_ENV_NUMBER is used to determine which test database to connect to. I personally can't rely on it to mean that the test is being run inside parallel_tests and I imagine I'm not the only one. |
You are the 1% :D
|
How do you runs specs? I'm getting a failure when I try. Am I not running them properly or is there a bug in HEAD?
|
try rake ;) On Wed, Jan 2, 2013 at 10:37 AM, Joseph Shraibman
|
Try to set PARALLEL_TESTS to number of processes. Add specs.
Updated. Test results: [jshraibman@Joseph-Shraibmans-MacBook-Pro feature/env_var ~/work/parallel_tests]$ time rake Pending: Finished in 68.45 seconds real 1m9.833s |
@@ -24,6 +24,13 @@ def call(*args) | |||
call(['xxx'],1,{}) | |||
end | |||
|
|||
# The PARALLEL_TESTS environment variable being set lets child processes know they are being run | |||
# inside parallel_tests. The actual value doesn't matter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say kill the comment and put it into the it
it "sets PARALLEL_TESTS to something so child processes know that parallel_tests is used"
…EL_TESTS can be set to it.
Updated. |
how about making it more meaningful, for example PARALLEL_TEST_GROUPS=4 ? On Wed, Jan 2, 2013 at 1:14 PM, Joseph Shraibman
|
I can do the change myself, just curious about your opinion / if you still need it |
Changing the name still fulfills the original purpose of having programs know they are being run inside parallel_tests. I would have made the change myself but I got caught up with other things. |
Set PARALLEL_TEST environment variable.
@@ -28,7 +28,7 @@ def self.run_tests_in_parallel(num_processes, options) | |||
report_number_of_tests runner, groups | |||
|
|||
test_results = Parallel.map(groups, :in_processes => groups.size) do |group| | |||
run_tests(runner, group, groups.index(group), options) | |||
run_tests(runner, group, groups.index(group), num_processes, options) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about groups.size instead of num_processes, since that's the real number of processes used (vs requested) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll just do it myself, also fits better with the env variable name :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realize there was a difference or I would have used groups.size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
99% the same, just in weird edge-cases...
On Thu, Jan 3, 2013 at 8:44 PM, Joseph Shraibman
notifications@git.luolix.topwrote:
In lib/parallel_tests/cli.rb:
@@ -28,7 +28,7 @@ def self.run_tests_in_parallel(num_processes, options)
report_number_of_tests runner, groupstest_results = Parallel.map(groups, :in_processes => groups.size) do |group|
run_tests(runner, group, groups.index(group), options)
run_tests(runner, group, groups.index(group), num_processes, options)
I didn't realize there was a difference or I would have used groups.size.
—
Reply to this email directly or view it on GitHubhttps://github.com//pull/172/files#r2547499.
-> 0.9.2, thanks for the pull :) |
This will allow me to fix SimpleCov issue 64
Setting this environment variable will let tools know they are being run inside parallel_tests and adjust accordingly.