TDL-20155 Add missing tap tester #46

prijendev · 2022-08-05T13:15:47Z

Description of change

Added bookmark and automatic_fields test cases.
Added missing assertion in rest of the test cases.

Manual QA steps

Risks

Rollback steps

revert this branch

savan-chovatiya · 2022-08-05T13:22:22Z

tests/base.py

+    BOOKMARK_FORMAT = "%Y-%m-%dT%H:%M:%S.%fZ"
+    INCREMENTAL = "INCREMENTAL"
+    FULL_TABLE = "FULL_TABLE"
+    ADITIONAL_AUTOMATIC = "aditional-automatic-fields"


Suggested change

ADITIONAL_AUTOMATIC = "aditional-automatic-fields"

ADDITIONAL_AUTOMATIC = "additional-automatic-fields"

I think this change is left to address.

savan-chovatiya · 2022-08-08T08:51:54Z

tests/test_start_date.py

+
+                    # Verify bookmark key values are greater than or equal to start_date of sync 1
+                    for start_date_key_value in start_date_key_sync_1:
+                        self.assertGreaterEqual(self.dt_to_ts(start_date_key_value), start_date_1_epoch)


Update dt_to_ts function in a base.py to convert date as per provided format. Add additional argument in function to define the format.

Updated the dt_to_ts function to call with date-format.

savan-chovatiya · 2022-08-08T08:53:05Z

tests/test_all_fields.py

@@ -20,7 +57,7 @@ def test_run(self):
        - Verify that more than just the automatic fields are replicated for each stream.
        """

-        expected_streams = self.expected_streams()
+        expected_streams = self.expected_streams() - {'ad_analytics_by_creative', 'ad_analytics_by_campaign'}


Add a comment about the above streams removal from expected_streams.

We are not going to skip these streams.

namrata270998 · 2022-08-08T09:58:00Z

tests/test_pagination.py

@@ -39,7 +47,7 @@ def test_run(self):
                                       for message in synced_records.get(stream).get('messages')
                                       if message.get('action') == 'upsert']

-                # verify records are more than page size so multiple page is working
+                # verify records are more than page size so multiple pages are working


Can you chunk pages and validate unique values across all pages instead of the just the first 2?
For reference https://github.com/singer-io/tap-mixpanel/blob/2cf3a86231da042f76f4e3ef9fa8c23600b6f593/tests/tap_tester/test_all_fields_pagination.py#L134-L153

Updated the code to validate unique values across all the pages instead of the first 2 pages.

namrata270998 · 2022-08-08T10:00:21Z

tests/test_pagination.py

+        """Returns the connection properties"""
+        return {
+            "start_date" : "2018-08-21T00:00:00Z",
+            "accounts": "503491473,502890776",


Can we use the env var instead of manually passing the account ids?

Removed get_properties since we can pass multiple accounts in the env variable.

namrata270998 · 2022-08-08T10:06:46Z

tests/test_pagination.py

+            "accounts": "503491473,502890776",
+            "page_size": 100
+        }
+


Also if we just wanna change the start date we can change the self.start_date and use connections.ensure_connection(self, original_properties=False) to use it

Updated the test.

namrata270998 · 2022-08-08T10:07:42Z

tests/test_all_fields.py

+            "accounts": os.getenv("TAP_LINKEDIN_ADS_ACCOUNTS"),
+            "page_size": 100
+        }
+


Also if we just wanna change the start date we can change the self.start_date and use connections.ensure_connection(self, original_properties=False) to use it

Updated the test.

namrata270998 · 2022-08-08T10:14:09Z

tests/test_bookmark.py

+        for stream, new_state in simulated_states.items():
+            new_states['bookmarks'][stream] = new_state


If the calculated_states_by_stream() returns the state in a dict format with stream name as keys, then I don't see any use of this for loop. If so can we please remove it?

Updated the code to directly assign dictionary, instead of looping through each stream.

tests/test_discovery.py

namrata270998 · 2022-08-08T10:25:03Z

tests/test_start_date.py

+            "start_date" : self.START_DATE,
+            "accounts": "503491473,502890776",
+        }
+


Can we remove this overriden get_properties() as the start_date will be overriden in the ensure_connection() itself?

Updated the test.

savan-chovatiya · 2022-08-22T12:28:55Z

tests/base.py

                self.PRIMARY_KEYS: {'id'},
-                self.REPLICATION_METHOD: 'INCREMENTAL',
+                self.REPLICATION_METHOD: self.INCREMENTAL,
                self.REPLICATION_KEYS: {'last_modified_time'}


Add OBEY_START_DATE and use it in the start_date test.

Added OBEY_START_DATE for the start_date test.

RushiT0122 · 2022-09-02T06:57:53Z

tests/test_start_date.py

@@ -49,50 +68,51 @@ def test_run(self):
        ##########################################################################

        print("REPLICATION START DATE CHANGE: {} ===>>> {} ".format(self.START_DATE, self.start_date_2))


Replace print statements by tap-tester LOGGER

Replaced print statements with LOGGER

RushiT0122 · 2022-09-02T07:00:41Z

tests/base.py

@@ -203,7 +218,7 @@ def expected_replication_keys(self):
    def run_and_verify_check_mode(self, conn_id):
        """
        Run the tap in check mode and verify it succeeds.
