Pārlūkot izejas kodu

- Running the tests now requires the "mock" package. This dependency has
been added to "tests_require" in setup.py. (Roger Hoover)

- Added support for setting the ownership and permissions for an FCGI socket.
This is done using new "socket_owner" and "socket_mode" options in an
[fcgi-program:x] section. See the manual for details. (Roger Hoover)

- Fixed a bug where the FCGI socket reference count was not getting
decremented on spawn error. (Roger Hoover)

Mike Naberezny 15 gadi atpakaļ
vecāks
revīzija
7476809cbd

+ 10 - 0
CHANGES.txt

@@ -29,6 +29,16 @@ Next Release
   - Fixed a bug where the --serverurl option of supervisorctl would not
     accept a URL with a "unix" scheme.  (Jason Kirtland)
 
+  - Running the tests now requires the "mock" package.  This dependency has 
+    been added to "tests_require" in setup.py.  (Roger Hoover)
+
+  - Added support for setting the ownership and permissions for an FCGI socket.
+    This is done using new "socket_owner" and "socket_mode" options in an
+    [fcgi-program:x] section.  See the manual for details.  (Roger Hoover)
+    
+  - Fixed a bug where the FCGI socket reference count was not getting 
+    decremented on spawn error.  (Roger Hoover)
+
 3.0a7 (2009-05-24)
  
   - We now bundle our own patched version of Medusa contributed by Jason

+ 1 - 1
setup.py

@@ -87,7 +87,7 @@ dist = setup(
     )],
     install_requires = requires,
     extras_require = {'iterparse':['cElementTree >= 1.0.2']},
-    tests_require = requires,
+    tests_require = requires + ['mock >= 0.5.0'],
     include_package_data = True,
     zip_safe = False,
     namespace_packages = ['supervisor'],

+ 41 - 4
src/supervisor/datatypes.py

@@ -202,7 +202,7 @@ class SocketConfig:
     def addr(self): # pragma: no cover
         raise NotImplementedError
         
-    def create(self): # pragma: no cover
+    def create_and_bind(self): # pragma: no cover
         raise NotImplementedError
 
 class InetStreamSocketConfig(SocketConfig):
@@ -219,28 +219,65 @@ class InetStreamSocketConfig(SocketConfig):
     def addr(self):
         return (self.host, self.port)
         
-    def create(self):
+    def create_and_bind(self):
         sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+        sock.bind(self.addr())
         return sock
         
 class UnixStreamSocketConfig(SocketConfig):
     """ Unix domain socket config helper """
 
     path = None # Unix domain socket path
+    mode = None # Unix permission mode bits for socket
+    owner = None # Tuple (uid, gid) for Unix ownership of socket
+    sock = None # socket object
     
-    def __init__(self, path):
+    def __init__(self, path, **kwargs):
         self.path = path
         self.url = 'unix://%s' % (path)
+        self.mode = kwargs.get('mode', None)
+        self.owner = kwargs.get('owner', None)
         
     def addr(self):
         return self.path
         
-    def create(self):
+    def create_and_bind(self):
         if os.path.exists(self.path):
             os.unlink(self.path)
         sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+        sock.bind(self.addr())
+        try:
+            self._chown()
+            self._chmod()
+        except:
+            sock.close()
+            os.unlink(self.path)
+            raise
         return sock
+        
+    def get_mode(self):
+        return self.mode
+        
+    def get_owner(self):
+        return self.owner
+        
+    def _chmod(self):
+        if self.mode is not None:
+            try:
+                os.chmod(self.path, self.mode)
+            except Exception, e:
+                raise ValueError("Could not change permissions of socket "
+                                    + "file: %s" % (e))
+        
+    def _chown(self):
+        if self.owner is not None:
+            try:
+                os.chown(self.path, self.owner[0], self.owner[1])
+            except Exception, e:
+                raise ValueError("Could not change ownership of socket file: "
+                                    + "%s" % (e))
+                
 
 def colon_separated_user_group(arg):
     try:

+ 42 - 3
src/supervisor/options.py

@@ -645,6 +645,25 @@ class ServerOptions(Options):
                 continue
             program_name = section.split(':', 1)[1]
             priority = integer(get(section, 'priority', 999))
