fix

Author: pauldambra

posthog/test/test_utils.py

@@ -20,14 +20,14 @@ class TestUtils(unittest.TestCase):
     def test_timezone_utils(self):
         now = datetime.now()
         utcnow = datetime.now(tz=tzutc())
-        self.assertTrue(utils.is_naive(now))
-        self.assertFalse(utils.is_naive(utcnow))
+        assert utils.is_naive(now) is True
+        assert utils.is_naive(utcnow) is False
 
         fixed = utils.guess_timezone(now)
-        self.assertFalse(utils.is_naive(fixed))
+        assert utils.is_naive(fixed) is False
 
         shouldnt_be_edited = utils.guess_timezone(utcnow)
-        self.assertEqual(utcnow, shouldnt_be_edited)
+        assert utcnow == shouldnt_be_edited
 
     def test_clean(self):
         simple = {
@@ -54,20 +54,20 @@ def test_clean(self):
         pre_clean_keys = combined.keys()
 
         utils.clean(combined)
-        self.assertEqual(combined.keys(), pre_clean_keys)
+        assert combined.keys() == pre_clean_keys
 
         # test UUID separately, as the UUID object doesn't equal its string representation according to Python
-        self.assertEqual(
-            utils.clean(UUID("12345678123456781234567812345678")),
-            "12345678-1234-5678-1234-567812345678",
+        assert (
+            utils.clean(UUID("12345678123456781234567812345678"))
+            == "12345678-1234-5678-1234-567812345678"
         )
 
     def test_clean_with_dates(self):
         dict_with_dates = {
             "birthdate": date(1980, 1, 1),
             "registration": datetime.now(tz=tzutc()),
         }
-        self.assertEqual(dict_with_dates, utils.clean(dict_with_dates))
+        assert dict_with_dates == utils.clean(dict_with_dates)
 
     def test_bytes(self):
         if six.PY3:
@@ -79,14 +79,11 @@ def test_bytes(self):
 
     def test_clean_fn(self):
         cleaned = utils.clean({"fn": lambda x: x, "number": 4})
-        self.assertEqual(cleaned["number"], 4)
-        # TODO: fixme, different behavior on python 2 and 3
-        if "fn" in cleaned:
-            self.assertEqual(cleaned["fn"], None)
+        assert cleaned == {"fn": None, "number": 4}
 
     def test_remove_slash(self):
-        self.assertEqual("http://posthog.io", utils.remove_trailing_slash("http://posthog.io/"))
-        self.assertEqual("http://posthog.io", utils.remove_trailing_slash("http://posthog.io"))
+        assert "http://posthog.io" == utils.remove_trailing_slash("http://posthog.io/")
+        assert "http://posthog.io" == utils.remove_trailing_slash("http://posthog.io")
 
     def test_clean_pydantic(self):
         class ModelV2(BaseModel):
@@ -101,19 +98,22 @@ class ModelV1(BaseModelV1):
         class NestedModel(BaseModel):
             foo: ModelV2
 
-        self.assertEqual(utils.clean(ModelV2(foo="1", bar=2)), {"foo": "1", "bar": 2, "baz": None})
-        self.assertEqual(utils.clean(ModelV1(foo=1, bar="2")), {"foo": 1, "bar": "2"})
-        self.assertEqual(
-            utils.clean(NestedModel(foo=ModelV2(foo="1", bar=2, baz="3"))),
-            {"foo": {"foo": "1", "bar": 2, "baz": "3"}},
-        )
+        assert utils.clean(ModelV2(foo="1", bar=2)) == {
+            "foo": "1",
+            "bar": 2,
+            "baz": None,
+        }
+        assert utils.clean(ModelV1(foo=1, bar="2")) == {"foo": 1, "bar": "2"}
+        assert utils.clean(NestedModel(foo=ModelV2(foo="1", bar=2, baz="3"))) == {
+            "foo": {"foo": "1", "bar": 2, "baz": "3"}
+        }
 
         class Dummy:
             def model_dump(self, required_param):
                 pass
 
         # Skips a class with a defined non-Pydantic `model_dump` method.
-        self.assertEqual(utils.clean({"test": Dummy()}), {})
+        assert utils.clean({"test": Dummy()}) == {}
 
     def test_clean_dataclass(self):
         @dataclass
@@ -130,31 +130,28 @@ class TestDataClass:
             bar: int
             nested: InnerDataClass
 
-        self.assertEqual(
-            utils.clean(
-                TestDataClass(
-                    foo="1",
-                    bar=2,
-                    nested=InnerDataClass(
-                        inner_foo="3",
-                        inner_bar=4,
-                        inner_uuid=UUID("12345678123456781234567812345678"),
-                        inner_date=datetime(2025, 1, 1),
-                    ),
-                )
-            ),
-            {
-                "foo": "1",
-                "bar": 2,
-                "nested": {
-                    "inner_foo": "3",
-                    "inner_bar": 4,
-                    "inner_uuid": "12345678-1234-5678-1234-567812345678",
-                    "inner_date": datetime(2025, 1, 1),
-                    "inner_optional": None,
-                },
+        assert utils.clean(
+            TestDataClass(
+                foo="1",
+                bar=2,
+                nested=InnerDataClass(
+                    inner_foo="3",
+                    inner_bar=4,
+                    inner_uuid=UUID("12345678123456781234567812345678"),
+                    inner_date=datetime(2025, 1, 1),
+                ),
+            )
+        ) == {
+            "foo": "1",
+            "bar": 2,
+            "nested": {
+                "inner_foo": "3",
+                "inner_bar": 4,
+                "inner_uuid": "12345678-1234-5678-1234-567812345678",
+                "inner_date": datetime(2025, 1, 1),
+                "inner_optional": None,
             },
-        )
+        }
 
 
 class TestSizeLimitedDict(unittest.TestCase):
@@ -165,12 +162,12 @@ def test_size_limited_dict(self):
         for i in range(100):
             values[i] = i
 
-            self.assertEqual(values[i], i)
-            self.assertEqual(len(values), i % size + 1)
+            assert values[i] == i
+            assert len(values) == i % size + 1
 
             if i % size == 0:
                 # old numbers should've been removed
-                self.assertIsNone(values.get(i - 1))
-                self.assertIsNone(values.get(i - 3))
-                self.assertIsNone(values.get(i - 5))
-                self.assertIsNone(values.get(i - 9))
+                assert values.get(i - 1) is None
+                assert values.get(i - 3) is None
+                assert values.get(i - 5) is None
+                assert values.get(i - 9) is None

posthog/utils.py

@@ -106,17 +106,29 @@ def _coerce_unicode(cmplx: Any) -> Optional[str]:
     after many isinstance checks are carried out in `utils.clean`.
     When we supported Python 2 it was safe to call `decode` on a `str`
     but in Python 3 that will throw.
-    So, we check if the input is bytes and only call `decode` in that case
+    So, we check if the input is bytes and only call `decode` in that case.
+
+    Previously we would always call `decode` on the input
+    That would throw an error.
+    Then we would call `decode` on the stringified error
+    That would throw an error.
+    And then we would return `None`
+
+    To avoid a breaking change, we can maintain the behavior
+    that anything which did not have `decode` in Python 2
+    returns None.
     """
+    item = None
     try:
         if isinstance(cmplx, bytes):
             item = cmplx.decode("utf-8", "strict")
-        else:
-            item = str(cmplx)
+        elif isinstance(cmplx, str):
+            item = cmplx
     except Exception as exception:
         item = ":".join(map(str, exception.args))
         log.warning("Error decoding: %s", item)
         return None
+
     return item