Improved message on config file read error

Previously, "Error: could not find config file ???" was
printed when a config file could not be opened. This also includes when
the file had the wrong read permissions. Thus, an incorrect error
description was given (since the file actually existed).

This commit corrects this behaviour. This is done by introducing the new
error message "Error: could not read config file ???". This message is
now presented when the config file exists, but can't be read. The
original message is only presented when the config file actually does
not exist.

Additional tests are committed that ensures correct behaviour.

This commit fixes issue #61.

supervisor/options.py

@@ -483,10 +483,12 @@ class ServerOptions(Options):
     def read_config(self, fp):
         section = self.configroot.supervisord
         if not hasattr(fp, 'read'):
+            if not os.path.exists(fp):
+                raise ValueError("could not find config file %s" % fp)
             try:
                 fp = open(fp, 'r')
             except (IOError, OSError):
-                raise ValueError("could not find config file %s" % fp)
+                raise ValueError("could not read config file %s" % fp)
         parser = UnhosedConfigParser()
         try:
             parser.readfp(fp)
@@ -1427,10 +1429,12 @@ class ClientOptions(Options):
         section = self.configroot.supervisorctl
         if not hasattr(fp, 'read'):
             self.here = os.path.dirname(normalize_path(fp))
+            if not os.path.exists(fp):
+                raise ValueError("could not find config file %s" % fp)
             try:
                 fp = open(fp, 'r')
             except (IOError, OSError):
-                raise ValueError("could not find config file %s" % fp)
+                raise ValueError("could not read config file %s" % fp)
         config = UnhosedConfigParser()
         config.mysection = 'supervisorctl'
         config.readfp(fp)

supervisor/tests/test_options.py

@@ -142,6 +142,45 @@ class ClientOptionsTests(unittest.TestCase):
         self.assertEqual(options.password, '123')
         self.assertEqual(options.history_file, history_file)
 
+    def test_unreadable_config_file(self):
+        # Quick and dirty way of coming up with a decent filename
+        tempf = tempfile.NamedTemporaryFile()
+        fname = tempf.name
+        tempf.close()
+        self.assertFalse(os.path.exists(fname))
+
+        instance = self._makeOne()
+        class DummyException(Exception):
+            def __init__(self, exitcode):
+                self.exitcode = exitcode
+        def dummy_exit(self, exitcode=2):
+            # Important default exitcode=2 like sys.exit.
+            raise DummyException(exitcode)
+        instance.exit = dummy_exit
+        try:
+            instance.realize(args=['-c', fname])
+        except DummyException, e:
+            self.assertEquals(e.exitcode, 2)
+        else:
+            self.fail("expected exception")
+
+        try:
+            instance.read_config(fname)
+        except ValueError, e:
+            self.assertTrue("could not find config file" in str(e))
+        else:
+            self.fail("expected exception")
+
+        tempf = tempfile.NamedTemporaryFile()
+        os.chmod(tempf.name, 0) # Removing read perms
+        try:
+            instance.read_config(tempf.name)
+        except ValueError, e:
+            self.assertTrue("could not read config file" in str(e))
+        else:
+            self.fail("expected exception")
+        tempf.close()
+
     def test_options_unixsocket_cli(self):
         from StringIO import StringIO
         fp = StringIO('[supervisorctl]')
@@ -457,6 +496,45 @@ class ServerOptionsTests(unittest.TestCase):
         self.assertEqual(proc.name, 'three')
         self.assertEqual(proc.command, '/bin/pig')
 
+    def test_unreadable_config_file(self):
+        # Quick and dirty way of coming up with a decent filename
+        tempf = tempfile.NamedTemporaryFile()
+        fname = tempf.name
+        tempf.close()
+        self.assertFalse(os.path.exists(fname))
+
+        instance = self._makeOne()
+        class DummyException(Exception):
+            def __init__(self, exitcode):
+                self.exitcode = exitcode
+        def dummy_exit(self, exitcode=2):
+            # Important default exitcode=2 like sys.exit.
+            raise DummyException(exitcode)
+        instance.exit = dummy_exit
+        try:
+            instance.realize(args=['-c', fname])
+        except DummyException, e:
+            self.assertEquals(e.exitcode, 2)
+        else:
+            self.fail("expected exception")
+
+        try:
+            instance.read_config(fname)
+        except ValueError, e:
+            self.assertTrue("could not find config file" in str(e))
+        else:
+            self.fail("expected exception")
+
+        tempf = tempfile.NamedTemporaryFile()
+        os.chmod(tempf.name, 0) # Removing read perms
+        try:
+            instance.read_config(tempf.name)
+        except ValueError, e:
+            self.assertTrue("could not read config file" in str(e))
+        else:
+            self.fail("expected exception")
+        tempf.close()
+
     def test_readFile_failed(self):
         from supervisor.options import readFile
         try: