Repository: opensistemas-hub/osbrain
Pull request #69 (2017-03-10 10:26:26)

Improve test coverage in NameServerProcess
Here is my attempt... Note that if you don't like it we can still consider cherry-picking the first commit (182d3cbb089f2100dd7b4babcf24e3815dea4495), which implements scheduled agent shutdowns (it can be useful anyways).

codecov[bot]: # [Codecov](https://codecov.io/gh/opensistemas-hub/osbrain/pull/69?src=pr&el=h1) Report
> Merging [#69](https://codecov.io/gh/opensistemas-hub/osbrain/pull/69?src=pr&el=desc) into [master](https://codecov.io/gh/opensistemas-hub/osbrain/commit/338b8b488bc9d1a0b6df366550535f3919b1272e?src=pr&el=desc) will **decrease** coverage by `0.18%`.
> The diff coverage is `69.23%`.


```diff
@@            Coverage Diff             @@
##           master      #69      +/-   ##
==========================================
- Coverage   95.88%   95.69%   -0.19%     
==========================================
  Files           7        7              
  Lines         875      883       +8     
  Branches      124      125       +1     
==========================================
+ Hits          839      845       +6     
- Misses         26       28       +2     
  Partials       10       10
```


| [Impacted Files](https://codecov.io/gh/opensistemas-hub/osbrain/pull/69?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [osbrain/agent.py](https://codecov.io/gh/opensistemas-hub/osbrain/compare/338b8b488bc9d1a0b6df366550535f3919b1272e...7fb07e4490bb0ef47af4f841861f826f4447879e?src=pr&el=tree#diff-b3NicmFpbi9hZ2VudC5weQ==) | `92.91% <100%> (+0.03%)` | :white_check_mark: |
| [osbrain/nameserver.py](https://codecov.io/gh/opensistemas-hub/osbrain/compare/338b8b488bc9d1a0b6df366550535f3919b1272e...7fb07e4490bb0ef47af4f841861f826f4447879e?src=pr&el=tree#diff-b3NicmFpbi9uYW1lc2VydmVyLnB5) | `95.72% <63.63%> (-1.58%)` | :x: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/opensistemas-hub/osbrain/pull/69?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/opensistemas-hub/osbrain/pull/69?src=pr&el=footer). Last update [338b8b4...7fb07e4](https://codecov.io/gh/opensistemas-hub/osbrain/compare/338b8b488bc9d1a0b6df366550535f3919b1272e...7fb07e4490bb0ef47af4f841861f826f4447879e?el=footer&src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
Peque: I simplified the code a bit as it didn't make sense to have `agent.shutdown(after=1)` when we could already do `agent.after(1, 'shutdown')`. According to the zen of Python "There should be one-- and preferably only one --obvious way to do it.".

Patch:
diff --git a/osbrain/nameserver.py b/osbrain/nameserver.py
index 9b0c4b2..03b6fec 100644
--- a/osbrain/nameserver.py
+++ b/osbrain/nameserver.py
@@ -130,8 +130,8 @@ class NameServerProcess(multiprocessing.Process):
         Shutdown all agents registered in the name server.
         """
         for agent in self.agents():
-            agent = Proxy(agent, self.addr)
-            agent.shutdown()
+            with Proxy(agent, self.addr) as agent:
+                agent.after(0, 'shutdown')
 
     def shutdown(self):
         """
@@ -147,10 +147,10 @@ class NameServerProcess(multiprocessing.Process):
         self.join()
 
 
-def random_nameserver(host='127.0.0.1', port_start=10000, port_stop=20000,
-                      timeout=3.):
+def random_nameserver_process(host='127.0.0.1', port_start=10000,
+                              port_stop=20000, timeout=3.):
     """
-    Start a random name server.
+    Start a random NameServerProcess.
 
     Parameters
     ----------
@@ -163,8 +163,8 @@ def random_nameserver(host='127.0.0.1', port_start=10000, port_stop=20000,
 
     Returns
     -------
-    SocketAddress
-        The name server address.
+    NameServerProcess
+        The name server process started.
     """
     t0 = time.time()
     exception = TimeoutError('Name server starting timed out!')
@@ -175,7 +175,7 @@ def random_nameserver(host='127.0.0.1', port_start=10000, port_stop=20000,
             addr = SocketAddress(host, port)
             nameserver = NameServerProcess(addr)
             nameserver.start()
-            return addr
+            return nameserver
         except RuntimeError as error:
             exception = error
         if time.time() - t0 > timeout:
@@ -200,7 +200,7 @@ def run_nameserver(addr=None):
         A proxy to the name server.
     """
     if not addr:
-        addr = random_nameserver()
+        addr = random_nameserver_process().addr
     else:
         NameServerProcess(addr).start()
     return NSProxy(addr)


Test patch:
diff --git a/osbrain/tests/test_nameserver.py b/osbrain/tests/test_nameserver.py
index b31cafc..bcab5b7 100644
--- a/osbrain/tests/test_nameserver.py
+++ b/osbrain/tests/test_nameserver.py
@@ -2,6 +2,7 @@
 Test file for nameserver.
 """
 import os
+import time
 import random
 from threading import Timer
 
@@ -10,9 +11,10 @@ import pytest
 from osbrain import SocketAddress
 from osbrain import run_agent
 from osbrain import run_nameserver
+from osbrain import Agent
 from osbrain import NSProxy
 from osbrain.nameserver import NameServerProcess
-from osbrain.nameserver import random_nameserver
+from osbrain.nameserver import random_nameserver_process
 
 from common import nsaddr  # pragma: no flakes
 from common import nsproxy  # pragma: no flakes
@@ -81,19 +83,9 @@ def test_nameserverprocess_shutdown():
     """
     Name server shutdown can be called directly from the name server process.
     """
-    while True:
-        try:
-            # Bind to random port
-            port = random.randrange(10000, 20000)
-            addr = SocketAddress('127.0.0.1', port)
-            nameserver = NameServerProcess(addr)
-            nameserver.start()
-            break
-        except RuntimeError:
-            pass
-    nsaddr = nameserver.addr
-    run_agent('a0', nsaddr=nsaddr)
-    run_agent('a1', nsaddr=nsaddr)
+    nameserver = random_nameserver_process()
+    run_agent('a0')
+    run_agent('a1')
     while not len(nameserver.agents()) == 2:
         continue
     assert 'a0' in nameserver.agents()
@@ -102,6 +94,25 @@ def test_nameserverprocess_shutdown():
     assert not nameserver.is_alive()
 
 
+def test_nameserverprocess_shutdown_lazy_agents():
+    """
+    Shutdown a name server process with agents that wait some time before
+    shutting down.
+    """
+    class Lazy(Agent):
+        def shutdown(self):
+            time.sleep(1)
+            super().shutdown()
+
+    nsprocess = random_nameserver_process()
+    run_agent('a0', base=Lazy)
+    run_agent('a1', base=Lazy)
+
+    t0 = time.time()
+    nsprocess.shutdown()
+    assert time.time() - t0 > 2
+
+
 def test_nameserver_proxy_timeout():
     """
     When creating a proxy to the name server, there should be a timeout
@@ -182,22 +193,24 @@ def test_nameserver_agent_address(nsproxy):
     assert nsproxy.addr('a1', 'bar') == addr1
 
 
-def test_random_nameserver():
+def test_random_nameserver_process():
     """
-    Basic random_nameserver function tests: port range and exceptions.
+    Basic random_nameserver_process function tests: port range and exceptions.
     """
     # Port range
     port_start = 11000
     port_stop = port_start + 100
-    address = random_nameserver(port_start=port_start, port_stop=port_stop)
+    nsprocess = random_nameserver_process(port_start=port_start,
+                                          port_stop=port_stop)
+    address = nsprocess.addr
     assert port_start <= address.port <= port_stop
     ns = NSProxy(address)
     ns.shutdown()
     # Raising exceptions
     with pytest.raises(ValueError):
-        random_nameserver(port_start=-1, port_stop=-2)
+        random_nameserver_process(port_start=-1, port_stop=-2)
     with pytest.raises(RuntimeError):
-        random_nameserver(port_start=22, port_stop=22, timeout=0.5)
+        random_nameserver_process(port_start=22, port_stop=22, timeout=0.5)
 
 
 def test_nameserver_oserror(nsaddr):