From 4e1609b12eb1a44b0fc8519de458362d7fd8cdf2 Mon Sep 17 00:00:00 2001 From: Luke Murphy Date: Mon, 28 Nov 2016 00:55:49 +0100 Subject: [PATCH] Add `type` validation. Argparse driven argument type validation is added for the `--junit-xml` and `--confcutdir` arguments. The commit partially reverts #2080. Closes #2089. --- CHANGELOG.rst | 8 ++++---- _pytest/config.py | 27 +++++++++++++++++++++++---- _pytest/junitxml.py | 3 +++ _pytest/main.py | 4 +++- testing/test_junitxml.py | 4 ++++ 5 files changed, 37 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 77b67ddbb..4025eb24b 100644 --- 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 index 61123f6ac..4eed5ace1 100644 --- 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 @@ class Config(object): "(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 @@ class Config(object): 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 index 4b9103949..317382e63 100644 --- a/_pytest/junitxml.py +++ b/_pytest/junitxml.py @@ -8,12 +8,14 @@ Based on initial code from Ross Lawley. # 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 index 2b5b12875..2562df3af 100644 --- 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 @@ try: 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 index 443b8111b..abbc9cd33 100644 --- a/testing/test_junitxml.py +++ b/testing/test_junitxml.py @@ -715,6 +715,10 @@ def test_logxml_makedir(testdir): 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("""