Improve test coverage

stephenrauch · stephenrauch · commit 75232ccf28a8 · 2016-04-02T13:49:40.000-07:00
Add more needed shutdown cleanup found via additional test coverage.

Force timeout calculation from milli to seconds to use floating point.
diff --git a/.gitignore b/.gitignore
@@ -1,2 +1,12 @@
 build/
+*.pyc
+*.pyo
+Thumbs.db
+.DS_Store
+.project
+.pydevproject
+.settings
+.idea
+.vslick
+.cache
 
diff --git a/setup.cfg b/setup.cfg
@@ -5,3 +5,4 @@ universal = 1
 show-source = 1
 import-order-style=google
 application-import-names=zeroconf
+max-line-length=110
diff --git a/test_zeroconf.py b/test_zeroconf.py
@@ -7,6 +7,7 @@
 import logging
 import socket
 import struct
+import time
 import unittest
 from threading import Event
 
@@ -149,6 +150,84 @@ def test_launch_and_close(self):
         rv.close()
 
 
+class Exceptions(unittest.TestCase):
+
+    def test_bad_service_info_name(self):
+        browser = Zeroconf()
+        self.assertRaises(r.BadTypeInNameException,
+                          browser.get_service_info, "type", "type_not")
+        browser.close()
+
+
+class Listener(unittest.TestCase):
+
+    def test_integration_with_listener_class(self):
+
+        service_added = Event()
+        service_removed = Event()
+
+        type_ = "_http._tcp.local."
+        name = "xxxyyy"
+        registration_name = "%s.%s" % (name, type_)
+
+        class MyListener(object):
+            def add_service(self, zeroconf, type, name):
+                zeroconf.get_service_info(type, name)
+                service_added.set()
+
+            def remove_service(self, zeroconf, type, name):
+                service_removed.set()
+
+        zeroconf_browser = Zeroconf()
+        zeroconf_browser.add_service_listener(type_, MyListener())
+
+        properties = dict(
+            prop_none=None,
+            prop_string=b'a_prop',
+            prop_float=1.0,
+            prop_blank=b'a blanked string',
+            prop_true=1,
+            prop_false=0,
+        )
+
+        zeroconf_registrar = Zeroconf()
+        desc = {'path': '/~paulsm/'}
+        desc.update(properties)
+        info = ServiceInfo(
+            type_, registration_name,
+            socket.inet_aton("10.0.1.2"), 80, 0, 0,
+            desc, "ash-2.local.")
+        zeroconf_registrar.register_service(info)
+
+        try:
+            service_added.wait(1)
+            assert service_added.is_set()
+
+            # short pause to allow multicast timers to expire
+            time.sleep(2)
+
+            # clear the answer cache to force query
+            for record in zeroconf_browser.cache.entries():
+                zeroconf_browser.cache.remove(record)
+
+            # get service info without answer cache
+            info = zeroconf_browser.get_service_info(type_, registration_name)
+
+            assert info.properties[b'prop_none'] is False
+            assert info.properties[b'prop_string'] == properties['prop_string']
+            assert info.properties[b'prop_float'] is False
+            assert info.properties[b'prop_blank'] == properties['prop_blank']
+            assert info.properties[b'prop_true'] is True
+            assert info.properties[b'prop_false'] is False
+
+            zeroconf_registrar.unregister_service(info)
+            service_removed.wait(1)
+            assert service_removed.is_set()
+        finally:
+            zeroconf_registrar.close()
+            zeroconf_browser.close()
+
+
 def test_integration():
     service_added = Event()
     service_removed = Event()
@@ -177,9 +256,8 @@ def on_service_state_change(zeroconf, service_type, state_change, name):
     try:
         service_added.wait(1)
         assert service_added.is_set()
-        zeroconf_registrar.unregister_service(info)
-        service_removed.wait(1)
-        assert service_removed.is_set()
+        # Don't remove service, allow close() to cleanup
+
     finally:
         zeroconf_registrar.close()
         browser.cancel()
diff --git a/zeroconf.py b/zeroconf.py
@@ -64,10 +64,6 @@ def emit(self, record):
 if log.level == logging.NOTSET:
     log.setLevel(logging.WARN)
 
-# hook for threads
-
-_GLOBAL_DONE = False
-
 # Some timing constants
 
 _UNREGISTER_TIME = 125
@@ -292,7 +288,7 @@ def get_expiration_time(self, percent):
 
     def get_remaining_ttl(self, now):
         """Returns the remaining TTL in seconds."""
-        return max(0, (self.get_expiration_time(100) - now) / 1000)
+        return max(0, (self.get_expiration_time(100) - now) / 1000.0)
 
     def is_expired(self, now):
         """Returns true if this record has expired."""
