Rewrite command-line parsing to use optparse instead of do-it-yourself.

Side effects include:

  * bin/zope-sendmail exits with a non-zero exit code in case of errors (I
    consider the previous behaviour to be a bug)

  * ConsoleApp(argv) raises SystemExit instead of setting self._error

  * ConsoleApp(argv, verbose=False) doesn't suppress error messages on
    sys.stderr any more.

src/zope/sendmail/queue.py

@@ -20,6 +20,7 @@
 import atexit
 import ConfigParser
 import logging
+import optparse
 import os
 import smtplib
 import stat
@@ -341,41 +342,36 @@ def string_or_none(s):
 class ConsoleApp(object):
     """Allows running of Queue Processor from the console."""
 
-    _usage = """%(script_name)s [OPTIONS] path/to/maildir
+    parser = optparse.OptionParser(usage='%prog [OPTIONS] path/to/maildir')
+    parser.add_option('--daemon', action='store_true',
+                      help="Run in daemon mode, periodically checking queue "
+                           "and sending messages.  Default is to send all "
+                           "messages in queue once and exit.")
+    parser.add_option('--interval', metavar='<#secs>', type='float', default=3,
+                      help="How often to check queue when in daemon mode. "
+                           "Default is %default seconds.")
+    parser.add_option('--hostname', default='localhost',
+                      help="Name of smtp host to use for delivery.  Default is "
+                           "%default.")
+    parser.add_option('--port', type='int', default=25,
+                      help="Which port on smtp server to deliver mail to. "
+                           "Default is %default.")
+    parser.add_option('--username',
+                      help="Username to use to log in to smtp server.  Default "
+                           "is none.")
+    parser.add_option('--password',
+                      help="Password to use to log in to smtp server.  Must be "
+                           "specified if username is specified.")
+    parser.add_option('--force-tls', action='store_true',
+                      help="Do not connect if TLS is not available.  Not "
+                           "enabled by default.")
+    parser.add_option('--no-tls', action='store_true',
+                      help="Do not use TLS even if is available.  Not enabled "
+                           "by default.")
+    parser.add_option('--config', metavar='<inifile>',
+                      help="Get configuration from specified ini file; it must "
+                           "contain a section [app:zope-sendmail].")
 
-    OPTIONS:
-
-        --daemon            Run in daemon mode, periodically checking queue
-                            and sending messages.  Default is to send all
-                            messages in queue once and exit.
-
-        --interval <#secs>  How often to check queue when in daemon mode.
-                            Default is 3 seconds.
-
-        --hostname          Name of smtp host to use for delivery.  Default is
-                            localhost.
-
-        --port              Which port on smtp server to deliver mail to.
-                            Default is 25.
-
-        --username          Username to use to log in to smtp server.  Default
-                            is none.
-
-        --password          Password to use to log in to smtp server.  Must be
-                            specified if username is specified.
-
-        --force-tls         Do not connect if TLS is not available.  Not
-                            enabled by default.
-
-        --no-tls            Do not use TLS even if is available.  Not enabled
-                            by default.
-
-        --config <inifile>  Get configuration from specified ini file; it must
-                            contain a section [app:zope-sendmail].
-
-    """
-
-    _error = False
     daemon = False
     interval = 3
     hostname = 'localhost'
@@ -386,74 +382,44 @@ class ConsoleApp(object):
     no_tls = False
     queue_path = None
 
-    def __init__(self, argv=sys.argv, verbose=True):
+    def __init__(self, argv=None, verbose=True):
+        if argv is None:
+            argv = sys.argv
         self.script_name = argv[0]
         self.verbose = verbose
         self._process_args(argv[1:])
         self.mailer = SMTPMailer(self.hostname, self.port, self.username,
             self.password, self.no_tls, self.force_tls)
 
     def main(self):
-        if self._error:
-            return
         queue = QueueProcessorThread(self.interval)
         queue.setMailer(self.mailer)
         queue.setQueuePath(self.queue_path)
         queue.run(forever=self.daemon)
 
     def _process_args(self, args):
