From 7634f0f530f255ab4d0d276e212fdf115d32a32a Mon Sep 17 00:00:00 2001
From: Blallo <blallo@autistici.org>
Date: Sat, 13 Jul 2019 11:40:35 +0200
Subject: [PATCH] Better logging style and improved tests.

---
 async_tests/test_async_ldap_model.py | 20 +++++++++++++-------
 src/phi/async_ldap/model.py          |  5 ++++-
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/async_tests/test_async_ldap_model.py b/async_tests/test_async_ldap_model.py
index 8a406d5..35bada6 100644
--- a/async_tests/test_async_ldap_model.py
+++ b/async_tests/test_async_ldap_model.py
@@ -485,7 +485,7 @@ async def test_User_sync_existing(client_fixture, caplog):
 
     await c.sync()
 
-    assert "[existing_user] sync -> res:" in caplog.text
+    assert f"User({c.name}): synced" in caplog.text
     assert client_fixture.called_with_args["search"]["args"] == (c.dn, 0)
     for k, v in EXISTING_USER[0].items():
         if k != "dn":
@@ -504,7 +504,7 @@ async def test_User_sync_not_existing(client_fixture, caplog):
 
 
 @pytest.mark.asyncio
-async def test_User_modify_existing(client_fixture):
+async def test_User_modify_existing(client_fixture, caplog):
     c = User("existing_user", client_fixture)
     c._entry = mock.MagicMock()
 
@@ -516,12 +516,13 @@ async def test_User_modify_existing(client_fixture):
     await c.sync()
     await c.modify("mail", "other@existing.org")
 
+    assert f"User({c.name}): modified (mail)" in caplog.text
     c._entry.__setitem__.assert_called_with("mail", "other@existing.org")
     c._entry.__delitem__.assert_called_with("mail")
 
 
 @pytest.mark.asyncio
-async def test_User_modify_existing_append(client_fixture):
+async def test_User_modify_existing_append(client_fixture, caplog):
     c = User("existing_user", client_fixture)
     c._entry = mock.MagicMock()
 
@@ -533,6 +534,7 @@ async def test_User_modify_existing_append(client_fixture):
     await c.sync()
     await c.modify("mail", "other@existing.org", append=True)
 
+    assert f"User({c.name}): modified (mail)" in caplog.text
     c._entry.__setitem__.assert_called_with("mail", "other@existing.org")
     c._entry.__delitem__.assert_not_called()
 
@@ -541,7 +543,7 @@ async def test_User_modify_existing_append(client_fixture):
 async def test_User_modify_not_existing(client_fixture):
     c = User("existing_user", client_fixture)
     c._entry = mock.MagicMock()
-    attr = {'__delitem__.side_effect': KeyError}
+    attr = {"__delitem__.side_effect": KeyError}
     c._entry.configure_mock(**attr)
 
     async def _modify():
@@ -550,12 +552,14 @@ async def test_User_modify_not_existing(client_fixture):
     c._entry.modify = _modify
 
     await c.sync()
-    with pytest.raises(PhiAttributeMissing):
+    with pytest.raises(PhiAttributeMissing) as e:
         await c.modify("snafu", "modified")
 
+    assert c.dn in str(e.value)
+    assert "snafu" in str(e.value)
 
 @pytest.mark.asyncio
-async def test_User_remove_existing(client_fixture):
+async def test_User_remove_existing(client_fixture, caplog):
     c = User("existing_user", client_fixture)
     c._entry = mock.MagicMock()
 
@@ -570,6 +574,7 @@ async def test_User_remove_existing(client_fixture):
     await c.sync()
     await c.remove()
 
+    assert f"User({c.name}): removed" in caplog.text
     delete.assert_called_once()
 
 
@@ -585,9 +590,10 @@ async def test_User_remove_not_existing(client_fixture):
     delete = _delete()
     c._entry.delete = delete
 
-    with pytest.raises(PhiUserDoesNotExist):
+    with pytest.raises(PhiEntryDoesNotExist) as e:
         await c.remove()
 
+    assert c.dn in str(e.value)
 
 def test_Service(client_fixture):
     c = Service("phi", client_fixture)
diff --git a/src/phi/async_ldap/model.py b/src/phi/async_ldap/model.py
index b233a90..e79cc0a 100644
--- a/src/phi/async_ldap/model.py
+++ b/src/phi/async_ldap/model.py
@@ -265,16 +265,17 @@ class User(Hackers):
         self._entry = await create_new_(
             self, uid=self.name, mail=mail, sn=_sn, cn=_cn, userPassword=hashed
         )
+        alog.info("User(%s): created", self.name)
 
     async def sync(self):
         async with self.client.connect(is_async=True) as conn:
             res = await conn.search(self.dn, 0)
-        alog.debug("[%s] sync -> res: %s", self.name, res)
         if len(res) == 0:
             raise PhiEntryDoesNotExist(self.dn)
         for k, v in res[0].items():
             if not k == "dn":
                 self._entry[k] = v
+        alog.info("User(%s): synced", self.name)
 
     async def modify(self, key, value, append=False):
         async with self.client.connect(is_async=True) as conn:
@@ -286,12 +287,14 @@ class User(Hackers):
             except KeyError:
                 raise PhiAttributeMissing(self.dn, key)
             await self._entry.modify()
+            alog.info("User(%s): modified (%s)", self.name, key)
 
     async def remove(self):
         async with self.client.connect(is_async=True) as conn:
             self._entry.connection = conn
             try:
                 await self._entry.delete()
+                alog.info("User(%s): removed", self.name)
             except NoSuchObjectError:
                 raise PhiEntryDoesNotExist(self.dn)