Update API tests to deal with pagination and new options #146
Conversation
camayoc/qcs_models.py
Outdated
| sat_ver = self.options.get('satellite_version', '6.2') | ||
| else: | ||
| sat_ver = '6.2' | ||
| if other.get(key).get('satellite_version') != sat_ver: |
There was a problem hiding this comment.
This logic is awkward and I'm interested to know if you have suggestion for some other flow.
There was a problem hiding this comment.
I don't have any good suggestion right now. The only thing I see is that if other does not have key then it will return None and then will raise AttributeError since None object does not have the get method. I would recommend calling other.get(key, {}).get('satellite_version').
| for server_source in server_srcs: | ||
| for created_source in created_srcs: | ||
| if server_source['id'] == created_source.fields()['id']: | ||
| assert_matches_server(created_source) |
There was a problem hiding this comment.
Now will fail on the guilty source
There was a problem hiding this comment.
This is good but it will only catch the first "invalid" occurrence. What if multiple sources are "invalid"?
I am leaving this as something to think about. I mean we can go with this as the improvement for now, but as a future though I think we should start trying to provide all the problematic instances in the future.
camayoc/qcs_models.py
Outdated
| sat_ver = self.options.get('satellite_version', '6.2') | ||
| else: | ||
| sat_ver = '6.2' | ||
| if other.get(key).get('satellite_version') != sat_ver: |
There was a problem hiding this comment.
I don't have any good suggestion right now. The only thing I see is that if other does not have key then it will return None and then will raise AttributeError since None object does not have the get method. I would recommend calling other.get(key, {}).get('satellite_version').
| created_srcs.remove(cp) | ||
| server_srcs = Source().list().json()['results'] | ||
| num_srcs = len(created_srcs) | ||
| for server_source in server_srcs: |
There was a problem hiding this comment.
I like having more descriptive names.
| for server_source in server_srcs: | ||
| for created_source in created_srcs: | ||
| if server_source['id'] == created_source.fields()['id']: | ||
| assert_matches_server(created_source) |
There was a problem hiding this comment.
This is good but it will only catch the first "invalid" occurrence. What if multiple sources are "invalid"?
I am leaving this as something to think about. I mean we can go with this as the improvement for now, but as a future though I think we should start trying to provide all the problematic instances in the future.
camayoc/tests/qcs/api/v1/utils.py
Outdated
| @@ -210,6 +210,8 @@ def prep_all_source_scan(cleanup, client, scan_type='inspect'): | |||
| hosts=s['hosts'], | |||
| credential_ids=[creds[s['credentials'][0]]], | |||
| ) | |||
| if s.get('options', False): | |||
There was a problem hiding this comment.
I am wondering if src.options will be defined by the constructor above? Even if assigned to None?
Because if it is set to None then this can become a one liner src.options = s.get('options'[, default]) (define default if the default value is different from None
| 'scan_type': 'connect', | ||
| 'sources': [153], | ||
| 'status': 'created' | ||
| 'id': 21, |
Updates tests to allow the results of listing all credentials or sources to be paginated. Also updates the equivlancy tests to set proper default values for ssl_cert_verify and satellite version. Closes #142
| else: | ||
| ssl_verify = True | ||
| if other.get(key, {}).get('ssl_cert_verify') != ssl_verify: | ||
| return False |
There was a problem hiding this comment.
updated this RE: your comment
|
thanks! |
Updates tests to allow the results of listing all credentials or sources to be
paginated. Also updates the equivlancy tests to set proper default values for
ssl_cert_verify and satellite version.
Closes #142