Merge pull request #1 from leohentschker/nit-fixing

Nits fixing error in mocking

cl/api/utils.py

@@ -732,31 +732,43 @@ def invert_user_logs(
     :return The inverted dictionary
     """
     r = get_redis_interface("STATS")
-    pipe = r.pipeline()
 
     dates = make_date_str_list(start, end)
-    for d in dates:
-        for version in ["v3", "v4"]:
+
+    versions = ["v3", "v4"]
+
+    for version in versions:
+        pipe = r.pipeline()
+        for d in dates:
             pipe.zrange(
                 f"api:{version}.user.d:{d}.counts", 0, -1, withscores=True
             )
-    results = pipe.execute()
+
+    all_results = pipe.execute()
+    results_by_version_all_dates = [
+        all_results[i : i + len(dates)]
+        for i in range(0, len(all_results), len(dates))
+    ]
 
     # results is a list of results for each of the zrange queries above. Zip
     # those results with the date that created it, and invert the whole thing.
     out: defaultdict = defaultdict(dict)
-    for d, result in zip(dates, results):
-        for user_id, count in result:
-            if user_id == "None" or user_id == "AnonymousUser":
-                user_id = "AnonymousUser"
-            else:
-                user_id = int(user_id)
-            count = int(count)
-            if out.get(user_id):
-                out[user_id][d] = count
-                out[user_id]["total"] += count
-            else:
-                out[user_id] = {d: count, "total": count}
+    for d, *results_by_version_for_date in zip(
+        dates, *results_by_version_all_dates
+    ):
+        for result in results_by_version_for_date:
+            for user_id, count in result:
+                if user_id == "None" or user_id == "AnonymousUser":
+                    user_id = "AnonymousUser"
+                else:
+                    user_id = int(user_id)
+                count = int(count)
+                if out.get(user_id):
+                    existing_count = out[user_id].get(d, 0)
+                    out[user_id][d] = existing_count + count
+                    out[user_id]["total"] += count
+                else:
+                    out[user_id] = {d: count, "total": count}
 
     # Sort the values
     for k, v in out.items():

cl/tests/api/utils.py

@@ -5,33 +5,32 @@
 
 USER_1_ID = 1
 USER_2_ID = 2
+USER_3_ID = 3
 
 
 class TestApiUsage(SimpleTestCase):
-    def setUp(self):
-        self.start_date = "2023-01-01"
-        self.end_date = "2023-01-01"
-        self.expected_dates = ["2023-01-01"]
-        self.v3_results = [(USER_1_ID, 10), (USER_2_ID, 20)]
-        self.v4_results = [(USER_1_ID, 15), (USER_2_ID, 25)]
 
     @patch("cl.api.utils.get_redis_interface")
-    def test_invert_user_logs_shows_v4_logs(self, mock_get_redis_interface):
+    def test_invert_user_logs_shows_v4_logs_one_date(
+        self, mock_get_redis_interface
+    ):
         mock_redis = MagicMock()
         mock_pipeline = MagicMock()
         mock_get_redis_interface.return_value = mock_redis
         mock_redis.pipeline.return_value = mock_pipeline
 
         mock_pipeline.execute.return_value = [
-            self.v3_results,
-            self.v4_results,
+            # v3
+            [(USER_1_ID, 10), (USER_2_ID, 20), (USER_3_ID, 30)],
+            # v4
+            [(USER_1_ID, 15), (USER_2_ID, 25), (USER_3_ID, 35)],
         ]
 
         results = invert_user_logs(
-            self.start_date, self.end_date, add_usernames=False
+            "2023-01-01", "2023-01-01", add_usernames=False
         )
 
-        for date in self.expected_dates:
+        for date in ["2023-01-01"]:
             mock_pipeline.zrange.assert_any_call(
                 f"api:v3.user.d:{date}.counts", 0, -1, withscores=True
             )
@@ -46,3 +45,50 @@ def test_invert_user_logs_shows_v4_logs(self, mock_get_redis_interface):
         self.assertEqual(results[USER_1_ID]["total"], 25)
         self.assertEqual(results[USER_2_ID]["2023-01-01"], 45)
         self.assertEqual(results[USER_2_ID]["total"], 45)
+        self.assertEqual(results[USER_3_ID]["2023-01-01"], 65)
+        self.assertEqual(results[USER_3_ID]["total"], 65)
+
+    @patch("cl.api.utils.get_redis_interface")
+    def test_invert_user_logs_shows_v4_logs_two_dates(
+        self, mock_get_redis_interface
+    ):
+        mock_redis = MagicMock()
+        mock_pipeline = MagicMock()
+        mock_get_redis_interface.return_value = mock_redis
+        mock_redis.pipeline.return_value = mock_pipeline
+
+        mock_pipeline.execute.return_value = [
+            # v3
+            [(USER_2_ID, 20), (USER_3_ID, 30)],
+            [(USER_1_ID, 15), (USER_2_ID, 25), (USER_3_ID, 35)],
+            # v4
+            [(USER_1_ID, 20), (USER_2_ID, 30)],
+            [(USER_1_ID, 25), (USER_2_ID, 35), (USER_3_ID, 45)],
+        ]
+
+        results = invert_user_logs(
+            "2023-01-01", "2023-01-02", add_usernames=False
+        )
+
+        for date in ["2023-01-01", "2023-01-02"]:
+            mock_pipeline.zrange.assert_any_call(
+                f"api:v3.user.d:{date}.counts", 0, -1, withscores=True
+            )
+            mock_pipeline.zrange.assert_any_call(
+                f"api:v4.user.d:{date}.counts", 0, -1, withscores=True
+            )
+
+        self.assertIn(USER_1_ID, results)
+        self.assertIn(USER_2_ID, results)
+
+        self.assertEqual(results[USER_1_ID]["2023-01-01"], 20)
+        self.assertEqual(results[USER_1_ID]["2023-01-02"], 40)
+        self.assertEqual(results[USER_1_ID]["total"], 60)
+
+        self.assertEqual(results[USER_2_ID]["2023-01-01"], 50)
+        self.assertEqual(results[USER_2_ID]["2023-01-02"], 60)
+        self.assertEqual(results[USER_2_ID]["total"], 110)
+
+        self.assertEqual(results[USER_3_ID]["2023-01-01"], 30)
+        self.assertEqual(results[USER_3_ID]["2023-01-02"], 80)
+        self.assertEqual(results[USER_3_ID]["total"], 110)