From 5b032dd804f3ae32204beec996df60031246039d Mon Sep 17 00:00:00 2001 From: Kyle Speer <54034650+kspeer825@users.noreply.github.com> Date: Wed, 16 Mar 2022 14:17:34 -0400 Subject: [PATCH] Qa/configurable props (#32) * add conversion window test * add conversion window test * wip updated tests to worka with currently syncing dev branch [skip ci] * Revert removal of metric compatibility removal (#29) * Revert removal of metric compatibility removal * Whitespace cleanup * Add currently syncing (#24) * Sync all customers for a given stream * Add logging to see when we retry requests * Update currently_syncing with customerId too. Write state as soon as we update it * Add the customerId to the bookmark keys * Add shuffle for customerId and tap_stream_id; add shuffle unit tests * Bug fix for when currently_syncing is null * Fix exception handling typeError * Fix none cases for currently_syncing * Fix currently_syncing to write a tuple we can read in later * Add get_customer_ids so we can use it in the tests * Fix manipulated_state to account for customer_ids * Update assertion for currently_syncing * Fix currently syncing assertion * Move bookmark access into Full Table assertions section Full Table doesn't need the "stream_name and customer id" key logic * Remove duplicate assertion * Revert 6db016e7ec29c2b00973b671c1efdf9451aca9c2 * Update bookmark to read stream->customer->replication_key * Update tap to write bookmarks as stream->customer->replication_key * Update manipulated state to nest stream->customer->replication_key * Run bookmark assertions for every customer * Fix dict comprehension typo * Fix conflict with main * Remove `get_state_key` again, use env var instead of hardcoded value * Add missing dependency * Move currently-syncing-null-out to the end of sync to prevent gaps * Sort selected_streams and customers to guarantee consistency across runs * Don't let the tap write (None, None) * Sort selected_streams and customers effectively * Update currently_syncing test assertions * Add sort functions for streams and customers * Update `shuffle` to handle a missing value * Update unit tests to use sort_function, add a test for shuffling streams * Add end date (#28) * Add optional end date, add unit tests Co-authored-by: Andy Lu * Test functions can't be named run_test apparently * Rename do_thing * Extract `get_queries_from_sync` as a function * Remove unused variable * Refactor tests to be more explicit * Mock singer.utils.now to return a specific date Co-authored-by: Andy Lu * add conversion_window test * fixed conversion window unittests, bug removed Co-authored-by: dylan-stitch <28106103+dylan-stitch@users.noreply.github.com> Co-authored-by: Andy Lu Co-authored-by: kspeer * Bump to v0.2.0, update changelog (#31) * Bump to v0.2.0, update changelog * Add link for this PR, fix link syntax * Update changelog format * expanded conversion window testing for error case, BUG linked * parallelism 8 -> 12 * added unittest for start date within conversion window Co-authored-by: kspeer 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: Andy Lu --- .circleci/config.yml | 2 +- tests/test_google_ads_conversion_window.py | 120 ++++++++++++++++ ...st_google_ads_conversion_window_invalid.py | 131 ++++++++++++++++++ tests/unittests/test_conversion_window.py | 117 +++++++++++++--- 4 files changed, 350 insertions(+), 20 deletions(-) create mode 100644 tests/test_google_ads_conversion_window.py create mode 100644 tests/test_google_ads_conversion_window_invalid.py diff --git a/.circleci/config.yml b/.circleci/config.yml index 7179ce9..f1386b6 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -69,7 +69,7 @@ jobs: run_integration_tests: executor: docker-executor - parallelism: 8 + parallelism: 12 steps: - checkout - attach_workspace: diff --git a/tests/test_google_ads_conversion_window.py b/tests/test_google_ads_conversion_window.py new file mode 100644 index 0000000..88e83b1 --- /dev/null +++ b/tests/test_google_ads_conversion_window.py @@ -0,0 +1,120 @@ +"""Test tap configurable properties. Specifically the conversion_window""" +import os +from datetime import datetime as dt +from datetime import timedelta + +from tap_tester import menagerie, connections, runner + +from base import GoogleAdsBase + + +class ConversionWindowBaseTest(GoogleAdsBase): + """ + Test tap's sync mode can execute with valid conversion_window values set. + + Validate setting the conversion_window configurable property. + + Test Cases: + + Verify tap throws critical error when a value is provided directly by a user which is + outside the set of acceptable values. + + Verify connection can be created, and tap can discover and sync with a conversion window + set to the following values + Acceptable values: { 1 through 30, 60, 90} + """ + conversion_window = '' + + def name(self): + return f"tt_google_ads_conv_window_{self.conversion_window}" + + def get_properties(self): + """Configurable properties, with a switch to override the 'start_date' property""" + return { + 'start_date': dt.strftime(dt.utcnow() - timedelta(days=91), self.START_DATE_FORMAT), + 'user_id': 'not used?', # TODO ? + 'customer_ids': ','.join(self.get_customer_ids()), + 'conversion_window': self.conversion_window, + 'login_customer_ids': [{"customerId": os.getenv('TAP_GOOGLE_ADS_CUSTOMER_ID'), + "loginCustomerId": os.getenv('TAP_GOOGLE_ADS_LOGIN_CUSTOMER_ID'),}], + } + + def run_test(self): + """ + Testing that basic sync functions without Critical Errors when + a valid conversion_windown is set. + """ + print("Configurable Properties Test (conversion_window)") + + streams_to_test = { + 'campagins', + 'account_performance_report', + } + + # Create a connection + conn_id = connections.ensure_connection(self) + + # Run a discovery job + found_catalogs = self.run_and_verify_check_mode(conn_id) + + # Perform table and field selection... + core_catalogs = [catalog for catalog in found_catalogs + if not self.is_report(catalog['stream_name']) + and catalog['stream_name'] in streams_to_test] + report_catalogs = [catalog for catalog in found_catalogs + if self.is_report(catalog['stream_name']) + and catalog['stream_name'] in streams_to_test] + # select all fields for core streams and... + self.select_all_streams_and_fields(conn_id, core_catalogs, select_all_fields=True) + # select 'default' fields for report streams + self.select_all_streams_and_default_fields(conn_id, report_catalogs) + + # set state to ensure conversion window is used + today_datetime = dt.strftime(dt.utcnow(), self.REPLICATION_KEY_FORMAT) + customer_id = os.getenv('TAP_GOOGLE_ADS_CUSTOMER_ID') + initial_state = { + 'bookmarks': {stream: {customer_id: {'date': today_datetime}} + for stream in streams_to_test + if self.is_report(stream)} + } + menagerie.set_state(conn_id, initial_state) + + # Run a sync + sync_job_name = runner.run_sync_mode(self, conn_id) + + # Verify the tap and target do not throw a critical error + exit_status = menagerie.get_exit_status(conn_id, sync_job_name) + menagerie.verify_sync_exit_status(self, exit_status, sync_job_name) + + # Verify tap replicates through today by check state + final_state = menagerie.get_state(conn_id) + self.assertDictEqual(final_state, initial_state) + + +class ConversionWindowTestOne(ConversionWindowBaseTest): + + conversion_window = '1' + + def test_run(self): + self.run_test() + +class ConversionWindowTestThirty(ConversionWindowBaseTest): + + conversion_window = '30' + + def test_run(self): + self.run_test() + +class ConversionWindowTestSixty(ConversionWindowBaseTest): + + conversion_window = '60' + + def test_run(self): + self.run_test() + +class ConversionWindowTestNinety(ConversionWindowBaseTest): + + conversion_window = '90' + + def test_run(self): + self.run_test() diff --git a/tests/test_google_ads_conversion_window_invalid.py b/tests/test_google_ads_conversion_window_invalid.py new file mode 100644 index 0000000..32928e7 --- /dev/null +++ b/tests/test_google_ads_conversion_window_invalid.py @@ -0,0 +1,131 @@ +"""Test tap configurable properties. Specifically the conversion_window""" +import os +import unittest +from datetime import datetime as dt +from datetime import timedelta + +from tap_tester import menagerie, connections, runner + +from base import GoogleAdsBase + + +class ConversionWindowInvalidTest(GoogleAdsBase): + """ + Test tap's sync mode can execute with valid conversion_window values set. + + Validate setting the conversion_window configurable property. + + Test Cases: + + Verify tap throws critical error when a value is provided directly by a user which is + outside the set of acceptable values. + + Verify connection can be created, and tap can discover and sync with a conversion window + set to the following values + Acceptable values: { 1 through 30, 60, 90} + """ + conversion_window = '' + + def name(self): + return f"tt_googleads_conversion_invalid_{self.conversion_window}" + + def get_properties(self): + """Configurable properties, with a switch to override the 'start_date' property""" + return { + 'start_date': dt.strftime(dt.utcnow() - timedelta(days=91), self.START_DATE_FORMAT), + 'user_id': 'not used?', # TODO ? + 'customer_ids': ','.join(self.get_customer_ids()), + 'conversion_window': self.conversion_window, + 'login_customer_ids': [{"customerId": os.getenv('TAP_GOOGLE_ADS_CUSTOMER_ID'), + "loginCustomerId": os.getenv('TAP_GOOGLE_ADS_LOGIN_CUSTOMER_ID'),}], + } + + def run_test(self): + """ + Testing that basic sync functions without Critical Errors when + a valid conversion_windown is set. + """ + print("Configurable Properties Test (conversion_window)") + + streams_to_test = { + 'campagins', + 'account_performance_report', + } + + try: + # Create a connection + conn_id = connections.ensure_connection(self) + + with self.subTest(): + raise AssertionError(f"Conenction should not have been created with conversion_window: " + f"value {self.conversion_window}, type {type(self.conversion_window)}") + + # Run a discovery job + found_catalogs = self.run_and_verify_check_mode(conn_id) + + # Perform table and field selection... + core_catalogs = [catalog for catalog in found_catalogs + if not self.is_report(catalog['stream_name']) + and catalog['stream_name'] in streams_to_test] + report_catalogs = [catalog for catalog in found_catalogs + if self.is_report(catalog['stream_name']) + and catalog['stream_name'] in streams_to_test] + # select all fields for core streams and... + self.select_all_streams_and_fields(conn_id, core_catalogs, select_all_fields=True) + # select 'default' fields for report streams + self.select_all_streams_and_default_fields(conn_id, report_catalogs) + + # set state to ensure conversion window is used + today_datetime = dt.strftime(dt.utcnow(), self.REPLICATION_KEY_FORMAT) + customer_id = os.getenv('TAP_GOOGLE_ADS_CUSTOMER_ID') + initial_state = { + 'bookmarks': {stream: {customer_id: {'date': today_datetime}} + for stream in streams_to_test + if self.is_report(stream)} + } + menagerie.set_state(conn_id, initial_state) + + # Run a sync + sync_job_name = runner.run_sync_mode(self, conn_id) + + # Verify the tap and target throw a critical error + exit_status = menagerie.get_exit_status(conn_id, sync_job_name) + menagerie.verify_sync_exit_status(self, exit_status, sync_job_name) + + # Verify tap replicates through today by check state + final_state = menagerie.get_state(conn_id) + self.assertDictEqual(final_state, initial_state) + + with self.subTest(): + raise AssertionError("Tap should not have ran sync with conversion_window: " + f"value {self.conversion_window}, type {type(self.conversion_window)}") + + except Exception as ex: + err_msg_1 = "'message': 'properties do not match schema'" + err_msg_2 = "'bad_properties': ['conversion_window']" + + print("Expected exception occurred.") + + # Verify connection cannot be made with invalid conversion_window + print(f"Validating error message contains {err_msg_1}") + self.assertIn(err_msg_1, ex.args[0]) + print(f"Validating error message contains {err_msg_2}") + self.assertIn(err_msg_2, ex.args[0]) + + +class ConversionWindowTestZeroInteger(ConversionWindowInvalidTest): + + conversion_window = 0 + + @unittest.skip("https://jira.talendforge.org/browse/TDL-18168" + "[tap-google-ads] Invalid conversion_window values can be set when running tap directly") + def test_run(self): + self.run_test() + + +class ConversionWindowTestZeroString(ConversionWindowInvalidTest): + + conversion_window = '0' + + def test_run(self): + self.run_test() diff --git a/tests/unittests/test_conversion_window.py b/tests/unittests/test_conversion_window.py index 6b8d039..4e5dac4 100644 --- a/tests/unittests/test_conversion_window.py +++ b/tests/unittests/test_conversion_window.py @@ -33,26 +33,31 @@ def test_bookmark_within_90_day_conversion_window(self, fake_make_request): self.execute(conversion_window, fake_make_request) def execute(self, conversion_window, fake_make_request): - start_date = datetime(2021, 12, 1, 0, 0, 0) - # Create the stream so we can call sync - my_report_stream = ReportStream( - fields=[], - google_ads_resource_names=['accessible_bidding_strategy'], - resource_schema=resource_schema, - primary_keys=['foo'] - ) + # Set config using conversion_window under test + start_date = datetime(2021, 12, 1, 0, 0, 0) config = { "start_date": str(start_date), "conversion_window": str(conversion_window), } end_date = datetime.now() - bookmark_value = str(end_date - timedelta(days=(conversion_window - 5))) + # Set state to fall inside {today - conversion_window} + bookmark_value = str(end_date - timedelta(days=(conversion_window - 5))) state = { "currently_syncing": (None, None), "bookmarks": {"hi": {"123": {'date': bookmark_value}},} } + + # Create the stream so we can call sync + my_report_stream = ReportStream( + fields=[], + google_ads_resource_names=['accessible_bidding_strategy'], + resource_schema=resource_schema, + primary_keys=['foo'] + ) + + # Execute sync directly and record requests made for stream my_report_stream.sync( Mock(), {"customerId": "123", @@ -63,7 +68,6 @@ def execute(self, conversion_window, fake_make_request): config, state, ) - all_queries_requested = [] for request_sent in fake_make_request.call_args_list: # The function signature is gas, query, customer_id @@ -79,6 +83,7 @@ def execute(self, conversion_window, fake_make_request): # Verify the number of days queried is based off the conversion window. self.assertEqual(len(all_queries_requested), conversion_window + 1) # inclusive + class TestBookmarkOnConversionWindow(unittest.TestCase): @patch('tap_google_ads.streams.make_request') @@ -102,7 +107,21 @@ def test_bookmark_on_90_day_conversion_window(self, fake_make_request): self.execute(conversion_window, fake_make_request) def execute(self, conversion_window, fake_make_request): + + # Set config using conversion_window under test start_date = datetime(2021, 12, 1, 0, 0, 0) + config = { + "start_date": str(start_date), + "conversion_window": str(conversion_window), + } + end_date = datetime.now() + + # Set state to fall on the conversion_window date + bookmark_value = str(end_date - timedelta(days=conversion_window)) + state = { + "currently_syncing": (None, None), + "bookmarks": {"hi": {"123": {'date': bookmark_value}},} + } # Create the stream so we can call sync my_report_stream = ReportStream( @@ -111,17 +130,78 @@ def execute(self, conversion_window, fake_make_request): resource_schema=resource_schema, primary_keys=['foo'] ) + + # Execute sync directly and record requests made for stream + my_report_stream.sync( + Mock(), + {"customerId": "123", + "loginCustomerId": "456"}, + {"tap_stream_id": "hi", + "stream": "hi", + "metadata": []}, + config, + state, + ) + all_queries_requested = [] + for request_sent in fake_make_request.call_args_list: + # The function signature is gas, query, customer_id + _, query, _ = request_sent.args + all_queries_requested.append(query) + + + # Verify the first date queried is the conversion window date / bookmark + expected_first_query_date = str(bookmark_value)[:10] + actual_first_query_date = str(all_queries_requested[0])[-11:-1] + self.assertEqual(expected_first_query_date, actual_first_query_date) + + # Verify the number of days queried is based off the conversion window. + self.assertEqual(len(all_queries_requested), conversion_window + 1) # inclusive + + +class TestStartDateWithinConversionWindow(unittest.TestCase): + + @patch('tap_google_ads.streams.make_request') + def test_bookmark_on_1_day_conversion_window(self, fake_make_request): + conversion_window = 1 + self.execute(conversion_window, fake_make_request) + + @patch('tap_google_ads.streams.make_request') + def test_bookmark_on_default_conversion_window(self, fake_make_request): + conversion_window = 30 + self.execute(conversion_window, fake_make_request) + + @patch('tap_google_ads.streams.make_request') + def test_bookmark_on_60_day_conversion_window(self, fake_make_request): + conversion_window = 60 + self.execute(conversion_window, fake_make_request) + + @patch('tap_google_ads.streams.make_request') + def test_bookmark_on_90_day_conversion_window(self, fake_make_request): + conversion_window = 90 + self.execute(conversion_window, fake_make_request) + + def execute(self, conversion_window, fake_make_request): + + # Set config using conversion_window under test + start_date = datetime.now().replace(hour=0, minute=0, second=0) config = { "start_date": str(start_date), "conversion_window": str(conversion_window), } end_date = datetime.now() - bookmark_value = str(end_date - timedelta(days=conversion_window)) - state = { - "currently_syncing": (None, None), - "bookmarks": {"hi": {"123": {'date': bookmark_value}},} - } + # Set state to empty + state = {} + + # Create the stream so we can call sync + my_report_stream = ReportStream( + fields=[], + google_ads_resource_names=['accessible_bidding_strategy'], + resource_schema=resource_schema, + primary_keys=['foo'] + ) + + # Execute sync directly and record requests made for stream my_report_stream.sync( Mock(), {"customerId": "123", @@ -132,7 +212,6 @@ def execute(self, conversion_window, fake_make_request): config, state, ) - all_queries_requested = [] for request_sent in fake_make_request.call_args_list: # The function signature is gas, query, customer_id @@ -141,12 +220,12 @@ def execute(self, conversion_window, fake_make_request): # Verify the first date queried is the conversion window date (not the bookmark) - expected_first_query_date = str(bookmark_value)[:10] + expected_first_query_date = str(start_date)[:10] actual_first_query_date = str(all_queries_requested[0])[-11:-1] self.assertEqual(expected_first_query_date, actual_first_query_date) - # Verify the number of days queried is based off the conversion window. - self.assertEqual(len(all_queries_requested), conversion_window + 1) # inclusive + # Verify the number of days queried is based off the start_date + self.assertEqual(len(all_queries_requested), 1) if __name__ == '__main__':