Add type validation.

decentral1se · decentral1se · commit 4e1609b12eb1 · 2016-11-28T02:16:01.000+01:00
Argparse driven argument type validation is added for the `--junit-xml` and `--confcutdir` arguments. The commit partially reverts #2080. Closes #2089.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -1,6 +1,9 @@
 3.0.5.dev0
 ==========
 
+* Now ``--confcutdir`` and ``--junit-xml`` are properly validated if they are directories
+  and filenames, respectively (`#2089`_ and `#2078`_). Thanks to `@lwm`_ for the PR.
+
 * Add hint to error message hinting possible missing ``__init__.py`` (`#478`_). Thanks `@DuncanBetts`_.
 
 * Provide ``:ref:`` targets for ``recwarn.rst`` so we can use intersphinx referencing.
@@ -10,10 +13,6 @@
   ``pytest.Function``, ``pytest.Module``, etc., instead (`#2034`_).
   Thanks `@nmundar`_ for the PR.
 
-* An error message is now displayed if ``--confcutdir`` is not a valid directory, avoiding
-  subtle bugs (`#2078`_).
-  Thanks `@nicoddemus`_ for the PR.
-
 * Fix error message using ``approx`` with complex numbers (`#2082`_).
   Thanks `@adler-j`_ for the report and `@nicoddemus`_ for the PR.
 
@@ -33,6 +32,7 @@
 .. _@nedbat: https://github.com/nedbat
 .. _@nmundar: https://github.com/nmundar
 
+.. _#2089: https://github.com/pytest-dev/pytest/issues/2089
 .. _#478: https://github.com/pytest-dev/pytest/issues/478
 .. _#2034: https://github.com/pytest-dev/pytest/issues/2034
 .. _#2038: https://github.com/pytest-dev/pytest/issues/2038
diff --git a/_pytest/config.py b/_pytest/config.py
@@ -70,6 +70,28 @@ class UsageError(Exception):
     """ error in pytest usage or invocation"""
 
 
+def filename_arg(path, optname):
+    """ Argparse type validator for filename arguments.
+
+    :path: path of filename
+    :optname: name of the option
+    """
+    if os.path.isdir(path):
+        raise UsageError("{0} must be a filename, given: {1}".format(optname, path))
+    return path
+
+
+def directory_arg(path, optname):
+    """Argparse type validator for directory arguments.
+
+    :path: path of directory
+    :optname: name of the option
+    """
+    if not os.path.isdir(path):
+        raise UsageError("{0} must be a directory, given: {1}".format(optname, path))
+    return path
+
+
 _preinit = []
 
 default_plugins = (
@@ -996,7 +1018,6 @@ def _warn_about_missing_assertion(self, mode):
                                  "(are you using python -O?)\n")
 
     def _preparse(self, args, addopts=True):
-        import pytest
         self._initini(args)
         if addopts:
             args[:] = shlex.split(os.environ.get('PYTEST_ADDOPTS', '')) + args
@@ -1009,9 +1030,7 @@ def _preparse(self, args, addopts=True):
         self.pluginmanager.consider_env()
         self.known_args_namespace = ns = self._parser.parse_known_args(args, namespace=self.option.copy())
         confcutdir = self.known_args_namespace.confcutdir
-        if confcutdir and not os.path.isdir(confcutdir):
-            raise pytest.UsageError('--confcutdir must be a directory, given: {0}'.format(confcutdir))
-        if confcutdir is None and self.inifile:
+        if self.known_args_namespace.confcutdir is None and self.inifile:
             confcutdir = py.path.local(self.inifile).dirname
             self.known_args_namespace.confcutdir = confcutdir
         try:
diff --git a/_pytest/junitxml.py b/_pytest/junitxml.py
@@ -8,12 +8,14 @@
 # Output conforms to https://github.com/jenkinsci/xunit-plugin/blob/master/
 # src/main/resources/org/jenkinsci/plugins/xunit/types/model/xsd/junit-10.xsd
 
+import functools
 import py
 import os
 import re
 import sys
 import time
 import pytest
+from _pytest.config import filename_arg
 
 # Python 2.X and 3.X compatibility
 if sys.version_info[0] < 3:
@@ -214,6 +216,7 @@ def pytest_addoption(parser):
         action="store",
         dest="xmlpath",
         metavar="path",
+        type=functools.partial(filename_arg, optname="--junitxml"),
         default=None,
         help="create junit-xml style report file at given path.")
     group.addoption(
diff --git a/_pytest/main.py b/_pytest/main.py
@@ -1,4 +1,5 @@
 """ core implementation of testing process: init, session, runtest loop. """
+import functools
 import os
 import sys
 
@@ -11,6 +12,7 @@
 except ImportError:
     from UserDict import DictMixin as MappingMixin
 
+from _pytest.config import directory_arg
 from _pytest.runner import collect_one_node
 
 tracebackcutdir = py.path.local(_pytest.__file__).dirpath()
@@ -58,7 +60,7 @@ def pytest_addoption(parser):
     # when changing this to --conf-cut-dir, config.py Conftest.setinitial
     # needs upgrading as well
     group.addoption('--confcutdir', dest="confcutdir", default=None,
-        metavar="dir",
+        metavar="dir", type=functools.partial(directory_arg, optname="--confcutdir"),
         help="only load conftest.py's relative to specified dir.")
     group.addoption('--noconftest', action="store_true",
         dest="noconftest", default=False,
diff --git a/testing/test_junitxml.py b/testing/test_junitxml.py
@@ -715,6 +715,10 @@ def test_pass():
     assert result.ret == 0
     assert testdir.tmpdir.join("path/to/results.xml").check()
 
+def test_logxml_check_isdir(testdir):
+    """Give an error if --junit-xml is a directory (#2089)"""
+    result = testdir.runpytest("--junit-xml=.")
+    result.stderr.fnmatch_lines(["*--junitxml must be a filename*"])
 
 def test_escaped_parametrized_names_xml(testdir):
     testdir.makepyfile("""