+            
+            proc_uid = name_to_uid(get(section, 'user', None))
+            
+            socket_owner = get(section, 'socket_owner', None)
+            if socket_owner is not None:
+                try:
+                    socket_owner = colon_separated_user_group(socket_owner)
+                except ValueError:
+                    raise ValueError('Invalid socket_owner value %s'
+                                                                % socket_owner)                
+                
+            socket_mode = get(section, 'socket_mode', None)
+            if socket_mode is not None:
+                try:
+                    socket_mode = octal_type(socket_mode)
+                except (TypeError, ValueError):
+                    raise ValueError('Invalid socket_mode value %s'
+                                                                % socket_mode)
+            
             socket = get(section, 'socket', None)
             if not socket:
                 raise ValueError('[%s] section requires a "socket" line' %
@@ -654,7 +673,8 @@ class ServerOptions(Options):
                           'program_name':program_name}
             socket = expand(socket, expansions, 'socket')
             try:
-                socket_config = self.parse_fcgi_socket(socket)
+                socket_config = self.parse_fcgi_socket(socket, proc_uid,
+                                                    socket_owner, socket_mode)
             except ValueError, e:
                 raise ValueError('%s in [%s] socket' % (str(e), section))
             
@@ -669,7 +689,7 @@ class ServerOptions(Options):
         groups.sort()
         return groups
 
-    def parse_fcgi_socket(self, sock):
+    def parse_fcgi_socket(self, sock, proc_uid, socket_owner, socket_mode):
         if sock.startswith('unix://'):
             path = sock[7:]
             #Check it's an absolute path
@@ -677,7 +697,21 @@ class ServerOptions(Options):
                 raise ValueError("Unix socket path %s is not an absolute path",
                                  path)
             path = normalize_path(path)
-            return UnixStreamSocketConfig(path)
+            
+            if socket_owner is None:
+                uid = os.getuid()
+                if proc_uid is not None and proc_uid != uid:
+                    socket_owner = (proc_uid, self.get_gid_for_uid(proc_uid))
+                    
+            if socket_mode is None:
+                socket_mode = 0700
+            
+            return UnixStreamSocketConfig(path, owner=socket_owner,
+                                                mode=socket_mode)
+        
+        if socket_owner is not None or socket_mode is not None:
+            raise ValueError("socket_owner and socket_mode params should"
+                    + " only be used with a Unix domain socket")
         
         m = re.match(r'tcp://([^\s:]+):(\d+)$', sock)
         if m:
@@ -686,6 +720,11 @@ class ServerOptions(Options):
             return InetStreamSocketConfig(host, port)
         
         raise ValueError("Bad socket format %s", sock)
+        
+    def get_gid_for_uid(self, uid):
+        import pwd
+        pwrec = pwd.getpwuid(uid)
+        return pwrec[3]
 
     def processes_from_section(self, parser, section, group_name,
                                klass=None):

+ 7 - 2
src/supervisor/process.py

@@ -579,7 +579,11 @@ class FastCGISubprocess(Subprocess):
         Overrides Subprocess.spawn() so we can hook in before it happens
         """
         self.before_spawn()
-        Subprocess.spawn(self)
+        pid = Subprocess.spawn(self)
+        if pid is None:
+            #Remove object reference to decrement the reference count on error
+            self.fcgi_sock = None
+        return pid
         
     def after_finish(self):
         """
@@ -592,8 +596,9 @@ class FastCGISubprocess(Subprocess):
         """
         Overrides Subprocess.finish() so we can hook in after it happens
         """
-        Subprocess.finish(self, pid, sts)
+        retval = Subprocess.finish(self, pid, sts)
         self.after_finish()
