Skip to content

Commit

Permalink
bugy#224 fixed newline trimming in parameters from script
Browse files Browse the repository at this point in the history
  • Loading branch information
bugy committed Aug 18, 2019
1 parent ed2009f commit 35c66c1
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 9 deletions.
14 changes: 10 additions & 4 deletions src/model/parameter_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,12 @@ def value_to_str(self, value):

return str(value)

def value_to_repr(self, value):
if self.secure:
return SECURE_MASK

return repr(value)

def get_secured_value(self, value):
if (not self.secure) or (value is None) or self.no_value:
return value
Expand Down Expand Up @@ -250,7 +256,7 @@ def validate_value(self, value, *, ignore_required=False):
return 'is not specified'
return None

value_string = self.value_to_str(value)
value_string = self.value_to_repr(value)

if self.no_value:
if value not in ['true', True, 'false', False]:
Expand Down Expand Up @@ -297,17 +303,17 @@ def validate_value(self, value, *, ignore_required=False):
if (self.type == 'list') or (self._is_plain_server_file()):
if value not in allowed_values:
return 'has value ' + value_string \
+ ', but should be in [' + ','.join(allowed_values) + ']'
+ ', but should be in ' + repr(allowed_values)
return None

if self.type == PARAM_TYPE_MULTISELECT:
if not isinstance(value, list):
return 'should be a list, but was: ' + value_string + '(' + str(type(value)) + ')'
for value_element in value:
if value_element not in allowed_values:
element_str = self.value_to_str(value_element)
element_str = self.value_to_repr(value_element)
return 'has value ' + element_str \
+ ', but should be in [' + ','.join(allowed_values) + ']'
+ ', but should be in ' + repr(allowed_values)
return None

if self._is_recursive_server_file():
Expand Down
15 changes: 15 additions & 0 deletions src/tests/parameter_config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,15 @@ def test_values_from_script(self):
'values': {'script': 'echo "123\n" "456"'}})
self.assertEqual(['123', ' 456'], parameter_model.values)

def test_values_from_script_win_newline(self):
test_utils.set_win()

parameter_model = _create_parameter_model({
'name': 'def_param',
'type': 'list',
'values': {'script': 'echo "123\r\n" "456"'}})
self.assertEqual(['123', ' 456'], parameter_model.values)

def test_allowed_values_for_non_list(self):
parameter_model = _create_parameter_model({
'name': 'def_param',
Expand Down Expand Up @@ -517,6 +526,12 @@ def test_list_with_script_when_matches(self):
error = parameter.validate_value('123')
self.assertIsNone(error)

def test_list_with_script_when_matches_and_win_newline(self):
parameter = create_parameter_model('param', type='list', values_script="echo '123\r\n' 'abc'")

error = parameter.validate_value('123')
self.assertIsNone(error)

def test_list_with_dependency_when_matches(self):
parameters = []
values = ObservableDict()
Expand Down
8 changes: 3 additions & 5 deletions src/utils/process_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,10 @@ def invoke(command, work_dir='.', *, environment_variables=None):
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
cwd=work_dir,
env=env)
env=env,
universal_newlines=True)

(output_bytes, error_bytes) = p.communicate()

output = output_bytes.decode("utf-8")
error = error_bytes.decode("utf-8")
(output, error) = p.communicate()

result_code = p.returncode
if result_code != 0:
Expand Down

0 comments on commit 35c66c1

Please sign in to comment.