-
Notifications
You must be signed in to change notification settings - Fork 53
Refactor e2e tests to use assertion helper class #41
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.
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
Changes from 2 commits
df53a64
987bbc3
92a675a
8dc582b
8b9a842
7be0423
dfa4574
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,103 @@ | ||
# Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"). You may | ||
# not use this file except in compliance with the License. A copy of the | ||
# License is located at | ||
# | ||
# http://aws.amazon.com/apache2.0/ | ||
# | ||
# or in the "license" file accompanying this file. This file is distributed | ||
# on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either | ||
# express or implied. See the License for the specific language governing | ||
# permissions and limitations under the License. | ||
|
||
"""Helper functions for ec2 tests | ||
""" | ||
|
||
class Ec2Validator: | ||
def __init__(self, ec2_client): | ||
self.ec2_client = ec2_client | ||
|
||
def assert_dhcp_options(self, dhcp_options_id: str, exists=True): | ||
brycahta marked this conversation as resolved.
Show resolved
Hide resolved
|
||
res_found = False | ||
try: | ||
aws_res = self.ec2_client.describe_dhcp_options(DhcpOptionsIds=[dhcp_options_id]) | ||
res_found = len(aws_res["DhcpOptions"]) > 0 | ||
except self.ec2_client.exceptions.ClientError: | ||
pass | ||
assert res_found is exists | ||
|
||
def assert_internet_gateway(self, ig_id: str, exists=True): | ||
res_found = False | ||
try: | ||
aws_res = self.ec2_client.describe_internet_gateways(InternetGatewayIds=[ig_id]) | ||
res_found = len(aws_res["InternetGateways"]) > 0 | ||
except self.ec2_client.exceptions.ClientError: | ||
pass | ||
assert res_found is exists | ||
|
||
def assert_route(self, route_table_id: str, gateway_id: str, origin: str, exists=True): | ||
res_found = False | ||
try: | ||
aws_res = self.ec2_client.describe_route_tables(RouteTableIds=[route_table_id]) | ||
routes = aws_res["RouteTables"][0]["Routes"] | ||
for route in routes: | ||
if route["Origin"] == origin and route["GatewayId"] == gateway_id: | ||
res_found = True | ||
except self.ec2_client.exceptions.ClientError: | ||
pass | ||
assert res_found is exists | ||
|
||
def assert_route_table(self, route_table_id: str, exists=True): | ||
res_found = False | ||
try: | ||
aws_res = self.ec2_client.describe_route_tables(RouteTableIds=[route_table_id]) | ||
res_found = len(aws_res["RouteTables"]) > 0 | ||
except self.ec2_client.exceptions.ClientError: | ||
pass | ||
assert res_found is exists | ||
|
||
def assert_security_group(self, sg_id: str, exists=True): | ||
res_found = False | ||
try: | ||
aws_res = self.ec2_client.describe_security_groups(GroupIds=[sg_id]) | ||
res_found = len(aws_res["SecurityGroups"]) > 0 | ||
except self.ec2_client.exceptions.ClientError: | ||
pass | ||
assert res_found is exists | ||
|
||
def assert_subnet(self, subnet_id: str, exists=True): | ||
res_found = False | ||
try: | ||
aws_res = self.ec2_client.describe_subnets(SubnetIds=[subnet_id]) | ||
res_found = len(aws_res["Subnets"]) > 0 | ||
except self.ec2_client.exceptions.ClientError: | ||
pass | ||
assert res_found is exists | ||
|
||
def assert_transit_gateway(self, tgw_id: str, exists=True): | ||
res_found = False | ||
try: | ||
aws_res = self.ec2_client.describe_transit_gateways(TransitGatewayIds=[tgw_id]) | ||
res_found = len(aws_res["TransitGateways"]) > 0 | ||
except self.ec2_client.exceptions.ClientError: | ||
pass | ||
assert res_found is exists | ||
|
||
def assert_vpc(self, vpc_id: str, exists=True): | ||
res_found = False | ||
try: | ||
aws_res = self.ec2_client.describe_vpcs(VpcIds=[vpc_id]) | ||
res_found = len(aws_res["Vpcs"]) > 0 | ||
except self.ec2_client.exceptions.ClientError: | ||
pass | ||
assert res_found is exists | ||
|
||
def assert_vpc_endpoint(self, vpc_endpoint_id: str, exists=True): | ||
res_found = False | ||
try: | ||
aws_res = self.ec2_client.describe_vpc_endpoints(VpcEndpointIds=[vpc_endpoint_id]) | ||
res_found = len(aws_res["VpcEndpoints"]) > 0 | ||
except self.ec2_client.exceptions.ClientError: | ||
pass | ||
assert res_found is exists |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,31 +22,14 @@ | |
from acktest.k8s import resource as k8s | ||
from e2e import service_marker, CRD_GROUP, CRD_VERSION, load_ec2_resource | ||
from e2e.replacement_values import REPLACEMENT_VALUES | ||
from e2e.tests.helper import Ec2Validator | ||
|
||
RESOURCE_PLURAL = "dhcpoptions" | ||
|
||
DEFAULT_WAIT_AFTER_SECONDS = 5 | ||
CREATE_WAIT_AFTER_SECONDS = 10 | ||
DELETE_WAIT_AFTER_SECONDS = 10 | ||
|
||
|
||
def get_dhcp_options(ec2_client, dhcp_options_id: str) -> dict: | ||
try: | ||
resp = ec2_client.describe_dhcp_options( | ||
Filters=[{"Name": "dhcp-options-id", "Values": [dhcp_options_id]}] | ||
) | ||
except Exception as e: | ||
logging.debug(e) | ||
return None | ||
|
||
if len(resp["DhcpOptions"]) == 0: | ||
return None | ||
return resp["DhcpOptions"][0] | ||
|
||
|
||
def dhcp_options_exist(ec2_client, dhcp_options_id: str) -> bool: | ||
return get_dhcp_options(ec2_client, dhcp_options_id) is not None | ||
|
||
@service_marker | ||
@pytest.mark.canary | ||
class TestDhcpOptions: | ||
|
@@ -84,14 +67,15 @@ def test_create_delete(self, ec2_client): | |
|
||
time.sleep(CREATE_WAIT_AFTER_SECONDS) | ||
|
||
# Check DHCP Options exists | ||
assert dhcp_options_exist(ec2_client, resource_id) | ||
# Check DHCP Options exists in AWS | ||
ec2_validator = Ec2Validator(ec2_client) | ||
ec2_validator.assert_dhcp_options(resource_id) | ||
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. While I think checking for existence is useful, I believe we can harden our tests by checking their spec matches. Especially for code with custom hooks. Perhaps, we should have methods in the 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. I agree with raising the quality of these tests, but should be addressed in a separate PR/issue. Do you know of an issue (or parent issue) to improve e2e testing or should I open one for this controller? Once this is merged, my plan is to open a follow-up PR with a resource-reference e2e test modeled after ApiGatewayV2 (checks existence of referenced resources) so we can get some coverage on the feature. Then, I can work on improving e2e tests to check resources at a more granular level across the entire suite. Lmk if you have any other context (i.e. if some automated test-gen in the works, you prefer to finish vpc enhancements before e2e test improvements, etc.) or if you think it can be approached better. 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 is on a per-controller basis. So if there isn't one for EC2, you can create one. No automated test-gen in the works atm :( |
||
|
||
# Delete k8s resource | ||
_, deleted = k8s.delete_custom_resource(ref) | ||
assert deleted is True | ||
|
||
time.sleep(DELETE_WAIT_AFTER_SECONDS) | ||
|
||
# Check DHCP Options doesn't exist | ||
assert not dhcp_options_exist(ec2_client, resource_id) | ||
# Check DHCP Options no longer exists in AWS | ||
ec2_validator.assert_dhcp_options(resource_id, exists=False) |
Uh oh!
There was an error while loading. Please reload this page.