-        got_queue_path = False
-        while args:
-            arg = args.pop(0)
-            if arg == "--daemon":
-                self.daemon = True
-            elif arg == "--interval":
-                try:
-                    self.interval = float(args.pop(0))
-                except:
-                    self._error_usage()
-            elif arg == "--hostname":
-                if not args:
-                    self._error_usage()
-                self.hostname = args.pop(0)
-            elif arg == "--port":
-                try:
-                    self.port = int(args.pop(0))
-                except:
-                    self._error_usage()
-            elif arg == "--username":
-                if not args:
-                    self._error_usage()
-                self.username = args.pop(0)
-            elif arg == "--password":
-                if not args:
-                    self._error_usage()
-                self.password = args.pop(0)
-            elif arg == "--force-tls":
-                self.force_tls = True
-            elif arg == "--no-tls":
-                self.no_tls = True
-            elif arg == "--config":
-                if not args:
-                    self._error_usage()
-                self._load_config(args.pop(0))
-            elif arg.startswith("-") or got_queue_path:
-                self._error_usage()
-            else:
-                self.queue_path = arg
-                got_queue_path = True
+        opts, args = self.parser.parse_args(args)
+        self.daemon = opts.daemon
+        self.interval = opts.interval
+        self.hostname = opts.hostname
+        self.port = opts.port
+        self.username = opts.username
+        self.password = opts.password
+        self.force_tls = opts.force_tls
+        self.no_tls = opts.no_tls
+        if opts.config:
+            self._load_config(opts.config)
+        if len(args) > 1:
+            self.parser.error('too many arguments')
+        elif args:
+            self.queue_path = args[0]
         if not self.queue_path:
-            self._error_usage()
+            self.parser.error('please specify the queue path')
         if (self.username or self.password) and \
            not (self.username and self.password):
-            if self.verbose:
-                print >>sys.stderr, "Must use username and password together."
-            self._error = True
+            self.parser.error('Must use username and password together.')
         if self.force_tls and self.no_tls:
-            if self.verbose:
-                print >>sys.stderr, \
-                    "--force-tls and --no-tls are mutually exclusive."
-            self._error = True
+            self.parser.error('--force-tls and --no-tls are mutually exclusive.')
 
     def _load_config(self, path):
         section = "app:zope-sendmail"
@@ -479,12 +445,6 @@ def _load_config(self, path):
         self.no_tls = boolean(config.get(section, "no_tls"))
         self.queue_path = string_or_none(config.get(section, "queue_path"))
 
-    def _error_usage(self):
-        if self.verbose:
-            print >>sys.stderr, self._usage % {"script_name": self.script_name,}
-        self._error = True
-
-
 def run():
     logging.basicConfig()
     app = ConsoleApp()

src/zope/sendmail/tests/test_queue.py

@@ -18,6 +18,8 @@
 
 import os.path
 import shutil
+import sys
+from cStringIO import StringIO
 
 from tempfile import mkdtemp
 from unittest import TestCase, TestSuite, makeSuite, main
@@ -177,16 +179,18 @@ def setUp(self):
         self.delivery = QueuedMailDelivery(self.queue_dir)
         self.maildir = Maildir(self.queue_dir, True)
         self.mailer = MailerStub()
-
+        self.real_stderr = sys.stderr
+        self.stderr = StringIO()
+
     def tearDown(self):
+        sys.stderr = self.real_stderr
         shutil.rmtree(self.dir)
 
     def test_args_processing(self):
         # simplest case that works
         cmdline = "zope-sendmail %s" % self.dir
         app = ConsoleApp(cmdline.split(), verbose=False)
         self.assertEquals("zope-sendmail", app.script_name)
-        self.assertFalse(app._error)
         self.assertEquals(self.dir, app.queue_path)
         self.assertFalse(app.daemon)
         self.assertEquals(3, app.interval)
