adb: Improve test_adb a bit more

This change:

* uses unittest.main(), which allows for a subset of the tests to be
  selected.
* drops the requirement to have a device already connected since all the
  tests that need a device now spin their own mock device.
* Splits the monolithic test class into more granular classes.
* Makes this file be pylint-compliant.

Bug: None
Test: python system/core/adb/test_adb.py
Test: pylint system/core/adb/test_adb.py
Change-Id: I91c7ced520c3c69f855d639e0dbf7e57bb690e97
diff --git a/adb/test_adb.py b/adb/test_adb.py
index ce4d4ec..8f31a53 100644
--- a/adb/test_adb.py
+++ b/adb/test_adb.py
@@ -36,10 +36,11 @@
 
 
 @contextlib.contextmanager
-def fake_adb_server(protocol=socket.AF_INET, port=0):
-    """Creates a fake ADB server that just replies with a CNXN packet."""
+def fake_adbd(protocol=socket.AF_INET, port=0):
+    """Creates a fake ADB daemon that just replies with a CNXN packet."""
 
     serversock = socket.socket(protocol, socket.SOCK_STREAM)
+    serversock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
     if protocol == socket.AF_INET:
         serversock.bind(('127.0.0.1', port))
     else:
@@ -60,33 +61,33 @@
         rlist = [readpipe, serversock]
         cnxn_sent = {}
         while True:
-            ready, _, _ = select.select(rlist, [], [])
-            for r in ready:
-                if r == readpipe:
+            read_ready, _, _ = select.select(rlist, [], [])
+            for ready in read_ready:
+                if ready == readpipe:
                     # Closure pipe
-                    os.close(r)
+                    os.close(ready)
                     serversock.shutdown(socket.SHUT_RDWR)
                     serversock.close()
                     return
-                elif r == serversock:
+                elif ready == serversock:
                     # Server socket
-                    conn, _ = r.accept()
+                    conn, _ = ready.accept()
                     rlist.append(conn)
                 else:
                     # Client socket
-                    data = r.recv(1024)
+                    data = ready.recv(1024)
                     if not data or data.startswith('OPEN'):
-                        if r in cnxn_sent:
-                            del cnxn_sent[r]
-                        r.shutdown(socket.SHUT_RDWR)
-                        r.close()
-                        rlist.remove(r)
+                        if ready in cnxn_sent:
+                            del cnxn_sent[ready]
+                        ready.shutdown(socket.SHUT_RDWR)
+                        ready.close()
+                        rlist.remove(ready)
                         continue
-                    if r in cnxn_sent:
+                    if ready in cnxn_sent:
                         continue
-                    cnxn_sent[r] = True
-                    r.sendall(_adb_packet('CNXN', 0x01000001, 1024 * 1024,
-                                          'device::ro.product.name=fakeadb'))
+                    cnxn_sent[ready] = True
+                    ready.sendall(_adb_packet('CNXN', 0x01000001, 1024 * 1024,
+                                              'device::ro.product.name=fakeadb'))
 
     port = serversock.getsockname()[1]
     server_thread = threading.Thread(target=_handle)
@@ -113,13 +114,13 @@
         yield
     finally:
         # Perform best-effort disconnection. Discard the output.
-        p = subprocess.Popen(['adb', 'disconnect', serial],
-                             stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-        p.communicate()
+        subprocess.Popen(['adb', 'disconnect', serial],
+                         stdout=subprocess.PIPE,
+                         stderr=subprocess.PIPE).communicate()
 
 
-class NonApiTest(unittest.TestCase):
-    """Tests for ADB that aren't a part of the AndroidDevice API."""
+class CommandlineTest(unittest.TestCase):
+    """Tests for the ADB commandline."""
 
     def test_help(self):
         """Make sure we get _something_ out of help."""
@@ -141,28 +142,37 @@
                 revision_line, r'^Revision [0-9a-f]{12}-android$')
 
     def test_tcpip_error_messages(self):
-        p = subprocess.Popen(['adb', 'tcpip'], stdout=subprocess.PIPE,
-                             stderr=subprocess.STDOUT)
-        out, _ = p.communicate()
-        self.assertEqual(1, p.returncode)
+        """Make sure 'adb tcpip' parsing is sane."""
+        proc = subprocess.Popen(['adb', 'tcpip'], stdout=subprocess.PIPE,
+                                stderr=subprocess.STDOUT)
+        out, _ = proc.communicate()
+        self.assertEqual(1, proc.returncode)
         self.assertIn('requires an argument', out)
 
