-
Notifications
You must be signed in to change notification settings - Fork 165
[1LP][WIPTEST]Added invalid cidr/ip tests for floating_ip_add and subnet_add #10033
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import pytest | ||
|
||
from cfme import test_requirements | ||
from cfme.cloud.provider.openstack import OpenStackProvider | ||
from cfme.networks.views import FloatingIpView | ||
|
||
pytestmark = [ | ||
test_requirements.sdn, | ||
pytest.mark.usefixtures('setup_provider'), | ||
pytest.mark.provider([OpenStackProvider], scope="module") | ||
] | ||
|
||
|
||
@pytest.fixture() | ||
def network_manager(appliance, provider): | ||
network_manager, = appliance.collections.network_providers.filter({"provider": provider}).all() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you wrap this potential ValueError in a pytest.skip call, so it doesn't start throwing errors when there's more/less than 1 element in the list. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mshriver Is this something like you wanted? https://github.com/ManageIQ/integration_tests/pull/10033/files#diff-d0ee99cb7213e3f233490166c76aab34R15 |
||
yield network_manager | ||
|
||
|
||
@pytest.mark.meta(automates=[1652501]) | ||
def test_network_ip_address_invalid_address(appliance, provider, network_manager, setup_provider): | ||
""" | ||
Bugzilla: 1652501 | ||
Polarion: | ||
assignee: mmojzis | ||
casecomponent: Cloud | ||
caseimportance: medium | ||
initialEstimate: 1/10h | ||
""" | ||
subnets_collection = appliance.collections.network_floating_ips | ||
invalid_address = 'test' | ||
|
||
with pytest.raises(AssertionError): | ||
subnets_collection.create(network_manager=network_manager.name, | ||
network_name=provider.data['public_network'], | ||
tenant=provider.data['tenant'], provider=provider, | ||
floating_ip_address=invalid_address) | ||
|
||
view = subnets_collection.create_view(FloatingIpView) | ||
view.flash.assert_message( | ||
f"Unable to create Floating IP \"{invalid_address}\": Invalid input for floating_ip_address" | ||
f". Reason: \'{invalid_address}\' is not a valid IP address.") |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import pytest | ||
|
||
from cfme import test_requirements | ||
from cfme.cloud.provider.openstack import OpenStackProvider | ||
from cfme.networks.views import SubnetView | ||
|
||
pytestmark = [ | ||
test_requirements.sdn, | ||
pytest.mark.usefixtures('setup_provider'), | ||
pytest.mark.provider([OpenStackProvider], scope="module") | ||
] | ||
|
||
|
||
@pytest.fixture() | ||
def network_manager(appliance, provider): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method does the same as the one in the other file, so I am frowning on picking the first network_manager here as well. But I think also having this a fixture doesn't provide enough (any) benefits yet it just creates some potential problems in future because it is a code dup. It is fixture for effectively a one-liner. I am sure one-liners dups are not considered a problem most of the times. I will ask other reviewers what they think about fixtures like this one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, in the future when they add another providers with network managers the fixture makes sense. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @JaryN valid concern for sure. I wouldn't consider this a blocker to merging this PR, but warrants some future enhancement. This could be a globally available fixture (cfme/fixtures/), or even a subcollection of the parent cloud provider. |
||
network_manager, = appliance.collections.network_providers.filter({"provider": provider}).all() | ||
yield network_manager | ||
|
||
|
||
@pytest.mark.meta(automates=[1652515]) | ||
def test_network_subnet_invalid_cidr(appliance, provider, network_manager, setup_provider): | ||
""" | ||
Bugzilla: 1652515 | ||
Polarion: | ||
assignee: mmojzis | ||
casecomponent: Cloud | ||
caseimportance: medium | ||
initialEstimate: 1/10h | ||
""" | ||
subnets_collection = appliance.collections.network_subnets | ||
invalid_cidr = 'test' | ||
|
||
with pytest.raises(AssertionError): | ||
subnets_collection.create(network_manager=network_manager.name, | ||
network_name=provider.data['public_network'], | ||
tenant=provider.data['tenant'], cidr=invalid_cidr, | ||
name='test_subnet', provider=provider) | ||
|
||
view = subnets_collection.create_view(SubnetView) | ||
view.flash.assert_message( | ||
f"Unable to create Cloud Subnet: Invalid input for cidr. Reason: '{invalid_cidr}' is not a" | ||
" valid IP subnet.") |
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.
We want to
assert_no error()
here instead and have the actual flash message asserted in a test.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.
Agreed @jawatts, @mmojzis can you cover this flash assertion with a positive CRUD test case? Looks like only negative test cases are included in this PR.