@@ -196,27 +200,20 @@ def test_args_processing(self):
         self.assertEquals(None, app.password)
         self.assertFalse(app.force_tls)
         self.assertFalse(app.no_tls)
-        # simplest case that doesn't work
+
+    def test_args_processing_no_queue_path(self):
+        # simplest case that doesn't work: no queue path specified
         cmdline = "zope-sendmail"
-        app = ConsoleApp(cmdline.split(), verbose=False)
-        self.assertEquals("zope-sendmail", app.script_name)
-        self.assertTrue(app._error)
-        self.assertEquals(None, app.queue_path)
-        self.assertFalse(app.daemon)
-        self.assertEquals(3, app.interval)
-        self.assertEquals("localhost", app.hostname)
-        self.assertEquals(25, app.port)
-        self.assertEquals(None, app.username)
-        self.assertEquals(None, app.password)
-        self.assertFalse(app.force_tls)
-        self.assertFalse(app.no_tls)
+        sys.stderr = self.stderr
+        self.assertRaises(SystemExit, ConsoleApp, cmdline.split(), verbose=False)
+
+    def test_args_processing_almost_all_options(self):
         # use (almost) all of the options
         cmdline = "zope-sendmail --daemon --interval 7 --hostname foo --port 75 " \
             "--username chris --password rossi --force-tls " \
             "%s" % self.dir
         app = ConsoleApp(cmdline.split(), verbose=False)
         self.assertEquals("zope-sendmail", app.script_name)
-        self.assertFalse(app._error)
         self.assertEquals(self.dir, app.queue_path)
         self.assertTrue(app.daemon)
         self.assertEquals(7, app.interval)
@@ -226,18 +223,19 @@ def test_args_processing(self):
         self.assertEquals("rossi", app.password)
         self.assertTrue(app.force_tls)
         self.assertFalse(app.no_tls)
+
+    def test_args_processing_username_without_password(self):
         # test username without password
         cmdline = "zope-sendmail --username chris %s" % self.dir
-        app = ConsoleApp(cmdline.split(), verbose=False)
-        self.assertTrue(app._error)
-        # test --tls and --no-tls together
-        cmdline = "zope-sendmail --tls --no-tls %s" % self.dir
-        app = ConsoleApp(cmdline.split(), verbose=False)
-        self.assertTrue(app._error)
+        sys.stderr = self.stderr
+        self.assertRaises(SystemExit, ConsoleApp, cmdline.split(), verbose=False)
+
+    def test_args_processing_force_tls_and_no_tls(self):
         # test force_tls and no_tls
-        comdline = "zope-sendmail --force-tls --no-tls %s" % self.dir
-        self.assertTrue(app._error)
-
+        cmdline = "zope-sendmail --force-tls --no-tls %s" % self.dir
+        sys.stderr = self.stderr
+        self.assertRaises(SystemExit, ConsoleApp, cmdline.split(), verbose=False)
+
     def test_ini_parse(self):
         ini_path = os.path.join(self.dir, "zope-sendmail.ini")
         f = open(ini_path, "w")
@@ -247,7 +245,6 @@ def test_ini_parse(self):
         cmdline = """zope-sendmail --config %s""" % ini_path
         app = ConsoleApp(cmdline.split(), verbose=False)
         self.assertEquals("zope-sendmail", app.script_name)
-        self.assertFalse(app._error)
         self.assertEquals("hammer/dont/hurt/em", app.queue_path)
         self.assertFalse(app.daemon)
         self.assertEquals(33, app.interval)
@@ -264,7 +261,6 @@ def test_ini_parse(self):
         cmdline = """zope-sendmail --config %s %s""" % (ini_path, self.dir)
         app = ConsoleApp(cmdline.split(), verbose=False)
         self.assertEquals("zope-sendmail", app.script_name)
-        self.assertFalse(app._error)
         self.assertEquals(self.dir, app.queue_path)
         self.assertFalse(app.daemon)
         self.assertEquals(3, app.interval)