Fixed a name clash in the 'from_environment_diff' function (#12116)

* Added a unit test reproducing the failure in 12085

* Fixed name clash in the 'from_environment_diff' function

The bug reported in #12085 stemmed from a name clash among variables,
introduced during the refactor in #10753 and not caught by unit tests
and reviews.
This commit is contained in:
Massimiliano Culpo 2019-07-24 17:25:24 +02:00 committed by Greg Becker
parent a8b12f0cb6
commit 6fac0ae687
2 changed files with 18 additions and 12 deletions

View file

@ -399,6 +399,7 @@ def test_sanitize_regex(env, blacklist, whitelist, expected, deleted):
assert all(x not in after for x in deleted) assert all(x not in after for x in deleted)
@pytest.mark.regression('12085')
@pytest.mark.parametrize('before,after,search_list', [ @pytest.mark.parametrize('before,after,search_list', [
# Set environment variables # Set environment variables
({}, {'FOO': 'foo'}, [environment.SetEnv('FOO', 'foo')]), ({}, {'FOO': 'foo'}, [environment.SetEnv('FOO', 'foo')]),
@ -420,7 +421,12 @@ def test_sanitize_regex(env, blacklist, whitelist, expected, deleted):
({'FOO_PATH': '/a/path:/b/path'}, {'FOO_PATH': '/c/path:/a/path'}, [ ({'FOO_PATH': '/a/path:/b/path'}, {'FOO_PATH': '/c/path:/a/path'}, [
environment.RemovePath('FOO_PATH', '/b/path'), environment.RemovePath('FOO_PATH', '/b/path'),
environment.PrependPath('FOO_PATH', '/c/path') environment.PrependPath('FOO_PATH', '/c/path')
]) ]),
# Modify two variables in the same environment
({'FOO': 'foo', 'BAR': 'bar'}, {'FOO': 'baz', 'BAR': 'baz'}, [
environment.SetEnv('FOO', 'baz'),
environment.SetEnv('BAR', 'baz'),
]),
]) ])
def test_from_environment_diff(before, after, search_list): def test_from_environment_diff(before, after, search_list):

View file

@ -607,12 +607,12 @@ def return_separator_if_any(*args):
env.unset(x) env.unset(x)
for x in modified_variables: for x in modified_variables:
before = before[x] value_before = before[x]
after = after[x] value_after = after[x]
sep = return_separator_if_any(before, after) sep = return_separator_if_any(value_before, value_after)
if sep: if sep:
before_list = before.split(sep) before_list = value_before.split(sep)
after_list = after.split(sep) after_list = value_after.split(sep)
# Filter out empty strings # Filter out empty strings
before_list = list(filter(None, before_list)) before_list = list(filter(None, before_list))
@ -623,8 +623,8 @@ def return_separator_if_any(*args):
before_list = list(dedupe(before_list)) before_list = list(dedupe(before_list))
after_list = list(dedupe(after_list)) after_list = list(dedupe(after_list))
# The reassembled cleaned entries # The reassembled cleaned entries
before = sep.join(before_list) value_before = sep.join(before_list)
after = sep.join(after_list) value_after = sep.join(after_list)
# Paths that have been removed # Paths that have been removed
remove_list = [ remove_list = [
@ -638,12 +638,12 @@ def return_separator_if_any(*args):
end = after_list.index(remaining_list[-1]) end = after_list.index(remaining_list[-1])
search = sep.join(after_list[start:end + 1]) search = sep.join(after_list[start:end + 1])
except IndexError: except IndexError:
env.prepend_path(x, after) env.prepend_path(x, value_after)
continue continue
if search not in before: if search not in value_before:
# We just need to set the variable to the new value # We just need to set the variable to the new value
env.prepend_path(x, after) env.prepend_path(x, value_after)
else: else:
try: try:
prepend_list = after_list[:start] prepend_list = after_list[:start]
@ -663,7 +663,7 @@ def return_separator_if_any(*args):
env.prepend_path(x, item) env.prepend_path(x, item)
else: else:
# We just need to set the variable to the new value # We just need to set the variable to the new value
env.set(x, after) env.set(x, value_after)
return env return env