Tdl 24145 #241
Tdl 24145 #241
Conversation
| "formField": True | ||
| } | ||
|
|
||
| # generate a contacts record |
There was a problem hiding this comment.
Dev has recommended using raise_for_status() after requests. As written we will just log bad responses and continue the test. Is this the desired behavior?
There was a problem hiding this comment.
post method is doing the raise_for_status check. create_contacts method is doing similar to what is beoing done here. I took that as the template.
tests/client.py
Outdated
| }, | ||
| { | ||
| "label": "Option C", | ||
| " value": "option_c" |
tests/test_hubspot_bookmarks.py
Outdated
| @@ -53,6 +53,9 @@ def create_test_data(self, expected_streams): | |||
| for stream in expected_streams} | |||
|
|
|||
| for stream in expected_streams - {'contacts_by_company'}: | |||
| """ Create custom properties for contacts """ | |||
There was a problem hiding this comment.
Is this meant to be a comment instead of a doc string?
There was a problem hiding this comment.
Meant to be a comment. I changed to #, don't really know the difference
| def get_custom_property(self, url, params=dict()): | ||
| """Perform a GET using the standard requests method and logs the action""" | ||
| """Do not handle exception for get failure in custom contact properties""" | ||
| response = requests.get(url, params=params, headers=self.HEADERS) |
There was a problem hiding this comment.
Consider using raise_for_status()
There was a problem hiding this comment.
This is intentional. I don't want to raise the exception - rather handle it. If the property is already there, I would delete it. See line# 789 where it is called. In the next line, I am checking the status
| if ( response.status_code == 404 ): | ||
| continue | ||
| url = f"{BASE_URL}/properties/v1/contacts/properties/named/" + property | ||
| response = self.delete(url) |
There was a problem hiding this comment.
Consider using raise_for_status(). Should we re-try if we fail on a delete?
There was a problem hiding this comment.
hmm... raise_for_status will log it as an error. I don't know if we want that.
| for record in actual_records: | ||
| for key, value in expected_primary_key_dict.items(): | ||
| actual_value = record[key] | ||
| if actual_value != value: |
There was a problem hiding this comment.
Would like to run this and see in debugger.
There was a problem hiding this comment.
Let me set up a time for this and to discuss the above comment
| ########################################################################## | ||
|
|
||
| @unittest.skip("Skip till all cards of missing fields are fixed. TDL-16145 ") | ||
| def test_all_fields_for_streams_are_replicated(self): |
There was a problem hiding this comment.
Seems like the same test is listed twice and skipped here the second time. Confusing
There was a problem hiding this comment.
I have removed the testname and added the code as a comment to avoid confusion
| 'subscription_changes', # BUG_TDL-14938 https://jira.talendforge.org/browse/TDL-14938 | ||
| }) | ||
|
|
||
| def setUp(self): |
There was a problem hiding this comment.
Usually setUp is written to only run once even if multiple tests are called. This implementation seems to only have one test so it may not be an issue but I'd like to maintain the pattern if possible.
There was a problem hiding this comment.
Let's discuss on how to do this tap specific setUp
| can_save = True | ||
| return ret_records | ||
|
|
||
| FIELDS_ADDED_BY_TAP = { |
There was a problem hiding this comment.
Many of these dicts, and methods will be useful in other tests. I think most of them belong in base.
Description of change
(https://jira.talendforge.org/browse/TDL-24145)
client.py - Added CRUD functionality for custom contact properties
test_hubspot_bookmarks.py - Call the create method in test data setup
Manual QA steps
Risks
Rollback steps