Repository: google/mobly

Issue:
Mobly unit tests are flaky
Running the unit tests gives different results without code changes:

```
$ python setup.py test
running test
running egg_info
writing requirements to mobly.egg-info/requires.txt
writing mobly.egg-info/PKG-INFO
writing top-level names to mobly.egg-info/top_level.txt
writing dependency_links to mobly.egg-info/dependency_links.txt
reading manifest file 'mobly.egg-info/SOURCES.txt'
writing manifest file 'mobly.egg-info/SOURCES.txt'
running build_ext
==================================================================================================================== test session starts =====================================================================================================================
platform linux2 -- Python 2.7.6, pytest-3.0.5, py-1.4.31, pluggy-0.4.0
rootdir: /usr/local/google/home/adorokhine/work/mobly.git, inifile: 
collected 114 items 

tests/mobly_android_device_test.py ................
tests/mobly_asserts_test.py .
tests/mobly_base_class_test.py .....................................................
tests/mobly_logger_test.py .
tests/mobly_records_test.py ................
tests/mobly_sl4a_client_test.py ...........
tests/mobly_test_runner_test.py ...........
tests/mobly_utils_test.py ..F

========================================================================================================================== FAILURES ==========================================================================================================================
_______________________________________________________________________________________________________ MoblyUtilsTest.test_is_port_available_positive _______________________________________________________________________________________________________

self = <mobly_utils_test.MoblyUtilsTest testMethod=test_is_port_available_positive>

    def test_is_port_available_positive(self):
        # Unfortunately, we cannot do this test reliably for SOCK_STREAM
        # because the kernel allow this socket to linger about for some
        # small amount of time.  We're not using SO_REUSEADDR because we
        # are working around behavior on darwin where binding to localhost
        # on some port and then binding again to the wildcard address
        # with SO_REUSEADDR seems to be allowed.
        test_s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
        test_s.bind(('localhost', 0))
        port = test_s.getsockname()[1]
        test_s.close()
>       self.assertTrue(utils.is_port_available(port))
E       AssertionError: None is not true

tests/mobly_utils_test.py:51: AssertionError
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
============================================================================================================ 1 failed, 111 passed in 0.48 seconds ============================================================================================================
```

Ran again:
```
$ python setup.py test
running test
running egg_info
writing requirements to mobly.egg-info/requires.txt
writing mobly.egg-info/PKG-INFO
writing top-level names to mobly.egg-info/top_level.txt
writing dependency_links to mobly.egg-info/dependency_links.txt
reading manifest file 'mobly.egg-info/SOURCES.txt'
writing manifest file 'mobly.egg-info/SOURCES.txt'
running build_ext
==================================================================================================================== test session starts =====================================================================================================================
platform linux2 -- Python 2.7.6, pytest-3.0.5, py-1.4.31, pluggy-0.4.0
rootdir: /usr/local/google/home/adorokhine/work/mobly.git, inifile: 
collected 114 items 

tests/mobly_android_device_test.py ................
tests/mobly_asserts_test.py .
tests/mobly_base_class_test.py .....................................................
tests/mobly_logger_test.py .
tests/mobly_records_test.py ................
tests/mobly_sl4a_client_test.py ...........
tests/mobly_test_runner_test.py ...........
tests/mobly_utils_test.py .....

================================================================================================================= 114 passed in 0.56 seconds =================================================================================================================
```

Discussion:
xpconanfan: Ughhh...
I guess we have to revert back to before the code before the refactoring, to the code as is in g3 today.

Patch:
diff --git a/mobly/controllers/android_device_lib/adb.py b/mobly/controllers/android_device_lib/adb.py
index 4a57cf0..a3e66ec 100644
--- a/mobly/controllers/android_device_lib/adb.py
+++ b/mobly/controllers/android_device_lib/adb.py
@@ -37,6 +37,28 @@ class AdbError(Exception):
                 ) % (self.cmd, self.ret_code, self.stdout, self.stderr)
 
 