+        return retval
 
     def _prepare_child_fds(self):
         """

+ 1 - 2
src/supervisor/socket_manager.py

@@ -107,8 +107,7 @@ class SocketManager:
         if not self.prepared:
             if self.logger:
                 self.logger.info('Creating socket %s' % self.socket_config)
-            self.socket = self.socket_config.create()
-            self.socket.bind(self.socket_config.addr())
+            self.socket = self.socket_config.create_and_bind()
             self.socket.listen(socket.SOMAXCONN)
             self.prepared = True
     

+ 1 - 1
src/supervisor/tests/base.py

@@ -333,7 +333,7 @@ class DummySocketConfig:
     def __ne__(self, other):
         return not self.__eq__(other)
 
-    def create(self):
+    def create_and_bind(self):
         return DummySocket(self.fd)
 
 class DummySocketManager:

+ 58 - 86
src/supervisor/tests/test_datatypes.py

@@ -5,6 +5,7 @@ import os
 import unittest
 import socket
 import tempfile
+from mock import Mock, patch, sentinel
 from supervisor import datatypes
 
 class DatatypesTest(unittest.TestCase):
@@ -176,7 +177,7 @@ class DatatypesTest(unittest.TestCase):
     def test_url_rejects_urlparse_unrecognized_scheme_with_path(self):
         bad_url = "bad://path"
         self.assertRaises(ValueError, datatypes.url, bad_url)
- 
+
 class InetStreamSocketConfigTests(unittest.TestCase):
     def _getTargetClass(self):
         return datatypes.InetStreamSocketConfig
@@ -187,6 +188,10 @@ class InetStreamSocketConfigTests(unittest.TestCase):
     def test_url(self):
         conf = self._makeOne('127.0.0.1', 8675)
         self.assertEqual(conf.url, 'tcp://127.0.0.1:8675')
+
+    def test___str__(self):
+        cfg = self._makeOne('localhost', 65531)
+        self.assertEqual(str(cfg), 'tcp://localhost:65531')
                 
     def test_repr(self):
         conf = self._makeOne('127.0.0.1', 8675)
@@ -205,32 +210,32 @@ class InetStreamSocketConfigTests(unittest.TestCase):
         addr = conf.addr()
         self.assertEqual(addr, ('localhost', 5001))
         
-    def test_create(self):
+    def test_create_and_bind(self):
         conf = self._makeOne('127.0.0.1', 8675)
-        sock = conf.create()
+        sock = conf.create_and_bind()
         reuse = sock.getsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR)
         self.assertTrue(reuse)
-        sock.close
+        self.assertEquals(conf.addr(), sock.getsockname()) #verifies that bind was called
+        sock.close()
         
     def test_same_urls_are_equal(self):
-        conf1 = self._makeOne('localhost', '5001')
-        conf2 = self._makeOne('localhost', '5001')
+        conf1 = self._makeOne('localhost', 5001)
+        conf2 = self._makeOne('localhost', 5001)
         self.assertTrue(conf1 == conf2)
         self.assertFalse(conf1 != conf2)
 
     def test_diff_urls_are_not_equal(self):
-        conf1 = self._makeOne('localhost', '5001')
-        conf2 = self._makeOne('localhost', '5002')
+        conf1 = self._makeOne('localhost', 5001)
+        conf2 = self._makeOne('localhost', 5002)
         self.assertTrue(conf1 != conf2)
         self.assertFalse(conf1 == conf2)
 
     def test_diff_objs_are_not_equal(self):
-        conf1 = self._makeOne('localhost', '5001')
+        conf1 = self._makeOne('localhost', 5001)
         conf2 = 'blah'
         self.assertTrue(conf1 != conf2)
-        self.assertFalse(conf1 == conf2)    
-        
-        
+        self.assertFalse(conf1 == conf2) 
+              
 class UnixStreamSocketConfigTests(unittest.TestCase):
     def _getTargetClass(self):
         return datatypes.UnixStreamSocketConfig
@@ -241,6 +246,10 @@ class UnixStreamSocketConfigTests(unittest.TestCase):
     def test_url(self):
         conf = self._makeOne('/tmp/foo.sock')
         self.assertEqual(conf.url, 'unix:///tmp/foo.sock')
+        
+    def test___str__(self):
+        cfg = self._makeOne('foo/bar')
+        self.assertEqual(str(cfg), 'unix://foo/bar')
             
     def test_repr(self):
         conf = self._makeOne('/tmp/foo.sock')
@@ -254,13 +263,42 @@ class UnixStreamSocketConfigTests(unittest.TestCase):
         addr = conf.addr()
         self.assertEqual(addr, '/tmp/foo.sock')
         
-    def test_create(self):
+    def test_create_and_bind(self):
         (tf_fd, tf_name) = tempfile.mkstemp()
-        conf = self._makeOne(tf_name)
-        os.close(tf_fd)
-        sock = conf.create()
-        self.assertFalse(os.path.exists(tf_name))
-        sock.close
+        owner = (sentinel.uid, sentinel.gid)
+        mode = sentinel.mode
+        conf = self._makeOne(tf_name, owner=owner, mode=mode)
+        
+        #Patch os.chmod and os.chown functions with mocks
+        #objects so that the test does not depend on
+        #any specific system users or permissions
+        chown_mock = Mock()
+        chmod_mock = Mock()
+        @patch('os.chown', chown_mock)
+        @patch('os.chmod', chmod_mock)
+        def call_create_and_bind(conf):
+            return conf.create_and_bind()
+        
+        sock = call_create_and_bind(conf)
+        self.assertTrue(os.path.exists(tf_name))
+        self.assertEquals(conf.addr(), sock.getsockname()) #verifies that bind was called
+        sock.close()
+        self.assertTrue(os.path.exists(tf_name))
+        os.unlink(tf_name)
+        #Verify that os.chown was called with correct args
+        self.assertEquals(1, chown_mock.call_count)
+        path_arg = chown_mock.call_args[0][0]
+        uid_arg = chown_mock.call_args[0][1]
+        gid_arg = chown_mock.call_args[0][2]
+        self.assertEquals(tf_name, path_arg)
+        self.assertEquals(owner[0], uid_arg)
+        self.assertEquals(owner[1], gid_arg)
+        #Verify that os.chmod was called with correct args
+        self.assertEquals(1, chmod_mock.call_count)
+        path_arg = chmod_mock.call_args[0][0]
+        mode_arg = chmod_mock.call_args[0][1]
+        self.assertEquals(tf_name, path_arg)
+        self.assertEquals(mode, mode_arg)
         
     def test_same_paths_are_equal(self):
         conf1 = self._makeOne('/tmp/foo.sock')
@@ -278,8 +316,8 @@ class UnixStreamSocketConfigTests(unittest.TestCase):
         conf1 = self._makeOne('/tmp/foo.sock')
         conf2 = 'blah'
         self.assertTrue(conf1 != conf2)
-        self.assertFalse(conf1 == conf2)    
-
+        self.assertFalse(conf1 == conf2) 
+        
 class RangeCheckedConversionTests(unittest.TestCase):
     def _getTargetClass(self):
         from supervisor.datatypes import RangeCheckedConversion
@@ -341,72 +379,6 @@ class TestSocketAddress(unittest.TestCase):
         self.assertEqual(addr.family, socket.AF_INET)
         self.assertEqual(addr.address, ('localhost', 8080))
         
-class TestInetStreamSocketConfig(unittest.TestCase):
-    def _getTargetClass(self):
-        from supervisor.datatypes import InetStreamSocketConfig
-        return InetStreamSocketConfig
-        
-    def _makeOne(self, host, port):
-        return self._getTargetClass()(host, port)
-
-    def test_addr(self):
-        cfg = self._makeOne('localhost', 8080)
-        self.assertEqual(cfg.addr(), ('localhost', 8080))
-        
-    def test_create(self):
-        cfg = self._makeOne('localhost', 65531)
-        sock = cfg.create()
-        sock.close()
-
-    def test___str__(self):
-        cfg = self._makeOne('localhost', 65531)
-        self.assertEqual(str(cfg), 'tcp://localhost:65531')
-
-    def test__eq__(self):
-        cfg = self._makeOne('localhost', 65531)
-        cfg2 = self._makeOne('localhost', 65531)
-        self.failUnless(cfg == cfg2)
-        
-    def test__ne__(self):
-        cfg = self._makeOne('localhost', 65531)
-        cfg2 = self._makeOne('localhost', 65532)
-        self.failUnless(cfg != cfg2)
-        
-class TestUnixStreamSocketConfig(unittest.TestCase):
-    def _getTargetClass(self):
-        from supervisor.datatypes import UnixStreamSocketConfig
-        return UnixStreamSocketConfig
-        
-    def _makeOne(self, path):
-        return self._getTargetClass()(path)
-
-    def test_addr(self):
-        cfg = self._makeOne('/foo/bar')
-        self.assertEqual(cfg.addr(), '/foo/bar')
-        
-    def test_create(self):
-        import tempfile
-        fn = tempfile.mktemp()
-        cfg = self._makeOne(fn)
-        sock = cfg.create()
-        sock.close()
-        if os.path.exists(fn):
-            os.unlink(fn)
-
-    def test___str__(self):
-        cfg = self._makeOne('foo/bar')
-        self.assertEqual(str(cfg), 'unix://foo/bar')
-
-    def test__eq__(self):
-        cfg = self._makeOne('/foo/bar')
-        cfg2 = self._makeOne('/foo/bar')
-        self.failUnless(cfg == cfg2)
-        
-    def test__ne__(self):
-        cfg = self._makeOne('/foo/bar')
-        cfg2 = self._makeOne('/foo/bar2')
-        self.failUnless(cfg != cfg2)
-        
 class TestColonSeparatedUserGroup(unittest.TestCase):
     def _callFUT(self, arg):
         from supervisor.datatypes import colon_separated_user_group

+ 119 - 22
src/supervisor/tests/test_options.py

@@ -8,6 +8,7 @@ import unittest
 import signal
 import shutil
 import errno
+from mock import Mock, patch, sentinel
 
 from supervisor.tests.base import DummySupervisor
 from supervisor.tests.base import DummyLogger
@@ -784,49 +785,93 @@ class ServerOptionsTests(unittest.TestCase):
         config.read_string(text)
         instance = self._makeOne()
         self.assertRaises(ValueError,instance.process_groups_from_parser,config)
-
+    
     def test_fcgi_programs_from_parser(self):
         from supervisor.options import FastCGIGroupConfig
         from supervisor.options import FastCGIProcessConfig
         text = lstrip("""\
         [fcgi-program:foo]
