Skip to content

Do not allow changes to ENV to leak from test to test #403

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 1 commit into from
Feb 17, 2019
Merged

Do not allow changes to ENV to leak from test to test #403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
Do not allow changes to ENV to leak from test to test 
Signed-off-by: James Couball <jcouball@yahoo.com>
  • Loading branch information
@jcouball
jcouball committed Jan 30, 2019
commit ae1be05dde0d3c00a809018efd79500fc208e038
16 changes: 15 additions & 1 deletion tests/test_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,5 +79,19 @@ def append_file(name, contents)
f.puts contents
end
end


# Runs a block inside an environment with customized ENV variables.
# It restores the ENV after execution.
#
# @param [Proc] block block to be executed within the customized environment
#
def with_custom_env_variables(&block)
saved_env = {}
begin
Git::Lib::ENV_VARIABLE_NAMES.each { |k| saved_env[k] = ENV[k] }
return block.call
ensure
Git::Lib::ENV_VARIABLE_NAMES.each { |k| ENV[k] = saved_env[k] }
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

(This reminds me about my dislike for test-unit; with rspec, this could easily be done using an around {} block instead.)

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer rspec for the structure it makes possible. I suspect that others don't care for rspec because that structure comes with additional complexity and a steeper learning curve.

That said, I respect any project that has tests no matter what framework!

Copy link
Contributor

Choose a reason for hiding this comment

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

Comparing rspec to other test frameworks in general, like testng in Java, I think the fact that you can actually "group" and structure your tests with rspec is a big boon. It's one of the things I'm missing from no longer using it actually.

If you feel like rewriting the tests to use rspec, I clearly wouldn't object, but this is definitely not an obligation in any way. 😃

end
36 changes: 19 additions & 17 deletions tests/units/test_config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,27 +29,29 @@ def test_set_config
end

def test_env_config
assert_equal(Git::Base.config.git_ssh, nil)

ENV['GIT_SSH'] = '/env/git/ssh'
with_custom_env_variables do
begin
assert_equal(Git::Base.config.git_ssh, nil)
ENV['GIT_SSH'] = '/env/git/ssh'

assert_equal(Git::Base.config.git_ssh, '/env/git/ssh')
assert_equal(Git::Base.config.git_ssh, '/env/git/ssh')

Git.configure do |config|
config.binary_path = '/usr/bin/git'
config.git_ssh = '/path/to/ssh/script'
end
assert_equal(Git::Base.config.git_ssh, '/path/to/ssh/script')
Git.configure do |config|
config.binary_path = '/usr/bin/git'
config.git_ssh = '/path/to/ssh/script'
end

assert_equal(Git::Base.config.git_ssh, '/path/to/ssh/script')

@git.log
ensure
ENV['GIT_SSH'] = nil
@git.log
ensure
ENV['GIT_SSH'] = nil

Git.configure do |config|
config.binary_path = nil
config.git_ssh = nil
Git.configure do |config|
config.binary_path = nil
config.git_ssh = nil
end
end
end
end

end
58 changes: 31 additions & 27 deletions tests/units/test_lib.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,40 +75,44 @@ def test_log_commits
a = @lib.full_log_commits :count => 20
assert_equal(20, a.size)
end

def test_environment_reset
ENV['GIT_DIR'] = '/my/git/dir'
ENV['GIT_WORK_TREE'] = '/my/work/tree'
ENV['GIT_INDEX_FILE'] = 'my_index'
with_custom_env_variables do
ENV['GIT_DIR'] = '/my/git/dir'
ENV['GIT_WORK_TREE'] = '/my/work/tree'
ENV['GIT_INDEX_FILE'] = 'my_index'

@lib.log_commits :count => 10
@lib.log_commits :count => 10

assert_equal(ENV['GIT_DIR'], '/my/git/dir')
assert_equal(ENV['GIT_WORK_TREE'], '/my/work/tree')
assert_equal(ENV['GIT_INDEX_FILE'],'my_index')
assert_equal(ENV['GIT_DIR'], '/my/git/dir')
assert_equal(ENV['GIT_WORK_TREE'], '/my/work/tree')
assert_equal(ENV['GIT_INDEX_FILE'],'my_index')
end
end

def test_git_ssh_from_environment_is_passed_to_binary
ENV['GIT_SSH'] = 'my/git-ssh-wrapper'

Dir.mktmpdir do |dir|
output_path = File.join(dir, 'git_ssh_value')
binary_path = File.join(dir, 'git')
Git::Base.config.binary_path = binary_path
File.open(binary_path, 'w') { |f|
f << "echo $GIT_SSH > #{output_path}"
}
FileUtils.chmod(0700, binary_path)
@lib.checkout('something')
assert_equal("my/git-ssh-wrapper\n", File.read(output_path))
end
ensure
Git.configure do |config|
config.binary_path = nil
config.git_ssh = nil
end
with_custom_env_variables do
begin
ENV['GIT_SSH'] = 'my/git-ssh-wrapper'

ENV['GIT_SSH'] = nil
Dir.mktmpdir do |dir|
output_path = File.join(dir, 'git_ssh_value')
binary_path = File.join(dir, 'git')
Git::Base.config.binary_path = binary_path
File.open(binary_path, 'w') { |f|
f << "echo $GIT_SSH > #{output_path}"
}
FileUtils.chmod(0700, binary_path)
@lib.checkout('something')
assert_equal("my/git-ssh-wrapper\n", File.read(output_path))
end
ensure
Git.configure do |config|
config.binary_path = nil
config.git_ssh = nil
end
end
end
end

def test_revparse
Expand Down
0 tests/units/test_thread_safty.rb → tests/units/test_thread_safety.rb
View file
Open in desktop
File renamed without changes.