+def list_occupied_adb_ports():
+    """Lists all the host ports occupied by adb forward.
+
+    This is useful because adb will silently override the binding if an attempt
+    to bind to a port already used by adb was made, instead of throwing binding
+    error. So one should always check what ports adb is using before trying to
+    bind to a port with adb.
+
+    Returns:
+        A list of integers representing occupied host ports.
+    """
+    out = AdbProxy().forward("--list")
+    clean_lines = str(out, 'utf-8').strip().split('\n')
+    used_ports = []
+    for line in clean_lines:
+        tokens = line.split(" tcp:")
+        if len(tokens) != 3:
+            continue
+        used_ports.append(int(tokens[1]))
+    return used_ports
+
+
 class AdbProxy():
     """Proxy class for ADB.
 
diff --git a/mobly/utils.py b/mobly/utils.py
index 0a68c2d..9c52fea 100644
--- a/mobly/utils.py
+++ b/mobly/utils.py
@@ -30,6 +30,7 @@ import subprocess
 import time
 import traceback
 
+from mobly.controllers.android_device_lib import adb
 # File name length is limited to 255 chars on some OS, so we need to make sure
 # the file names we output fits within the limit.
 MAX_FILENAME_LEN = 255
@@ -429,21 +430,15 @@ def timeout(sec):
 
 
 def get_available_host_port():
-    """Finds a semi-random available port.
-
-    A race condition is still possible after the port number is returned, if
-    another process happens to bind it.
+    """Gets a host port number available for adb forward.
 
     Returns:
-        A port number that is unused on both TCP and UDP.
+        An integer representing a port number on the host available for adb
+        forward.
     """
-    # On the 2.6 kernel, calling _try_bind() on UDP socket returns the
-    # same port over and over. So always try TCP first.
     while True:
-        # Ask the OS for an unused port.
-        port = _try_bind(0, socket.SOCK_STREAM, socket.IPPROTO_TCP)
-        # Check if this port is unused on the other protocol.
-        if port and _try_bind(port, socket.SOCK_DGRAM, socket.IPPROTO_UDP):
+        port = random.randint(1024, 9900)
+        if is_port_available(port):
             return port
 
 
@@ -456,19 +451,18 @@ def is_port_available(port):
     Returns:
         True if the port is available; False otherwise.
     """
-    return (_try_bind(port, socket.SOCK_STREAM, socket.IPPROTO_TCP) and
-            _try_bind(port, socket.SOCK_DGRAM, socket.IPPROTO_UDP))
-
-
-def _try_bind(port, socket_type, socket_proto):
-    s = socket.socket(socket.AF_INET, socket_type, socket_proto)
+    # Make sure adb is not using this port so we don't accidentally interrupt
+    # ongoing runs by trying to bind to the port.
+    if port in adb.list_occupied_adb_ports():
+        return False
+    s = None
     try:
-        try:
-            s.bind(('', port))
-            # The result of getsockname() is protocol dependent, but for both
-            # IPv4 and IPv6 the second field is a port number.
-            return s.getsockname()[1]
-        except socket.error:
-            return None
+        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+        s.bind(('localhost', port))
+        return True
+    except socket.error:
+        return False
     finally:
-        s.close()
+        if s:
+            s.close()


Test patch:
diff --git a/tests/mobly/utils_test.py b/tests/mobly/utils_test.py
index 31d776a..fb2f519 100755
--- a/tests/mobly/utils_test.py
+++ b/tests/mobly/utils_test.py
@@ -14,6 +14,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import mock
 import socket
 import time
 import unittest
@@ -37,29 +38,16 @@ class UtilsTest(unittest.TestCase):
                                      "Process .* has terminated"):
             utils.stop_standing_subprocess(p)
 
-    def test_is_port_available_positive(self):
-        # Unfortunately, we cannot do this test reliably for SOCK_STREAM
-        # because the kernel allow this socket to linger about for some
-        # small amount of time.  We're not using SO_REUSEADDR because we
-        # are working around behavior on darwin where binding to localhost
-        # on some port and then binding again to the wildcard address
-        # with SO_REUSEADDR seems to be allowed.
-        test_s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
+    @mock.patch('mobly.controllers.android_device_lib.adb.list_occupied_adb_ports')
+    def test_is_port_available_positive(self, mock_list_occupied_adb_ports):
+        test_s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         test_s.bind(('localhost', 0))
         port = test_s.getsockname()[1]
         test_s.close()
         self.assertTrue(utils.is_port_available(port))
 
-    def test_detects_udp_port_in_use(self):
-        test_s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
-        test_s.bind(('localhost', 0))
-        port = test_s.getsockname()[1]
-        try:
-            self.assertFalse(utils.is_port_available(port))
-        finally:
-            test_s.close()
-
-    def test_detects_tcp_port_in_use(self):
+    @mock.patch('mobly.controllers.android_device_lib.adb.list_occupied_adb_ports')
+    def test_is_port_available_negative(self, mock_list_occupied_adb_ports):
         test_s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         test_s.bind(('localhost', 0))
         port = test_s.getsockname()[1]
@@ -68,6 +56,5 @@ class UtilsTest(unittest.TestCase):
         finally:
             test_s.close()
 
-
 if __name__ == "__main__":
-   unittest.main()
\ No newline at end of file
+   unittest.main()