Merge pull request #2458 from segevfiner/fix-required-options-help

Fix --help with required options

AUTHORS

@@ -144,6 +144,7 @@ Ross Lawley
 Russel Winder
 Ryan Wooden
 Samuele Pedroni
+Segev Finer
 Simon Gomizelj
 Skylar Downes
 Stefan Farmbauer

_pytest/config.py

@@ -71,6 +71,12 @@ class UsageError(Exception):
     """ error in pytest usage or invocation"""
 
 
+class PrintHelp(Exception):
+    """Raised when pytest should print it's help to skip the rest of the
+    argument parsing and validation."""
+    pass
+
+
 def filename_arg(path, optname):
     """ Argparse type validator for filename arguments.
 
@@ -1100,14 +1106,18 @@ class Config(object):
         self._preparse(args, addopts=addopts)
         # XXX deprecated hook:
         self.hook.pytest_cmdline_preparse(config=self, args=args)
-        args = self._parser.parse_setoption(args, self.option, namespace=self.option)
-        if not args:
-            cwd = os.getcwd()
-            if cwd == self.rootdir:
-                args = self.getini('testpaths')
+        self._parser.after_preparse = True
+        try:
+            args = self._parser.parse_setoption(args, self.option, namespace=self.option)
             if not args:
-                args = [cwd]
-        self.args = args
+                cwd = os.getcwd()
+                if cwd == self.rootdir:
+                    args = self.getini('testpaths')
+                if not args:
+                    args = [cwd]
+            self.args = args
+        except PrintHelp:
+            pass
 
     def addinivalue_line(self, name, line):
         """ add a line to an ini-file option. The option must have been

_pytest/helpconfig.py

@@ -3,13 +3,46 @@ from __future__ import absolute_import, division, print_function
 
 import py
 import pytest
+from _pytest.config import PrintHelp
 import os, sys
+from argparse import Action
+
+
+class HelpAction(Action):
+    """This is an argparse Action that will raise an exception in
+    order to skip the rest of the argument parsing when --help is passed.
+    This prevents argparse from quitting due to missing required arguments
+    when any are defined, for example by ``pytest_addoption``.
+    This is similar to the way that the builtin argparse --help option is
+    implemented by raising SystemExit.
+    """
+
+    def __init__(self,
+                 option_strings,
+                 dest=None,
+                 default=False,
+                 help=None):
+        super(HelpAction, self).__init__(
+            option_strings=option_strings,
+            dest=dest,
+            const=True,
+            default=default,
+            nargs=0,
+            help=help)
+
+    def __call__(self, parser, namespace, values, option_string=None):
+        setattr(namespace, self.dest, self.const)
+
+        # We should only skip the rest of the parsing after preparse is done
+        if getattr(parser._parser, 'after_preparse', False):
+            raise PrintHelp
+
 
 def pytest_addoption(parser):
     group = parser.getgroup('debugconfig')
     group.addoption('--version', action="store_true",
             help="display pytest lib version and import information.")
-    group._addoption("-h", "--help", action="store_true", dest="help",
+    group._addoption("-h", "--help", action=HelpAction, dest="help",
             help="show help message and configuration info")
     group._addoption('-p', action="append", dest="plugins", default = [],
                metavar="name",

changelog/1999.bugfix

@@ -0,0 +1,2 @@
+Required options added via ``pytest_addoption`` will no longer prevent
+using --help without passing them.

testing/test_conftest.py

@@ -449,3 +449,15 @@ def test_hook_proxy(testdir):
         '*test_foo4.py*',
         '*3 passed*',
     ])
+
+
+def test_required_option_help(testdir):
+    testdir.makeconftest("assert 0")
+    x = testdir.mkdir("x")
+    x.join("conftest.py").write(_pytest._code.Source("""
+        def pytest_addoption(parser):
+            parser.addoption("--xyz", action="store_true", required=True)
+    """))
+    result = testdir.runpytest("-h", x)
+    assert 'argument --xyz is required' not in result.stdout.str()
+    assert 'general:' in result.stdout.str()