Conversion window validation (AutoIDM#33)

* Add conversion_window validation * Consolidate conversion window tests * Enable invalid conversion window integer test Co-authored-by: Bryant Gray <[email protected]>
dadosfera · Mar 18, 2022 · fe7cdf2 · fe7cdf2
1 parent 4c57419
commit fe7cdf2
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 3 deletions.
diff --git a/tap_google_ads/streams.py b/tap_google_ads/streams.py
@@ -23,6 +23,20 @@
 DEFAULT_CONVERSION_WINDOW = 30
 
 
+def get_conversion_window(config):
+    """Fetch the conversion window from the config and error on invalid values"""
+    conversion_window = config.get("conversion_window") or DEFAULT_CONVERSION_WINDOW
+
+    try:
+        conversion_window = int(conversion_window)
+    except (ValueError, TypeError) as err:
+        raise RuntimeError("Conversion Window must be an int or string") from err
+
+    if conversion_window in set(range(1,31)) or conversion_window in {60, 90}:
+        return conversion_window
+
+    raise RuntimeError("Conversion Window must be between 1 - 30 inclusive, 60, or 90")
+
 def create_nested_resource_schema(resource_schema, fields):
     new_schema = {
         "type": ["null", "object"],
@@ -440,7 +454,7 @@ def sync(self, sdk_client, customer, stream, config, state):
         singer.write_state(state)
 
         conversion_window = timedelta(
-            days=int(config.get("conversion_window") or DEFAULT_CONVERSION_WINDOW)
+            days=get_conversion_window(config)
         )
         conversion_window_date = utils.now().replace(hour=0, minute=0, second=0, microsecond=0) - conversion_window
 

diff --git a/tests/test_google_ads_conversion_window_invalid.py b/tests/test_google_ads_conversion_window_invalid.py
@@ -117,8 +117,6 @@ 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()
 

diff --git a/tests/unittests/test_conversion_window.py b/tests/unittests/test_conversion_window.py
@@ -4,6 +4,7 @@
 from unittest.mock import MagicMock
 from unittest.mock import Mock
 from unittest.mock import patch
+from tap_google_ads.streams import get_conversion_window
 from tap_google_ads.streams import ReportStream
 from tap_google_ads.streams import make_request
 
@@ -228,5 +229,48 @@ def execute(self, conversion_window, fake_make_request):
         self.assertEqual(len(all_queries_requested), 1)
 
 
+class TestGetConversionWindow(unittest.TestCase):
+    def test_int_conversion_date_in_allowable_range(self):
+        actual = get_conversion_window({"conversion_window": 12})
+        expected = 12
+        self.assertEqual(expected, actual)
+
+    def test_str_conversion_date_in_allowable_range(self):
+        actual = get_conversion_window({"conversion_window": "12"})
+        expected = 12
+        self.assertEqual(expected, actual)
+
+    def test_conversion_date_outside_allowable_range(self):
+        with self.assertRaises(RuntimeError):
+            get_conversion_window({"conversion_window": 42})
+
+        with self.assertRaises(RuntimeError):
+            get_conversion_window({"conversion_window": "42"})
+
+    def test_non_int_or_str_conversion_date(self):
+        with self.assertRaises(RuntimeError):
+            get_conversion_window({"conversion_window": {"12": 12}})
+
+        with self.assertRaises(RuntimeError):
+            get_conversion_window({"conversion_window": [12]})
+
+    def test_empty_data_types_conversion_date_returns_default(self):
+        expected = 30
+
+        actual = get_conversion_window({"conversion_window": ""})
+        self.assertEqual(expected, actual)
+
+        actual = get_conversion_window({"conversion_window": {}})
+        self.assertEqual(expected, actual)
+
+        actual = get_conversion_window({"conversion_window": []})
+        self.assertEqual(expected, actual)
+
+    def test_None_conversion_date_returns_default(self):
+        actual = get_conversion_window({"conversion_window": None})
+        expected = 30
+        self.assertEqual(expected, actual)
+
+
 if __name__ == '__main__':
     unittest.main()
diff --git a/tests/unittests/test_utils.py b/tests/unittests/test_utils.py
@@ -354,5 +354,6 @@ def test_shuffle_last_customer(self):
         ]
         self.assertListEqual(expected, actual)
 
+
 if __name__ == '__main__':
     unittest.main()