@@ -820,7 +816,7 @@ def __init__(self, zc):
         self.start()
 
     def run(self):
-        while not _GLOBAL_DONE:
+        while not self.zc._GLOBAL_DONE:
             with self.condition:
                 rs = self.readers.keys()
                 if len(rs) == 0:
@@ -831,7 +827,7 @@ def run(self):
             if len(rs) != 0:
                 try:
                     rr, wr, er = select.select(rs, [], [], self.timeout)
-                    if not _GLOBAL_DONE:
+                    if not self.zc._GLOBAL_DONE:
                         for socket_ in rr:
                             reader = self.readers.get(socket_)
                             if reader:
@@ -840,7 +836,7 @@ def run(self):
                 except socket.error as e:
                     # If the socket was closed by another thread, during
                     # shutdown, ignore it and exit
-                    if e.errno != socket.EBADF or not _GLOBAL_DONE:
+                    if e.errno != socket.EBADF or not self.zc._GLOBAL_DONE:
                         raise
 
     def add_reader(self, reader, socket_):
@@ -901,7 +897,7 @@ def __init__(self, zc):
     def run(self):
         while True:
             self.zc.wait(10 * 1000)
-            if _GLOBAL_DONE:
+            if self.zc._GLOBAL_DONE:
                 return
             now = current_time_millis()
             for record in self.zc.cache.entries():
@@ -1035,7 +1031,7 @@ def run(self):
             now = current_time_millis()
             if len(self._handlers_to_call) == 0 and self.next_time > now:
                 self.zc.wait(self.next_time - now)
-            if _GLOBAL_DONE or self.done:
+            if self.zc._GLOBAL_DONE or self.done:
                 return
             now = current_time_millis()
 
@@ -1049,7 +1045,7 @@ def run(self):
                 self.next_time = now + self.delay
                 self.delay = min(20 * 1000, self.delay * 2)
 
-            if len(self._handlers_to_call) > 0 and not _GLOBAL_DONE:
+            if len(self._handlers_to_call) > 0 and not self.zc._GLOBAL_DONE:
                 handler = self._handlers_to_call.pop(0)
                 handler(self.zc)
 
@@ -1345,8 +1341,8 @@ def __init__(
 
         :type interfaces: :class:`InterfaceChoice` or sequence of ip addresses
         """
-        global _GLOBAL_DONE
-        _GLOBAL_DONE = False
+        # hook for threads
+        self._GLOBAL_DONE = False
 
         self._listen_socket = new_socket()
         interfaces = normalize_interface_choice(interfaces, socket.AF_INET)
@@ -1398,7 +1394,7 @@ def wait(self, timeout):
         """Calling thread waits for a given number of milliseconds or
         until notified."""
         with self.condition:
-            self.condition.wait(timeout / 1000)
+            self.condition.wait(timeout / 1000.0)
 
     def notify_all(self):
         """Notifies all waiting threads"""
@@ -1429,7 +1425,7 @@ def remove_service_listener(self, listener):
 
     def remove_all_service_listeners(self):
         """Removes a listener from the set that is currently listening."""
-        for listener in self.browsers.keys():
+        for listener in [k for k in self.browsers]:
             self.remove_service_listener(listener)
 
     def register_service(self, info, ttl=_DNS_TTL):
@@ -1674,7 +1670,7 @@ def send(self, out, addr=_MDNS_ADDR, port=_MDNS_PORT):
         packet = out.packet()
         log.debug('Sending %r as %r...', out, packet)
         for s in self._respond_sockets:
-            if _GLOBAL_DONE:
+            if self._GLOBAL_DONE:
                 return
             bytes_sent = s.sendto(packet, 0, (addr, port))
             if bytes_sent != len(packet):
@@ -1685,17 +1681,19 @@ def send(self, out, addr=_MDNS_ADDR, port=_MDNS_PORT):
     def close(self):
         """Ends the background threads, and prevent this instance from
         servicing further queries."""
-        global _GLOBAL_DONE
-        if not _GLOBAL_DONE:
-            _GLOBAL_DONE = True
+        if not self._GLOBAL_DONE:
+            self._GLOBAL_DONE = True
+            # remove service listeners
+            self.remove_all_service_listeners()
+            self.unregister_all_services()
+
             # shutdown recv socket and thread
             self.engine.del_reader(self._listen_socket)
             self._listen_socket.close()
             self.engine.join()
 
             # shutdown the rest
             self.notify_all()
-            self.unregister_all_services()
-            self.remove_all_service_listeners()
+            self.reaper.join()
             for s in self._respond_sockets:
                 s.close()