-        p = subprocess.Popen(['adb', 'tcpip', 'foo'], stdout=subprocess.PIPE,
-                             stderr=subprocess.STDOUT)
-        out, _ = p.communicate()
-        self.assertEqual(1, p.returncode)
+        proc = subprocess.Popen(['adb', 'tcpip', 'foo'], stdout=subprocess.PIPE,
+                                stderr=subprocess.STDOUT)
+        out, _ = proc.communicate()
+        self.assertEqual(1, proc.returncode)
         self.assertIn('invalid port', out)
 
-    # Helper method that reads a pipe until it is closed, then sets the event.
-    def _read_pipe_and_set_event(self, pipe, event):
-        x = pipe.read()
+
+class ServerTest(unittest.TestCase):
+    """Tests for the ADB server."""
+
+    @staticmethod
+    def _read_pipe_and_set_event(pipe, event):
+        """Reads a pipe until it is closed, then sets the event."""
+        pipe.read()
         event.set()
 
-    # Test that launch_server() does not let the adb server inherit
-    # stdin/stdout/stderr handles which can cause callers of adb.exe to hang.
-    # This test also runs fine on unix even though the impetus is an issue
-    # unique to Windows.
     def test_handle_inheritance(self):
+        """Test that launch_server() does not inherit handles.
+
+        launch_server() should not let the adb server inherit
+        stdin/stdout/stderr handles, which can cause callers of adb.exe to hang.
+        This test also runs fine on unix even though the impetus is an issue
+        unique to Windows.
+        """
         # This test takes 5 seconds to run on Windows: if there is no adb server
         # running on the the port used below, adb kill-server tries to make a
         # TCP connection to a closed port and that takes 1 second on Windows;
@@ -184,29 +194,30 @@
 
         try:
             # Run the adb client and have it start the adb server.
-            p = subprocess.Popen(['adb', '-P', str(port), 'start-server'],
-                                 stdin=subprocess.PIPE, stdout=subprocess.PIPE,
-                                 stderr=subprocess.PIPE)
+            proc = subprocess.Popen(['adb', '-P', str(port), 'start-server'],
+                                    stdin=subprocess.PIPE,
+                                    stdout=subprocess.PIPE,
+                                    stderr=subprocess.PIPE)
 
             # Start threads that set events when stdout/stderr are closed.
             stdout_event = threading.Event()
             stdout_thread = threading.Thread(
-                    target=self._read_pipe_and_set_event,
-                    args=(p.stdout, stdout_event))
+                target=ServerTest._read_pipe_and_set_event,
+                args=(proc.stdout, stdout_event))
             stdout_thread.daemon = True
             stdout_thread.start()
 
             stderr_event = threading.Event()
             stderr_thread = threading.Thread(
-                    target=self._read_pipe_and_set_event,
-                    args=(p.stderr, stderr_event))
+                target=ServerTest._read_pipe_and_set_event,
+                args=(proc.stderr, stderr_event))
             stderr_thread.daemon = True
             stderr_thread.start()
 
             # Wait for the adb client to finish. Once that has occurred, if
             # stdin/stderr/stdout are still open, it must be open in the adb
             # server.
-            p.wait()
+            proc.wait()
 
             # Try to write to stdin which we expect is closed. If it isn't
             # closed, we should get an IOError. If we don't get an IOError,
@@ -214,7 +225,7 @@
             # probably letting the adb server inherit stdin which would be
             # wrong.
             with self.assertRaises(IOError):
-                p.stdin.write('x')
+                proc.stdin.write('x')
 
             # Wait a few seconds for stdout/stderr to be closed (in the success
             # case, this won't wait at all). If there is a timeout, that means
@@ -228,8 +239,12 @@
             subprocess.check_output(['adb', '-P', str(port), 'kill-server'],
                                     stderr=subprocess.STDOUT)
 
-    # Use SO_LINGER to cause TCP RST segment to be sent on socket close.
+
+class EmulatorTest(unittest.TestCase):
+    """Tests for the emulator connection."""
+
     def _reset_socket_on_close(self, sock):
+        """Use SO_LINGER to cause TCP RST segment to be sent on socket close."""
         # The linger structure is two shorts on Windows, but two ints on Unix.
         linger_format = 'hh' if os.name == 'nt' else 'ii'
         l_onoff = 1