-        socket=unix:///tmp/%(program_name)s.sock
+        socket = unix:///tmp/%(program_name)s.sock
+        socket_owner = testuser:testgroup
+        socket_mode = 0666
         process_name = %(program_name)s_%(process_num)s
         command = /bin/foo
         numprocs = 2
         priority = 1
 
         [fcgi-program:bar]
-        socket=tcp://localhost:6000
+        socket = unix:///tmp/%(program_name)s.sock
         process_name = %(program_name)s_%(process_num)s
         command = /bin/bar
+        user = testuser
         numprocs = 3
+        
+        [fcgi-program:flub]
+        socket = unix:///tmp/%(program_name)s.sock
+        command = /bin/flub
+        
+        [fcgi-program:cub]
+        socket = tcp://localhost:6000
+        command = /bin/cub
         """)
         from supervisor.options import UnhosedConfigParser
         config = UnhosedConfigParser()
         config.read_string(text)
         instance = self._makeOne()
-        gconfigs = instance.process_groups_from_parser(config)
-        self.assertEqual(len(gconfigs), 2)
-
-        gconfig0 = gconfigs[0]
-        self.assertEqual(gconfig0.__class__, FastCGIGroupConfig)
-        self.assertEqual(gconfig0.name, 'foo')
-        self.assertEqual(gconfig0.priority, 1)
-        self.assertEqual(gconfig0.socket_config.url,
-                         'unix:///tmp/foo.sock')
-        self.assertEqual(len(gconfig0.process_configs), 2)
-        self.assertEqual(gconfig0.process_configs[0].__class__,
-                         FastCGIProcessConfig)
-        self.assertEqual(gconfig0.process_configs[1].__class__,
-                         FastCGIProcessConfig)
         
-        gconfig1 = gconfigs[1]
-        self.assertEqual(gconfig1.name, 'bar')
-        self.assertEqual(gconfig1.priority, 999)
-        self.assertEqual(gconfig1.socket_config.url,
+        #Patch pwd and grp module functions to give us sentinel
+        #uid/gid values so that the test does not depend on
+        #any specific system users
+        pwd_mock = Mock()
+        pwd_mock.return_value = (None, None, sentinel.uid, sentinel.gid)
+        grp_mock = Mock()
+        grp_mock.return_value = (None, None, sentinel.gid)
+        @patch('pwd.getpwuid', pwd_mock)
+        @patch('pwd.getpwnam', pwd_mock)
+        @patch('grp.getgrnam', grp_mock)
+        def get_process_groups(instance, config):
+            return instance.process_groups_from_parser(config)
+
+        gconfigs = get_process_groups(instance, config)
+        exp_owner = (sentinel.uid, sentinel.gid)
+
+        self.assertEqual(len(gconfigs), 4)
+
+        gconf_foo = gconfigs[0]
+        self.assertEqual(gconf_foo.__class__, FastCGIGroupConfig)
+        self.assertEqual(gconf_foo.name, 'foo')
+        self.assertEqual(gconf_foo.priority, 1)
+        self.assertEqual(gconf_foo.socket_config.url,
+                                'unix:///tmp/foo.sock')
+        self.assertEqual(exp_owner, gconf_foo.socket_config.get_owner())
+        self.assertEqual(0666, gconf_foo.socket_config.get_mode())
+        self.assertEqual(len(gconf_foo.process_configs), 2)
+        pconfig_foo = gconf_foo.process_configs[0]
+        self.assertEqual(pconfig_foo.__class__, FastCGIProcessConfig)
+        
+        gconf_bar = gconfigs[2]
+        self.assertEqual(gconf_bar.name, 'bar')
+        self.assertEqual(gconf_bar.priority, 999)
+        self.assertEqual(gconf_bar.socket_config.url,
+                         'unix:///tmp/bar.sock')
+        self.assertEqual(exp_owner, gconf_bar.socket_config.get_owner())
+        self.assertEqual(0700, gconf_bar.socket_config.get_mode())
+        self.assertEqual(len(gconf_bar.process_configs), 3)
+        
+        gconf_flub = gconfigs[1]
+        self.assertEqual(gconf_flub.name, 'flub')
+        self.assertEqual(gconf_flub.socket_config.url,
+                         'unix:///tmp/flub.sock')
+        self.assertEqual(None, gconf_flub.socket_config.get_owner())
+        self.assertEqual(0700, gconf_flub.socket_config.get_mode())
+        self.assertEqual(len(gconf_flub.process_configs), 1)
+        
+        gconf_cub = gconfigs[3]
+        self.assertEqual(gconf_cub.name, 'cub')
+        self.assertEqual(gconf_cub.socket_config.url,
                          'tcp://localhost:6000')
-        self.assertEqual(len(gconfig1.process_configs), 3)
+        self.assertEqual(len(gconf_cub.process_configs), 1)
+        
 
     def test_fcgi_program_no_socket(self):
         text = lstrip("""\