-        This should be ran prior to field selection and initial sync.
+        This should be run before field selection and initial sync.


Replace print statements by tap-tester LOGGER in the base.py

Replaced print statements with LOGGER

RushiT0122 · 2022-09-02T07:19:10Z

tests/test_start_date.py

@@ -12,35 +17,49 @@ def name():
        return "tap_tester_linkedin_ads_start_date_test"

    def test_run(self):
-        """Instantiate start date according to the desired data set and run the test"""
+
+        expected_streams = {"account_users"}


Can we rename this variable? On L#24 it reads little off as self.expected_streams() - expected_streams

RushiT0122 · 2022-09-02T07:29:41Z

tests/test_start_date.py

+            # Checking sync test for "ad_analytics_by_campaign", "ad_analytics_by_creative"
            if stream in ["ad_analytics_by_campaign", "ad_analytics_by_creative"]:
                self.assertGreater(record_count_by_stream_1.get(stream, 0), 0)
                self.assertGreater(record_count_by_stream_2.get(stream, 0), 0)


Why are we handling "ad_analytics_by_campaign", "ad_analytics_by_creative" separately? Also since this is not part of subTest(), on failure, execution will halt.

Missing record count greater than 0 for other streams.

RushiT0122 · 2022-09-02T07:33:19Z

tests/test_start_date.py

+        # Verify the total number of records replicated in sync 1 is
        # greater than the number of records replicated in sync 2
        self.assertGreater(sum(record_count_by_stream_1.values()), sum(record_count_by_stream_2.values()))



Do we need this assertion becuase we are testing this for individual stream on L#151? Also on failure, execution halt denying us to find out which stream is causing the issue.

Removed this assertion.

RushiT0122 · 2022-09-02T08:12:16Z

tests/test_sync_canary.py

@@ -2,6 +2,7 @@
 from base import TestLinkedinAdsBase

 class LinkedinAdsSyncTest(TestLinkedinAdsBase):
+    """Test tap sync mode and metadata conforms to standards."""


Aren't we covering this scenario in tests like all fields? Do we need this test?

Yes, we are covering this scenario in other test cases. Removed test_sync_canary test case.

RushiT0122 · 2022-09-02T08:15:03Z

tests/test_sync_without_refrence_organization_id.py

Please update clear objective of this test in doc string and why we are handling it separately since this is not a standard test.

There is a field reference_organization_id in the records of the users stream. If this field is absent, then the tap was throwing an error. It is fixed now and if this field is missing then the warning message would print. This test case is written to verify that case. But, this scenario is already covered in the unit test case. So, removed this tap tester test case.

