Repository: zodb/perfmetrics (language: python)

Issue:
FakeStatsdClient should define __bool__ to be true
It defines `__len__` and is thus falsey until a packet has been sent. But it's common for client code to write `if statsd_client(): ...`, leading to branches getting missed.

This would potentially be a breaking change for existing tests, so perhaps this should go in a subclass.

ETA: I doubt that's much of an issue, and I don't want to perpetuate the difference between the testing class and the other classes.


Discussion / hints:

Fix patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 49a75ed..3b2f908 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -9,6 +9,9 @@
 - Move to GitHub Actions from Travis CI.
 - Support PyHamcrest 1.10 and later. See `issue 26
   <https://github.com/zodb/perfmetrics/issues/26>`_.
+- The ``FakeStatsDClient`` for testing is now always true whether or
+  not any observations have been seen, like the normal clients. See
+  `issue <https://github.com/zodb/perfmetrics/issues/23>`_.
 
 3.0.0 (2019-09-03)
 ==================
@@ -35,7 +38,7 @@
   https://github.com/zodb/perfmetrics/issues/11.
 
 - Include support for testing applications instrumented with
-  perfmetrics in ``perfmetrics.testing``. This was previously release
+  perfmetrics in ``perfmetrics.testing``. This was previously released
   externally as ``nti.fakestatsd``. See https://github.com/zodb/perfmetrics/issues/9.
 
 - Read the ``PERFMETRICS_DISABLE_DECORATOR`` environment variable when


Test patch:
diff --git a/src/perfmetrics/testing/client.py b/src/perfmetrics/testing/client.py
index eb4b59c..8b0acc4 100644
--- a/src/perfmetrics/testing/client.py
+++ b/src/perfmetrics/testing/client.py
@@ -40,6 +40,10 @@ class FakeStatsDClient(StatsdClient):
     rather than pushing them over a socket. This class is a drop
     in replacement for `perfmetrics.statsd.StatsdClient` that collects statsd
     packets and `~.Observation` that are sent through the client.
+
+    .. versionchanged:: 3.1.0
+       Like the normal clients, this object is now always true, whether or
+       not any observations have been sent.
     """
 
     def __init__(self, prefix=''):
@@ -58,12 +62,15 @@ def clear(self):
         """
         self.udp_sock.clear()
 
+    def __bool__(self):
+        return True
+
+    __nonzero__ = __bool__ # Python 2
+
     def __len__(self):
         """
         The number of metrics sent. This accounts for multi metric packets
         that may be sent.
-
-        Note that this object will be false if no metrics have been sent.
         """
         return len(self.udp_sock.observations)
 
diff --git a/src/perfmetrics/testing/tests/test_client.py b/src/perfmetrics/testing/tests/test_client.py
index 6026251..ecc66bf 100644
--- a/src/perfmetrics/testing/tests/test_client.py
+++ b/src/perfmetrics/testing/tests/test_client.py
@@ -18,6 +18,7 @@
 from hamcrest import has_properties
 from hamcrest import has_property
 
+from nti.testing.matchers import is_true
 
 from .. import FakeStatsDClient as MockStatsDClient
 
@@ -31,6 +32,9 @@ class TestMockStatsDClient(unittest.TestCase):
     def setUp(self):
         self.client = MockStatsDClient()
 
+    def test_true_initially(self):
+        assert_that(self.client, is_true())
+
     def test_tracks_metrics(self):
         self.client.incr('mycounter')
         self.client.gauge('mygauge', 5)
@@ -57,10 +61,11 @@ def test_tracks_metrics(self):
     def test_clear(self):
         self.client.incr('mycounter')
         assert_that(self.client, has_length(1))
+        assert_that(self.client, is_true())
 
         self.client.clear()
         assert_that(self.client, has_length(0))
-
+        assert_that(self.client, is_true())
 
     def test_tracks_multimetrics(self):
         packet = 'gorets:1|c\nglork:320|ms\ngaugor:333|g\nuniques:765|s'
diff --git a/src/perfmetrics/tests/test_statsd.py b/src/perfmetrics/tests/test_statsd.py
index 08debd7..464de5b 100644
--- a/src/perfmetrics/tests/test_statsd.py
+++ b/src/perfmetrics/tests/test_statsd.py
@@ -7,6 +7,7 @@
 
 from hamcrest import assert_that
 from nti.testing.matchers import validly_provides
+from nti.testing.matchers import is_true
 
 from perfmetrics.interfaces import IStatsdClient
 
@@ -42,6 +43,9 @@ def _makeOne(self, *args, **kwargs):
     def test_implements(self):
         assert_that(self._makeOne(), validly_provides(IStatsdClient))
 
+    def test_true(self):
+        assert_that(self._makeOne(), is_true())
+
 class TestNullStatsdClient(TestBasics):
 
     @property