Skip to content
Closed
Changes from all commits
Commits
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
69 changes: 60 additions & 9 deletions tests/foreman/cli/test_subnet.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,7 @@ def test_positive_set_parameter_option_presence(module_target_sat):
)


@pytest.mark.stubbed
def test_positive_create_with_parameter_and_multiple_values():
def test_positive_create_with_parameter_and_multiple_values(module_target_sat):
"""Subnet parameters can be created with multiple values
Comment on lines +264 to 265

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm trying to understand what the intent was for this test in #4941. The name is "create_with_parameter_…" which implies the parameter should be already set during creation, but that's not possible, you can only set parameters on existing subnets. In #21127 you already removed test_positive_create_with_parameter as a duplicate of test_positive_subnet_CRUD_parameters and the only difference here is that the value contains a comma, which doesn't sound too useful to me?

One could interpret "multiple values" as "the value is an array" (we have parameter_type, which is not covered in tests for subnets at all), but then "multiple names" in the test below still makes no real sense?


:id: 1e9ef184-bdf9-4eba-8055-f55b0dd9d6a0
Expand All @@ -278,10 +277,23 @@ def test_positive_create_with_parameter_and_multiple_values():

:BZ: 1426612

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this BZ is long closed and doesn't really relate to the things that we're testing here (it was just marked here as the test would fail before the BZ was fixed), so I think we can remove this marker from all tests.

Comment on lines 265 to 278

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please provide steps.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you think it makes sense to just close this PR and drop these tests? They seem to mostly duplicate existing coverage right?

"""
subnet = module_target_sat.cli_factory.make_subnet()
param_name = gen_string('alphanumeric', 10)
value_parts = [gen_string('alpha', 8) for _ in range(3)]
comma_value = ','.join(value_parts)
module_target_sat.cli.Subnet.set_parameter(
{'subnet-id': subnet['id'], 'name': param_name, 'value': comma_value}
)
subnet_info = module_target_sat.cli.Subnet.info({'id': subnet['id']}, output_format='json')
params = subnet_info['parameters']
param = next(parameter for parameter in params if param_name == parameter['name'])
stored = param['value']
stored_str = str(stored).strip()
assert stored_str == comma_value
module_target_sat.cli.Subnet.delete({'id': subnet['id']})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if the assert fails, this won't be executed, leaving the subnet behind and possibly interfering with other tests.

you can add a finalizer instead (will need to add the request fixture too):

Suggested change
module_target_sat.cli.Subnet.delete({'id': subnet['id']})
request.addfinalizer(lambda: module_target_sat.cli.Subnet.delete({'id': subnet['id']}))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, sure !!



@pytest.mark.stubbed
def test_positive_create_with_parameter_and_multiple_names():
def test_positive_create_with_parameter_and_multiple_names(module_target_sat):
"""Subnet parameters can be created with multiple names with valid
separators

Expand All @@ -298,10 +310,23 @@ def test_positive_create_with_parameter_and_multiple_names():

:BZ: 1426612
"""
subnet = module_target_sat.cli_factory.make_subnet()
name_parts = [gen_string('alphanumeric', 6) for _ in range(3)]
param_name = '/'.join(name_parts)
param_value = gen_string('alphanumeric', 10)
module_target_sat.cli.Subnet.set_parameter(
{'subnet-id': subnet['id'], 'name': param_name, 'value': param_value}
)
subnet_info = module_target_sat.cli.Subnet.info({'id': subnet['id']}, output_format='json')
params = subnet_info['parameters']
param = next(parameter for parameter in params if param_name == parameter['name'])
stored = param['value']
stored_str = str(stored).strip()
assert stored_str == param_value
module_target_sat.cli.Subnet.delete({'id': subnet['id']})


@pytest.mark.stubbed
def test_negative_create_with_parameter_and_invalid_separator():
def test_negative_create_with_parameter_and_invalid_separator(module_target_sat):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

def test_negative_create_with_parameter_and_invalid_separators(module_target_sat):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of 1 separator can we test bunch of seperators?

"""Subnet parameters can not be created with multiple names with
invalid separators

Expand All @@ -311,18 +336,29 @@ def test_negative_create_with_parameter_and_invalid_separator():

1. Attempt to 'Create Subnet' with all the details
2. Also with parameter having key with multiple names separated by
invalid separators(e.g comma) and value
invalid separators(e.g spaces) and value

:expectedresults: The parameter with multiple names separated by
invalid separators should not be saved in subnet

:BZ: 1426612
"""
subnet = module_target_sat.cli_factory.make_subnet()
name_parts = [gen_string('alphanumeric', 6) for _ in range(3)]
param_name = ' '.join(name_parts)
param_value = gen_string('alphanumeric', 10)
with pytest.raises(CLIReturnCodeError):
module_target_sat.cli.Subnet.set_parameter(
{'subnet-id': subnet['id'], 'name': param_name, 'value': param_value}
)
subnet_info = module_target_sat.cli.Subnet.info({'id': subnet['id']}, output_format='json')
params = subnet_info['parameters']
assert len(params) == 0
module_target_sat.cli.Subnet.delete({'id': subnet['id']})


@pytest.mark.stubbed
@pytest.mark.upgrade
def test_positive_create_with_multiple_parameters():
def test_positive_create_with_multiple_parameters(module_target_sat):
"""Subnet with more than one parameters

:id: 2a9b3043-1add-43d2-af2f-ff39304eb698
Expand All @@ -337,6 +373,21 @@ def test_positive_create_with_multiple_parameters():

:BZ: 1426612
"""
subnet = module_target_sat.cli_factory.make_subnet()
param_specs = [
(gen_string('alphanumeric', 10), gen_string('alphanumeric', 10)) for _ in range(3)
]
for name, value in param_specs:
module_target_sat.cli.Subnet.set_parameter(
{'subnet-id': subnet['id'], 'name': name, 'value': value}
)
subnet_info = module_target_sat.cli.Subnet.info({'id': subnet['id']}, output_format='json')
params = subnet_info['parameters']
assert len(params) == len(param_specs)
for name, value in param_specs:
param = next(parameter for parameter in params if name == parameter['name'])
assert str(param['value']).strip() == value
module_target_sat.cli.Subnet.delete({'id': subnet['id']})


@pytest.mark.stubbed
Expand Down
Loading