From 0572c0d6c9340b44583a6bc84e20c880441fee9e Mon Sep 17 00:00:00 2001 From: Elijah DeLee Date: Fri, 13 Oct 2017 14:50:50 -0400 Subject: [PATCH] Refactor based on code review Update names of functions and remove unneeded code. --- camayoc/api.py | 19 +++--- camayoc/qcs_models.py | 4 +- camayoc/tests/qcs/conftest.py | 4 +- camayoc/tests/qcs/test_credentials.py | 85 ++++++++++++--------------- camayoc/tests/qcs/test_profiles.py | 22 +++---- 5 files changed, 60 insertions(+), 74 deletions(-) diff --git a/camayoc/api.py b/camayoc/api.py index ea78de1d..eea93070 100644 --- a/camayoc/api.py +++ b/camayoc/api.py @@ -20,12 +20,12 @@ ) -def echo_handler(server_config, response): # pylint:disable=unused-argument +def echo_handler(response): """Immediately return ``response``.""" return response -def code_handler(server_config, response): # pylint:disable=unused-argument +def code_handler(response): """Check the response status code, and return the response. :raises: ``requests.exceptions.HTTPError`` if the response status code is @@ -35,7 +35,7 @@ def code_handler(server_config, response): # pylint:disable=unused-argument return response -def json_handler(server_config, response): +def json_handler(response): """Like ``code_handler``, but also return a JSON-decoded response body. Do what :func:`camayoc.api.code_handler` does. In addition, decode the @@ -91,13 +91,13 @@ def __init__(self, response_handler=None, url=None): """ self._cfg = config.get_config() qcs_settings = self._cfg.get('qcs') - self.url = self.url = urljoin(url, QCS_API_VERSION) if url else None + self.url = urljoin(url, QCS_API_VERSION) if url else None if qcs_settings and not url: if not qcs_settings.get('hostname'): raise exceptions.QCSBaseUrlNotFound( - '\n\'qcs\' section specified in camayoc config file, but' - ' no \'hostname\' key found.' + "\n'qcs' section specified in camayoc config file, but" + "no 'hostname' key found." ) self.url = urljoin(qcs_settings.get('hostname'), QCS_API_VERSION) @@ -154,10 +154,7 @@ def request(self, method, url, **kwargs): # # request(method, url, **kwargs) # - return self.response_handler( - self._cfg, - requests.request(method, url, **kwargs), - ) + return self.response_handler(requests.request(method, url, **kwargs)) class QCSClient(Client): @@ -181,7 +178,7 @@ def __init__(self, *args, **kwargs): """ self.credential_path = QCS_CREDENTIALS_PATH self.profile_path = QCS_PROFILES_PATH - super(QCSClient, self).__init__(*args, **kwargs) + super().__init__(*args, **kwargs) def create_credential(self, payload): """Send POST to QCS to create new host credential.""" diff --git a/camayoc/qcs_models.py b/camayoc/qcs_models.py index e5534e00..3b337afa 100644 --- a/camayoc/qcs_models.py +++ b/camayoc/qcs_models.py @@ -69,7 +69,7 @@ def __init__( >>> create_response = client.create_credential(host.payload()) >>> host._id = create_response.json()['id'] """ - super(HostCredential, self).__init__(name=name, _id=_id) + super().__init__(name=name, _id=_id) self.username = str(uuid.uuid4()) if not username else username self.password = password self.ssh_keyfile = ssh_keyfile @@ -122,7 +122,7 @@ def __init__( credential_ids=['host._id']) >>> client.create_net_prof(netprof.payload()) """ - super(NetworkProfile, self).__init__(name=name, _id=_id) + super().__init__(name=name, _id=_id) self.hosts = hosts self.ssh_port = 22 if not name else name self.credentials = credential_ids diff --git a/camayoc/tests/qcs/conftest.py b/camayoc/tests/qcs/conftest.py index 791dc16b..0e1c76cf 100644 --- a/camayoc/tests/qcs/conftest.py +++ b/camayoc/tests/qcs/conftest.py @@ -5,7 +5,7 @@ @pytest.fixture -def cleanup_credentials(): +def credentials_to_cleanup(): """Fixture that cleans up any created host credentials.""" credential_ids = [] @@ -17,7 +17,7 @@ def cleanup_credentials(): @pytest.fixture -def cleanup_profiles(): +def profiles_to_cleanup(): """Fixture that cleans up any created network profiles.""" profile_ids = [] diff --git a/camayoc/tests/qcs/test_credentials.py b/camayoc/tests/qcs/test_credentials.py index 6ffa9c20..dff325ac 100644 --- a/camayoc/tests/qcs/test_credentials.py +++ b/camayoc/tests/qcs/test_credentials.py @@ -29,7 +29,7 @@ def confirm_credential(cred): assert cred == artifact -def test_create_with_password(isolated_filesystem, cleanup_credentials): +def test_create_with_password(credentials_to_cleanup): """Create a host credential with username and password. :id: d04e3e1b-c7f1-4cc2-a4a4-a3d3317f95ce @@ -37,40 +37,32 @@ def test_create_with_password(isolated_filesystem, cleanup_credentials): :steps: Send POST with necessary data to documented api endpoint. :expectedresults: A new host credential entry is created with the data. """ - # obtain list of credentials to destroy after test is over - # from the cleanup_credentials fixture - ids_to_cleanup = cleanup_credentials - client = api.QCSClient() cred = HostCredential(password=str(uuid4())) cred._id = client.create_credential(cred.payload()).json()['id'] # add the id to the list to destroy after the test is done - ids_to_cleanup.append(cred._id) + credentials_to_cleanup.append(cred._id) confirm_credential(cred) -def test_update_username(isolated_filesystem, cleanup_credentials): +def test_update_username(credentials_to_cleanup): """Create a host credential and then update its username. :id: 73ed2ed5-e623-48ec-9ea6-153017464d9c :description: Create a host credential with password, then update its username. - :steps: 1) Create a host credential with a username and password. - 2) Update the host credential with a new username. - 3) Confirm host credential has been updated. + :steps: + 1) Create a host credential with a username and password. + 2) Update the host credential with a new username. + 3) Confirm host credential has been updated. :expectedresults: The host credential is updated. """ - # obtain list of credentials to destroy after test is over - # from the cleanup_credentials fixture - ids_to_cleanup = cleanup_credentials - client = api.QCSClient() cred = HostCredential(password=str(uuid4())) cred._id = client.create_credential(cred.payload()).json()['id'] - # add the id to the list to destroy after the test is done - ids_to_cleanup.append(cred._id) + credentials_to_cleanup.append(cred._id) confirm_credential(cred) @@ -80,44 +72,45 @@ def test_update_username(isolated_filesystem, cleanup_credentials): confirm_credential(cred) -def test_update_password_to_sshkeyfile( - isolated_filesystem, - cleanup_credentials): +def test_update_password_to_sshkeyfile(credentials_to_cleanup): """Create a host credential using password and switch it to use sshkey. :id: 6e557092-192b-4f75-babc-abc5774fe965 :description: Create a host credential with password, then update it to use a sshkey. - :steps: 1) Create a host credential with a username and password. - 2) Update the host credential deleting password and adding sshkey. - 3) Confirm host credential has been updated. + :steps: + 1) Create a host credential with a username and password. + 2) Update the host credential deleting password and adding sshkey. + 3) Confirm host credential has been updated. :expectedresults: The host credential is updated. """ pass -def test_update_sshkey_to_password(isolated_filesystem, cleanup_credentials): +def test_update_sshkey_to_password(credentials_to_cleanup): """Create a host credential using password and switch it to use sshkey. :id: d24a54b5-3d8c-44e4-a0ae-61584a15b127 :description: Create a host credential with a sshkey, then update it to use a password. - :steps: 1) Create a host credential with a username and sshkey. - 2) Update the host credential deleting sshkey and updating - password. - 3) Confirm host credential has been updated. + :steps: + 1) Create a host credential with a username and sshkey. + 2) Update the host credential deleting sshkey and updating + password. + 3) Confirm host credential has been updated. :expectedresults: The host credential is updated. """ pass -def test_negative_update_to_invalid(isolated_filesystem, cleanup_credentials): +def test_negative_update_to_invalid(credentials_to_cleanup): """Attempt to update valid credential with invalid data. :id: c34ea917-ee36-4b93-8907-24a5f87bbed3 :description: Create valid host credentials, then attempt to update to be - invalid. - :steps: 1) Create valid credentials with passwords or sshkey. + invalid. + :steps: + 1) Create valid credentials with passwords or sshkey. 2) Update the host credentials: a) missing usernames b) using both password and sshkey @@ -128,7 +121,7 @@ def test_negative_update_to_invalid(isolated_filesystem, cleanup_credentials): pass -def test_create_with_sshkey(isolated_filesystem, cleanup_credentials): +def test_create_with_sshkey(credentials_to_cleanup): """Create a host credential with username and sshkey. :id: ab6fd574-2e9f-46b8-847d-17b23c19fdd2 @@ -139,9 +132,7 @@ def test_create_with_sshkey(isolated_filesystem, cleanup_credentials): pass -def test_negative_create_key_and_pass( - isolated_filesystem, - cleanup_credentials): +def test_negative_create_key_and_pass(credentials_to_cleanup): """Attempt to create a host credential with sshkey and password. The request should be met with a 4XX response. @@ -154,7 +145,7 @@ def test_negative_create_key_and_pass( pass -def test_negative_create_no_name(isolated_filesystem, cleanup_credentials): +def test_negative_create_no_name(credentials_to_cleanup): """Attempt to create a host credential missing a name. The request should be met with a 4XX response. @@ -167,8 +158,7 @@ def test_negative_create_no_name(isolated_filesystem, cleanup_credentials): pass -def test_negative_create_no_key_or_pass( - isolated_filesystem, cleanup_credentials): +def test_negative_create_no_key_or_pass(credentials_to_cleanup): """Attempt to create a host credential missing both password and sshkey. The request should be met with a 4XX response. @@ -181,45 +171,48 @@ def test_negative_create_no_key_or_pass( pass -def test_read_all(isolated_filesystem, cleanup_credentials): +def test_read_all(credentials_to_cleanup): """After created, retrieve all host credentials with GET to api. :id: fa05b857-5b01-4388-9226-8dfb5639c815 :description: The API should return list with all host credentials created when a GET request is sent to the host credential endpoint. - :steps: 1) Create collection of host credentials, saving the information. + :steps: + 1) Create collection of host credentials, saving the information. 2) Send GET request to host credential endpoint to get list of - created host credentials. + created host credentials. 3) Confirm that all hosts are in the list. :expectedresults: All hosts are present in data returned by API. """ pass -def test_read_indv(isolated_filesystem, cleanup_credentials): +def test_read_indv(credentials_to_cleanup): """After created, retrieve each host credential with GET to api. :id: 4d381119-2dc3-42b6-9b41-e27307d61fcc :description: The API should a single host credential when a GET is made to the host credentials path and a host id is specified. - :steps: 1) Create collection of host credentials, saving the information. + :steps: + 1) Create collection of host credentials, saving the information. 2) Send GET request to host credential endpoint with the host id - specified. + specified. 3) Confirm that each host is retrieved :expectedresults: All hosts are present in data returned by API. """ pass -def test_delete(isolated_filesystem, cleanup_credentials): +def test_delete(credentials_to_cleanup): """After creating several host credentials, delete one. :id: e71b521c-59f9-483a-9063-1fbd5087c667 :description: Test that we can delete an individual host credential by id - :steps: 1) Create collection of host credentials, saving the information. + :steps: + 1) Create collection of host credentials, saving the information. 2) Send a DELETE request to destroy individual host credential 3) Send GET request to host credential endpoint to get list of - created host credentials. + created host credentials. 4) Confirm that all hosts are in the list except the deleted one. 5) Repeat until all hosts are deleted. :expectedresults: All hosts are present in data returned by API except diff --git a/camayoc/tests/qcs/test_profiles.py b/camayoc/tests/qcs/test_profiles.py index f451cb40..8a2a5191 100644 --- a/camayoc/tests/qcs/test_profiles.py +++ b/camayoc/tests/qcs/test_profiles.py @@ -22,24 +22,20 @@ @pytest.mark.parametrize('scan_host', CREATE_DATA) -def test_create(isolated_filesystem, - cleanup_credentials, - cleanup_profiles, - scan_host): +def test_create( + credentials_to_cleanup, + profiles_to_cleanup, + scan_host): """Create a Network Profile using a single credential. :id: db459fc2-d34c-45cf-915a-1535406a9320 :description: Create network profile of single host and credential - :steps: 1) Create host credential + :steps: + 1) Create host credential 2) Send POST with data to create network profile using the host - credential to the profile endpoint. + credential to the profile endpoint. :expectedresults: A new network profile entry is created with the data. """ - # obtain list of credentials to destroy after test is over - # from the cleanup_credentials fixture - cred_ids_to_cleanup = cleanup_credentials - profile_ids_to_cleanup = cleanup_profiles - cred = HostCredential(password=str(uuid4())) client = api.QCSClient() cred._id = client.create_credential(cred.payload()).json()['id'] @@ -50,8 +46,8 @@ def test_create(isolated_filesystem, profile._id = client.create_profile(profile.payload()).json()['id'] # add the ids to the lists to destroy after the test is done - cred_ids_to_cleanup.append(cred._id) - profile_ids_to_cleanup.append(profile._id) + credentials_to_cleanup.append(cred._id) + profiles_to_cleanup.append(profile._id) artifact = client.read_profiles(profile_id=profile._id).json() assert profile == artifact