@@ -248,7 +263,7 @@
         Bug: https://code.google.com/p/android/issues/detail?id=21021
         """
         with contextlib.closing(
-                socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as listener:
+            socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as listener:
             # Use SO_REUSEADDR so subsequent runs of the test can grab the port
             # even if it is in TIME_WAIT.
             listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
@@ -258,7 +273,7 @@
 
             # Now that listening has started, start adb emu kill, telling it to
             # connect to our mock emulator.
-            p = subprocess.Popen(
+            proc = subprocess.Popen(
                 ['adb', '-s', 'emulator-' + str(port), 'emu', 'kill'],
                 stderr=subprocess.STDOUT)
 
@@ -267,12 +282,16 @@
                 # If WSAECONNABORTED (10053) is raised by any socket calls,
                 # then adb probably isn't reading the data that we sent it.
                 conn.sendall('Android Console: type \'help\' for a list ' +
-                                'of commands\r\n')
+                             'of commands\r\n')
                 conn.sendall('OK\r\n')
 
-                with contextlib.closing(conn.makefile()) as f:
-                    self.assertEqual('kill\n', f.readline())
-                    self.assertEqual('quit\n', f.readline())
+                with contextlib.closing(conn.makefile()) as connf:
+                    line = connf.readline()
+                    if line.startswith('auth'):
+                        # Ignore the first auth line.
+                        line = connf.readline()
+                    self.assertEqual('kill\n', line)
+                    self.assertEqual('quit\n', connf.readline())
 
                 conn.sendall('OK: killing emulator, bye bye\r\n')
 
@@ -285,11 +304,15 @@
                 self._reset_socket_on_close(conn)
 
             # Wait for adb to finish, so we can check return code.
-            p.communicate()
+            proc.communicate()
 
             # If this fails, adb probably isn't ignoring WSAECONNRESET when
             # reading the response from the adb emu kill command (on Windows).
-            self.assertEqual(0, p.returncode)
+            self.assertEqual(0, proc.returncode)
+
+
+class ConnectionTest(unittest.TestCase):
+    """Tests for adb connect."""
 
     def test_connect_ipv4_ipv6(self):
         """Ensure that `adb connect localhost:1234` will try both IPv4 and IPv6.
@@ -298,7 +321,7 @@
         """
         for protocol in (socket.AF_INET, socket.AF_INET6):
             try:
-                with fake_adb_server(protocol=protocol) as port:
+                with fake_adbd(protocol=protocol) as port:
                     serial = 'localhost:{}'.format(port)
                     with adb_connect(self, serial):
                         pass
@@ -309,7 +332,7 @@
     def test_already_connected(self):
         """Ensure that an already-connected device stays connected."""
 
-        with fake_adb_server() as port:
+        with fake_adbd() as port:
             serial = 'localhost:{}'.format(port)
             with adb_connect(self, serial):
                 # b/31250450: this always returns 0 but probably shouldn't.
@@ -320,7 +343,7 @@
     def test_reconnect(self):
         """Ensure that a disconnected device reconnects."""
 
-        with fake_adb_server() as port:
+        with fake_adbd() as port:
             serial = 'localhost:{}'.format(port)
             with adb_connect(self, serial):
                 output = subprocess.check_output(['adb', '-s', serial,
@@ -328,10 +351,10 @@
                 self.assertEqual(output.strip(), 'device')
 
                 # This will fail.
-                p = subprocess.Popen(['adb', '-s', serial, 'shell', 'true'],
-                                     stdout=subprocess.PIPE,
-                                     stderr=subprocess.STDOUT)
-                output, _ = p.communicate()
+                proc = subprocess.Popen(['adb', '-s', serial, 'shell', 'true'],
+                                        stdout=subprocess.PIPE,
+                                        stderr=subprocess.STDOUT)
+                output, _ = proc.communicate()
                 self.assertEqual(output.strip(), 'error: closed')
 
                 subprocess.check_call(['adb', '-s', serial, 'wait-for-device'])
@@ -349,18 +372,16 @@
                     subprocess.check_output(['adb', '-s', serial, 'get-state'],
                                             stderr=subprocess.STDOUT)
                     self.fail('Device should not be available')
-                except subprocess.CalledProcessError as e:
+                except subprocess.CalledProcessError as err:
                     self.assertEqual(
-                        e.output.strip(),
+                        err.output.strip(),
                         'error: device \'{}\' not found'.format(serial))
 
+
 def main():
+    """Main entrypoint."""
     random.seed(0)
-    if len(adb.get_devices()) > 0:
-        suite = unittest.TestLoader().loadTestsFromName(__name__)
-        unittest.TextTestRunner(verbosity=3).run(suite)
-    else:
-        print('Test suite must be run with attached devices')
+    unittest.main(verbosity=3)
 
 
 if __name__ == '__main__':