* Refactored dict base code to class base. * Updated parent child bookmark logic * Added detailed unit test cases. * Resolved pylint errors. * Updated Makefile. * Update Makefile to run pylint. * Updated unit test cases. * Added integration test case for parent child independent. * Updated streams.py * Updated sync to resolve pylint. * Added detailed code comments. * Added unit test to achive more coverage * Updateed get_streams_to_sync method. * Added unit test scenarios for split chunk method. * Updated Makefile to update pylint. * Remove version pins for dev requirements (#38) * Remove version pins for dev requirements * Making pylint happy * Actually make pylint happy * Add standard list of disables to the extras already discussed Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com> Co-authored-by: Dan Mosora <30501696+dmosorast@users.noreply.github.com> Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com> * Reverted back pylint changes in config.yml * Resolved pylint errors. * typecast date_window_size with int data type. * Updated sync_without_refresh_token test. * Added missing fields in the schemas and upgraded API version. (#47) * Added missing fields in the schemas and upgraded API version. * Updated schemas and tap tester. * Updated streams file. * Updated schema files. * Updated all_field test case. * Updated all fields test case. * Replaced header param `expires_in` with `expires_at`. * Resolved pylint errors. * Replaced current_time with time.time() * Skipped fields for analytics stream that are in beta. Co-authored-by: Dylan <28106103+dsprayberry@users.noreply.github.com> Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com> Co-authored-by: Dan Mosora <30501696+dmosorast@users.noreply.github.com>

* Tdl 20157 Add backoff error handling for 5xx (#45) * Added backoff for all 5xx errors. * Skipped test case to check backoff. * Updated unit test case for exception handling. * Updated comment in client. * Updated error handling for 500 error. * TDL-20155 Add missing tap tester (#46) * Refactored tap tester test cases. * Updated pylint. * Added comments in the tap-tester. * Updated spell mistake in base.py * Updated base.py * Updated sync_without_resfresh_token test case. * Updated tap-tester test cases. * Removed sync_canary test case. * TDl-20154 dict base to class base implementation (#48) * Refactored dict base code to class base. * Updated parent child bookmark logic * Added detailed unit test cases. * Resolved pylint errors. * Updated Makefile. * Update Makefile to run pylint. * Updated unit test cases. * Added integration test case for parent child independent. * Updated streams.py * Updated sync to resolve pylint. * Added detailed code comments. * Added unit test to achive more coverage * Updateed get_streams_to_sync method. * Added unit test scenarios for split chunk method. * Updated Makefile to update pylint. * Remove version pins for dev requirements (#38) * Remove version pins for dev requirements * Making pylint happy * Actually make pylint happy * Add standard list of disables to the extras already discussed Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com> Co-authored-by: Dan Mosora <30501696+dmosorast@users.noreply.github.com> Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com> * Reverted back pylint changes in config.yml * Resolved pylint errors. * typecast date_window_size with int data type. * Updated sync_without_refresh_token test. * Added missing fields in the schemas and upgraded API version. (#47) * Added missing fields in the schemas and upgraded API version. * Updated schemas and tap tester. * Updated streams file. * Updated schema files. * Updated all_field test case. * Updated all fields test case. * Replaced header param `expires_in` with `expires_at`. * Resolved pylint errors. * Replaced current_time with time.time() * Skipped fields for analytics stream that are in beta. Co-authored-by: Dylan <28106103+dsprayberry@users.noreply.github.com> Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com> Co-authored-by: Dan Mosora <30501696+dmosorast@users.noreply.github.com> Co-authored-by: Dylan <28106103+dsprayberry@users.noreply.github.com> Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com> Co-authored-by: Dan Mosora <30501696+dmosorast@users.noreply.github.com> * Updated field name in campaigns. * TDL-20798 Write Access Token in Config (#52) * Initial commit. * Replaces expires_at with expires_in. * Updated unit tests. * Added comments to explain config rewrite. * Fixed all_fields test case. * Changes - Moved the unittest scenarios to test_client.py. Fixed test_all_fields by adding reference_person_id in the missing fields. * Fixed the arguments for Linkedin client in unit test case * Removed the functionality of generating new access token from the integration test * Fixed unit test case mock value * Reverted the unit test case name Co-authored-by: Sourabh Gandhi <sgandhi@talend.com> Co-authored-by: Dylan <28106103+dsprayberry@users.noreply.github.com> Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com> Co-authored-by: Dan Mosora <30501696+dmosorast@users.noreply.github.com> Co-authored-by: Sourabh Gandhi <sgandhi@talend.com>

* Refactored tap tester test cases. * Updated pylint. * Added comments in the tap-tester. * Updated spell mistake in base.py * Updated base.py * Updated sync_without_resfresh_token test case. * Updated tap-tester test cases. * Removed sync_canary test case. * TDl-20154 dict base to class base implementation (#48) * Refactored dict base code to class base. * Updated parent child bookmark logic * Added detailed unit test cases. * Resolved pylint errors. * Updated Makefile. * Update Makefile to run pylint. * Updated unit test cases. * Added integration test case for parent child independent. * Updated streams.py * Updated sync to resolve pylint. * Added detailed code comments. * Added unit test to achive more coverage * Updateed get_streams_to_sync method. * Added unit test scenarios for split chunk method. * Updated Makefile to update pylint. * Remove version pins for dev requirements (#38) * Remove version pins for dev requirements * Making pylint happy * Actually make pylint happy * Add standard list of disables to the extras already discussed Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com> Co-authored-by: Dan Mosora <30501696+dmosorast@users.noreply.github.com> Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com> * Reverted back pylint changes in config.yml * Resolved pylint errors. * typecast date_window_size with int data type. * Updated sync_without_refresh_token test. * Added missing fields in the schemas and upgraded API version. (#47) * Added missing fields in the schemas and upgraded API version. * Updated schemas and tap tester. * Updated streams file. * Updated schema files. * Updated all_field test case. * Updated all fields test case. * Replaced header param `expires_in` with `expires_at`. * Resolved pylint errors. * Replaced current_time with time.time() * Skipped fields for analytics stream that are in beta. Co-authored-by: Dylan <28106103+dsprayberry@users.noreply.github.com> Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com> Co-authored-by: Dan Mosora <30501696+dmosorast@users.noreply.github.com> Co-authored-by: Dylan <28106103+dsprayberry@users.noreply.github.com> Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com> Co-authored-by: Dan Mosora <30501696+dmosorast@users.noreply.github.com>

* Tdl 20157 Add backoff error handling for 5xx (#45) * Added backoff for all 5xx errors. * Skipped test case to check backoff. * Updated unit test case for exception handling. * Updated comment in client. * Updated error handling for 500 error. * TDL-20155 Add missing tap tester (#46) * Refactored tap tester test cases. * Updated pylint. * Added comments in the tap-tester. * Updated spell mistake in base.py * Updated base.py * Updated sync_without_resfresh_token test case. * Updated tap-tester test cases. * Removed sync_canary test case. * TDl-20154 dict base to class base implementation (#48) * Refactored dict base code to class base. * Updated parent child bookmark logic * Added detailed unit test cases. * Resolved pylint errors. * Updated Makefile. * Update Makefile to run pylint. * Updated unit test cases. * Added integration test case for parent child independent. * Updated streams.py * Updated sync to resolve pylint. * Added detailed code comments. * Added unit test to achive more coverage * Updateed get_streams_to_sync method. * Added unit test scenarios for split chunk method. * Updated Makefile to update pylint. * Remove version pins for dev requirements (#38) * Remove version pins for dev requirements * Making pylint happy * Actually make pylint happy * Add standard list of disables to the extras already discussed Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com> Co-authored-by: Dan Mosora <30501696+dmosorast@users.noreply.github.com> Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com> * Reverted back pylint changes in config.yml * Resolved pylint errors. * typecast date_window_size with int data type. * Updated sync_without_refresh_token test. * Added missing fields in the schemas and upgraded API version. (#47) * Added missing fields in the schemas and upgraded API version. * Updated schemas and tap tester. * Updated streams file. * Updated schema files. * Updated all_field test case. * Updated all fields test case. * Replaced header param `expires_in` with `expires_at`. * Resolved pylint errors. * Replaced current_time with time.time() * Skipped fields for analytics stream that are in beta. Co-authored-by: Dylan <28106103+dsprayberry@users.noreply.github.com> Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com> Co-authored-by: Dan Mosora <30501696+dmosorast@users.noreply.github.com> Co-authored-by: Dylan <28106103+dsprayberry@users.noreply.github.com> Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com> Co-authored-by: Dan Mosora <30501696+dmosorast@users.noreply.github.com> * Updated field name in campaigns. * TDL-20798 Write Access Token in Config (#52) * Initial commit. * Replaces expires_at with expires_in. * Updated unit tests. * Added comments to explain config rewrite. * Fixed all_fields test case. * Changes - Moved the unittest scenarios to test_client.py. Fixed test_all_fields by adding reference_person_id in the missing fields. * Fixed the arguments for Linkedin client in unit test case * Removed the functionality of generating new access token from the integration test * Fixed unit test case mock value * Reverted the unit test case name Co-authored-by: Sourabh Gandhi <sgandhi@talend.com> * Upgrade creatives api endpoint (#57) * modify code for the creatives API endpoint update * add specific headers for creatives endpoint and update the comments with the documentation link * make pylint happy. fix the integration tests. * update readme file * remove the older unit test case file * major bump version changes --------- Co-authored-by: Prijen Khokhani <88327452+prijendev@users.noreply.github.com> Co-authored-by: Dylan <28106103+dsprayberry@users.noreply.github.com> Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com> Co-authored-by: Dan Mosora <30501696+dmosorast@users.noreply.github.com> Co-authored-by: prijendev <prijen.khokhani@crestdatasys.com>

Refactored tap tester test cases.

5bbb36b

prijendev changed the title ~~Refactored tap tester test cases.~~ TDL-20155 Add missing tap tester Aug 5, 2022

prijendev requested review from dbshah1212 and savan-chovatiya August 5, 2022 13:16

savan-chovatiya reviewed Aug 5, 2022

View reviewed changes

savan-chovatiya reviewed Aug 8, 2022

View reviewed changes

namrata270998 reviewed Aug 8, 2022

View reviewed changes

tests/test_discovery.py Show resolved Hide resolved

namrata270998 reviewed Aug 8, 2022

View reviewed changes

Updated pylint.

185ae27

savan-chovatiya reviewed Aug 22, 2022

View reviewed changes

Added comments in the tap-tester.

b24dc2c

prijendev requested review from savan-chovatiya and namrata270998 August 23, 2022 09:15

prijendev added 2 commits August 23, 2022 18:45

Updated spell mistake in base.py

34a6811

Updated base.py

5e37ec9

dbshah1212 approved these changes Aug 24, 2022

View reviewed changes

savan-chovatiya approved these changes Aug 24, 2022

View reviewed changes

prijendev requested review from RushiT0122 and kspeer825 August 24, 2022 11:29

kethan1122 self-requested a review August 29, 2022 10:16

bryantgray approved these changes Aug 29, 2022

View reviewed changes

prijendev added 2 commits August 31, 2022 13:37

Merge branch 'crest-master' into TDL-20155-add-missing-tap-tester

3e7a37e

Updated sync_without_resfresh_token test case.

034b13e

RushiT0122 requested changes Sep 2, 2022

View reviewed changes

Updated tap-tester test cases.

9ca16d5

prijendev requested a review from RushiT0122 September 2, 2022 10:54

Removed sync_canary test case.

1175bab

RushiT0122 approved these changes Sep 2, 2022

View reviewed changes

prijendev merged commit df28ff0 into crest-master Sep 6, 2022

prijendev mentioned this pull request Sep 6, 2022

Crest master #49

Merged

sgandhi1311 mentioned this pull request Oct 10, 2022

minor bump version to 1.3.0 #54

Merged

dsprayberry deleted the TDL-20155-add-missing-tap-tester branch April 10, 2023 17:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TDL-20155 Add missing tap tester #46

TDL-20155 Add missing tap tester #46

prijendev commented Aug 5, 2022

savan-chovatiya Aug 5, 2022

savan-chovatiya Aug 23, 2022

prijendev Aug 23, 2022

savan-chovatiya Aug 8, 2022

prijendev Aug 12, 2022

savan-chovatiya Aug 8, 2022

prijendev Aug 12, 2022

namrata270998 Aug 8, 2022

prijendev Aug 12, 2022

namrata270998 Aug 8, 2022

prijendev Aug 12, 2022

namrata270998 Aug 8, 2022

prijendev Aug 12, 2022

namrata270998 Aug 8, 2022

prijendev Aug 12, 2022

namrata270998 Aug 8, 2022 •

edited

prijendev Aug 12, 2022

namrata270998 Aug 8, 2022 •

edited

prijendev Aug 12, 2022

savan-chovatiya Aug 22, 2022

prijendev Aug 23, 2022

RushiT0122 Sep 2, 2022

prijendev Sep 2, 2022

RushiT0122 Sep 2, 2022

prijendev Sep 2, 2022

RushiT0122 Sep 2, 2022

RushiT0122 Sep 2, 2022

RushiT0122 Sep 2, 2022

RushiT0122 Sep 2, 2022

prijendev Sep 2, 2022

RushiT0122 Sep 2, 2022

prijendev Sep 2, 2022 •

edited

RushiT0122 Sep 2, 2022

prijendev Sep 2, 2022

	ADITIONAL_AUTOMATIC = "aditional-automatic-fields"
	ADDITIONAL_AUTOMATIC = "additional-automatic-fields"

		for stream, new_state in simulated_states.items():
		new_states['bookmarks'][stream] = new_state

		@@ -49,50 +68,51 @@ def test_run(self):
		##########################################################################

		print("REPLICATION START DATE CHANGE: {} ===>>> {} ".format(self.START_DATE, self.start_date_2))

TDL-20155 Add missing tap tester #46

TDL-20155 Add missing tap tester #46

Conversation

prijendev commented Aug 5, 2022

Description of change

Manual QA steps

Risks

Rollback steps

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

namrata270998 Aug 8, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

namrata270998 Aug 8, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prijendev Sep 2, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

namrata270998 Aug 8, 2022 •

edited

namrata270998 Aug 8, 2022 •

edited

prijendev Sep 2, 2022 •

edited