@@ -901,6 +946,58 @@ class ServerOptionsTests(unittest.TestCase):
         config.read_string(text)
         instance = self._makeOne()
         self.assertRaises(ValueError,instance.process_groups_from_parser,config)
+
+    def test_fcgi_program_socket_owner_set_for_tcp(self):
+        text = lstrip("""\
+        [fcgi-program:foo]
+        socket=tcp://localhost:8000
+        socket_owner=nobody:nobody
+        command = /bin/foo
+        """)
+        from supervisor.options import UnhosedConfigParser
+        config = UnhosedConfigParser()
+        config.read_string(text)
+        instance = self._makeOne()
+        self.assertRaises(ValueError,instance.process_groups_from_parser,config)
+
+    def test_fcgi_program_socket_mode_set_for_tcp(self):
+        text = lstrip("""\
+        [fcgi-program:foo]
+        socket = tcp://localhost:8000
+        socket_mode = 0777
+        command = /bin/foo
+        """)
+        from supervisor.options import UnhosedConfigParser
+        config = UnhosedConfigParser()
+        config.read_string(text)
+        instance = self._makeOne()
+        self.assertRaises(ValueError,instance.process_groups_from_parser,config)
+
+    def test_fcgi_program_bad_socket_owner(self):
+        text = lstrip("""\
+        [fcgi-program:foo]
+        socket = unix:///tmp/foo.sock
+        socket_owner = sometotaljunkuserthatshouldnobethere
+        command = /bin/foo
+        """)
+        from supervisor.options import UnhosedConfigParser
+        config = UnhosedConfigParser()
+        config.read_string(text)
+        instance = self._makeOne()
+        self.assertRaises(ValueError,instance.process_groups_from_parser,config)
+
+    def test_fcgi_program_bad_socket_mode(self):
+        text = lstrip("""\
+        [fcgi-program:foo]
+        socket = unix:///tmp/foo.sock
+        socket_mode = junk
+        command = /bin/foo
+        """)
+        from supervisor.options import UnhosedConfigParser
+        config = UnhosedConfigParser()
+        config.read_string(text)
+        instance = self._makeOne()
+        self.assertRaises(ValueError,instance.process_groups_from_parser,config)
     
     def test_heterogeneous_process_groups_from_parser(self):
         text = lstrip("""\

+ 57 - 0
src/supervisor/tests/test_process.py

@@ -4,6 +4,7 @@ import time
 import unittest
 import sys
 import errno
+from mock import Mock, patch_object, sentinel
 
 from supervisor.tests.base import DummyOptions
 from supervisor.tests.base import DummyPConfig
@@ -17,6 +18,8 @@ from supervisor.tests.base import DummyProcessGroup
 from supervisor.tests.base import DummyFCGIProcessGroup
 from supervisor.tests.base import DummySocketManager
 
+from supervisor.process import Subprocess
+
 class SubprocessTests(unittest.TestCase):
     def _getTargetClass(self):
         from supervisor.process import Subprocess
@@ -1234,6 +1237,60 @@ class FastCGISubprocessTests(unittest.TestCase):
         instance.fcgi_sock = 'hello'
         instance.after_finish()
         self.assertTrue(instance.fcgi_sock is None)
+    
+    #Patch Subprocess.finish() method for this test to verify override
+    @patch_object(Subprocess, 'finish', Mock(return_value=sentinel.finish_result))
+    def test_finish_override(self):
+        options = DummyOptions()
+        config = DummyPConfig(options, 'good', '/good/filename', uid=1)
+        instance = self._makeOne(config)
+        instance.after_finish = Mock()
+        result = instance.finish(sentinel.pid, sentinel.sts)
+        self.assertEqual(sentinel.finish_result, result,
+                        'FastCGISubprocess.finish() did not pass thru result')
+        self.assertEqual(1, instance.after_finish.call_count,
+                            'FastCGISubprocess.after_finish() not called once')
+        finish_mock = Subprocess.finish
+        self.assertEqual(1, finish_mock.call_count,
+                            'Subprocess.finish() not called once')
+        pid_arg = finish_mock.call_args[0][1]
+        sts_arg = finish_mock.call_args[0][2]
+        self.assertEqual(sentinel.pid, pid_arg,
+                            'Subprocess.finish() pid arg was not passed')
+        self.assertEqual(sentinel.sts, sts_arg,
+                            'Subprocess.finish() sts arg was not passed')
+                            
+    #Patch Subprocess.spawn() method for this test to verify override
+    @patch_object(Subprocess, 'spawn', Mock(return_value=sentinel.ppid))
+    def test_spawn_override_success(self):
+        options = DummyOptions()
+        config = DummyPConfig(options, 'good', '/good/filename', uid=1)
+        instance = self._makeOne(config)
+        instance.before_spawn = Mock()
+        result = instance.spawn()
+        self.assertEqual(sentinel.ppid, result,
+                        'FastCGISubprocess.spawn() did not pass thru result')
+        self.assertEqual(1, instance.before_spawn.call_count,
+                            'FastCGISubprocess.before_spawn() not called once')
+        spawn_mock = Subprocess.spawn
+        self.assertEqual(1, spawn_mock.call_count,
+                            'Subprocess.spawn() not called once')
+
+    #Patch Subprocess.spawn() method for this test to verify error handling
+    @patch_object(Subprocess, 'spawn', Mock(return_value=None))
+    def test_spawn_error(self):
+        options = DummyOptions()
+        config = DummyPConfig(options, 'good', '/good/filename', uid=1)
+        instance = self._makeOne(config)
+        instance.before_spawn = Mock()
+        instance.fcgi_sock = 'nuke me on error'
+        result = instance.spawn()
+        self.assertEqual(None, result,
+                        'FastCGISubprocess.spawn() did return None on error')
+        self.assertEqual(1, instance.before_spawn.call_count,
+                            'FastCGISubprocess.before_spawn() not called once')
+        self.assertEqual(None, instance.fcgi_sock,
+                'FastCGISubprocess.spawn() did not remove sock ref on error')    
 
 class ProcessGroupBaseTests(unittest.TestCase):
     def _getTargetClass(self):

+ 1 - 2
src/supervisor/tests/test_socket_manager.py

@@ -174,8 +174,7 @@ class SocketManagerTest(unittest.TestCase):
         sock_manager = self._makeOne(conf)
         sock = sock_manager.get_socket()
         self.assertTrue(sock_manager.is_prepared())
-        self.assertTrue(sock.bind_called)
-        self.assertEqual(sock.bind_addr, 'dummy addr')
+        self.assertFalse(sock.bind_called)
         self.assertTrue(sock.listen_called)
         self.assertEqual(sock.listen_backlog, socket.SOMAXCONN)
         self.assertFalse(sock.close_called)