tests of (and fixes for) Config.fromdictargs

[bukzor]

failures shown here:

============================= test session starts ==============================
platform linux2 -- Python 2.7.6, pytest-2.8.0, py-1.4.30, pluggy-0.3.1
rootdir: /nail/home/buck/trees/mine/pytest, inifile: tox.ini
collected 50 items

testing/test_config.py FFF

=================================== FAILURES ===================================
__________________ TestConfigFromdictargs.test_basic_behavior __________________

self = <test_config.TestConfigFromdictargs instance at 0x2201830>

    def test_basic_behavior(self):
        from _pytest.config import Config
        option_dict = {
            'verbose': 1e100,
            'foo': 'bar',
        }
        args = ['a', 'b']

        config = Config.fromdictargs(option_dict, args)
        with pytest.raises(AssertionError):
>           config.parse(['should to parse again'])

testing/test_config.py:279: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
_pytest/config.py:956: in parse
    self._preparse(args)
_pytest/config.py:927: in _preparse
    args=args, parser=self._parser)
_pytest/vendored_packages/pluggy.py:724: in __call__
    return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
_pytest/vendored_packages/pluggy.py:338: in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
_pytest/vendored_packages/pluggy.py:333: in <lambda>
    _MultiCall(methods, kwargs, hook.spec_opts).execute()
_pytest/vendored_packages/pluggy.py:595: in execute
    return _wrapped_call(hook_impl.function(*args), self.execute)
_pytest/vendored_packages/pluggy.py:244: in _wrapped_call
    next(wrap_controller)   # first yield
_pytest/capture.py:37: in pytest_load_initial_conftests
    pluginmanager.register(capman, "capturemanager")
_pytest/config.py:206: in register
    ret = super(PytestPluginManager, self).register(plugin, name)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <_pytest.config.PytestPluginManager object at 0x220d5d0>
plugin = <_pytest.capture.CaptureManager instance at 0x22fe680>
name = 'capturemanager'

    def register(self, plugin, name=None):
        """ Register a plugin and return its canonical name or None if the name
            is blocked from registering.  Raise a ValueError if the plugin is already
            registered. """
        plugin_name = name or self.get_canonical_name(plugin)

        if plugin_name in self._name2plugin or plugin in self._plugin2hookcallers:
            if self._name2plugin.get(plugin_name, -1) is None:
                return  # blocked plugin, return None to indicate no registration
            raise ValueError("Plugin already registered: %s=%s\n%s" %
>                           (plugin_name, plugin, self._name2plugin))
E           ValueError: Plugin already registered: capturemanager=<_pytest.capture.CaptureManager instance at 0x22fe680>
E           {'cacheprovider': <module '_pytest.cacheprovider' from '/nail/home/buck/trees/mine/pytest/_pytest/cacheprovider.pyc'>, 'helpconfig': <module '_pytest.helpconfig' from '/nail/home/buck/trees/mine/pytest/_pytest/helpconfig.pyc'>, 'pytestconfig': <_pytest.config.Config object at 0x2147950>, 'runner': <module '_pytest.runner' from '/nail/home/buck/trees/mine/pytest/_pytest/runner.pyc'>, 'unittest': <module '_pytest.unittest' from '/nail/home/buck/trees/mine/pytest/_pytest/unittest.pyc'>, 'pastebin': <module '_pytest.pastebin' from '/nail/home/buck/trees/mine/pytest/_pytest/pastebin.pyc'>, 'skipping': <module '_pytest.skipping' from '/nail/home/buck/trees/mine/pytest/_pytest/skipping.pyc'>, 'genscript': <module '_pytest.genscript' from '/nail/home/buck/trees/mine/pytest/_pytest/genscript.pyc'>, 'tmpdir': <module '_pytest.tmpdir' from '/nail/home/buck/trees/mine/pytest/_pytest/tmpdir.pyc'>, 'capture': <module '_pytest.capture' from '/nail/home/buck/trees/mine/pytest/_pytest/capture.pyc'>, 'assertion': <module '_pytest.assertion' from '/nail/home/buck/trees/mine/pytest/_pytest/assertion/__init__.pyc'>, 'mark': <module '_pytest.mark' from '/nail/home/buck/trees/mine/pytest/_pytest/mark.pyc'>, 'terminal': <module '_pytest.terminal' from '/nail/home/buck/trees/mine/pytest/_pytest/terminal.pyc'>, 'main': <module '_pytest.main' from '/nail/home/buck/trees/mine/pytest/_pytest/main.pyc'>, 'nose': <module '_pytest.nose' from '/nail/home/buck/trees/mine/pytest/_pytest/nose.pyc'>, 'python': <module '_pytest.python' from '/nail/home/buck/trees/mine/pytest/_pytest/python.pyc'>, 'recwarn': <module '_pytest.recwarn' from '/nail/home/buck/trees/mine/pytest/_pytest/recwarn.pyc'>, '35706320': <_pytest.config.PytestPluginManager object at 0x220d5d0>, 'monkeypatch': <module '_pytest.monkeypatch' from '/nail/home/buck/trees/mine/pytest/_pytest/monkeypatch.pyc'>, 'resultlog': <module '_pytest.resultlog' from '/nail/home/buck/trees/mine/pytest/_pytest/resultlog.pyc'>, 'capturemanager': <_pytest.capture.CaptureManager instance at 0x22f8878>, 'junitxml': <module '_pytest.junitxml' from '/nail/home/buck/trees/mine/pytest/_pytest/junitxml.pyc'>, 'doctest': <module '_pytest.doctest' from '/nail/home/buck/trees/mine/pytest/_pytest/doctest.pyc'>, 'pytester': <module '_pytest.pytester' from '/nail/home/buck/trees/mine/pytest/_pytest/pytester.pyc'>, 'pdb': <module '_pytest.pdb' from '/nail/home/buck/trees/mine/pytest/_pytest/pdb.pyc'>}

_pytest/vendored_packages/pluggy.py:350: ValueError
_____________________ TestConfigFromdictargs.test_origargs _____________________

self = <test_config.TestConfigFromdictargs instance at 0x24add88>

    def test_origargs(self):
        """Show that fromdictargs can handle args in their "orig" format"""
        from _pytest.config import Config
        option_dict = {}
        args = ['-vvvv', 'a', 'b']

        config = Config.fromdictargs(option_dict, args)
>       assert config.args == ['a', 'b']
E       AttributeError: 'Config' object has no attribute 'args'

testing/test_config.py:291: AttributeError
___________________ TestConfigFromdictargs.test_inifilename ____________________

self = <test_config.TestConfigFromdictargs instance at 0x230f248>

    def test_inifilename(self):
        from _pytest.config import Config
        inifile = '../../foo/bar.ini',
        option_dict = {
            'inifilename': inifile,
        }

        config = Config.fromdictargs(option_dict, ())
        assert config.option.inifilename == inifile

        # this indicates this is the file used for getting configuration values
>       assert config.inifile == inifile
E       assert local('/nail/home/buck/trees/mine/pytest/tox.ini') == ('../../foo/bar.ini',)
E        +  where local('/nail/home/buck/trees/mine/pytest/tox.ini') = <_pytest.config.Config object at 0x22fd450>.inifile

testing/test_config.py:306: AssertionError
=========================== short test summary info ============================
FAIL testing/test_config.py::TestConfigFromdictargs::()::test_basic_behavior
FAIL testing/test_config.py::TestConfigFromdictargs::()::test_origargs
FAIL testing/test_config.py::TestConfigFromdictargs::()::test_inifilename
====================== 47 tests deselected by '-kdictarg' ======================
=================== 3 failed, 47 deselected in 0.29 seconds ====================

I tried quite hard to also supply a fixing patch, but there's a tangled hairball of mutation in parse, _preparse, Config._parser.get_parser() that I couldn't break through.

This patch fixes all the assertions but the (most important) inifile assertion, and is still kind of messy:

diff --git a/_pytest/config.py b/_pytest/config.py
index def26a0..56eddd4 100644
--- a/_pytest/config.py
+++ b/_pytest/config.py
@@ -878,7 +878,7 @@ class Config(object):
     def fromdictargs(cls, option_dict, args):
         """ constructor useable for subprocesses. """
         config = get_config()
-        config._preparse(args, addopts=False)
+        config.parse(args, addopts=False)
         config.option.__dict__.update(option_dict)
         for x in config.option.plugins:
             config.pluginmanager.consider_pluginarg(x)
@@ -946,14 +946,14 @@ class Config(object):
                     self.inicfg.config.path, self.inicfg.lineof('minversion'),
                     minver, pytest.__version__))

-    def parse(self, args):
+    def parse(self, args, addopts=True):
         # parse given cmdline arguments into this config object.
         assert not hasattr(self, 'args'), (
                 "can only parse cmdline args at most once per Config object")
         self._origargs = args
         self.hook.pytest_addhooks.call_historic(
                                   kwargs=dict(pluginmanager=self.pluginmanager))
-        self._preparse(args)
+        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)

[nicoddemus]

Thanks for your work anyway @bukzor!

Sorry for asking, was this requested by someone or did you just decided to add some tests out of the blue? Anyway, this is very much appreciated!

[bukzor]

@nicoddemus This is causing me major pain problems under xdist: the -c option is not honored.
This is also the root cause for -s option not working under xdist.

I discussed my issues with ronny in #pylib and we agreed a good starting point would be a test suite.
@RonnyPfannschmidt

[bukzor]

Here's the transcript, for context (also so I don't lose it):

buck1 i'm seeing that xdist is gobbling options, and having trouble forming a patch
buck1 shall i just write the test and let someone else fill in the blank?
ronny buck1: please start with creating a test, then we can get a basic idea and help you figure more
buck1 ronny: is there anywhere i can look to understand the design intent behind Config._preparse, plugin.consider_preparse
ronny buck1: its to allow the plugin manager t enable/disable extra given plugins
ronny like -p no:cov to dissable the coverage plugin when its installed
buck1 option_dict['plugins'].append("no:terminal")
ronny buck1: what are you trying to do?
buck1 trying to get xdist to honor my -c
buck1 that's just pre-existing code in xdist i pasted
ronny ah, does it open a different configfile?
buck1 the orig_args are not passed, not available during _preparse
buck1 and _preparse doesn't look at option_dict
buck1 and _preparse is what sets inifile
buck1 i'm trying to pass the orig-args, but maybe that's wrongheaded
ronny buck1: you might want to open a wip pull request, that adds a xfailing test and the builds uppon that to make a problem solution
ronny (currently im short of time)
buck1 ronny: i can't decide whether this is a xdist or pytest bug
buck1 might want regression tests in both spots
buck1 regardless of patch
buck1 hopefully Config.fromdictargs has a unit test
buck1 i'll start there if so
buck1 ronny: thanks for your time  :)

[bukzor]

In total, minus ~3 bugs, plus 51 lines of test, and minus 1 line of implementation.
When pushed, xdist will work much better, especially with respect to -c/--inifile option.

[RonnyPfannschmidt]

TheParse known vs parse known and unknown distinction is needed currently since that api is used by downstreams and the change you pushed breaks them

The rest of the change looks fabulous at first glance, since subtle backward compatibility issues are involved I'll do a deeper review them s weekend

[bukzor]

I kept those bits separate, easily pulled back out. Shall do.

If this gets merged, when might it make it to pypi? I'll start using this
branch in my project today, and will keep an eye open for issues.

On Thu, Sep 24, 2015, 21:56 Ronny Pfannschmidt [email protected]
wrote:

TheParse known vs parse known and unknown distinction is needed currently
since that api is used by downstreams and the change you pushed breaks them

The rest of the change looks fabulous at first glance, since subtle
backward compatibility issues are involved I'll do a deeper review them s
weekend

—
Reply to this email directly or view it on GitHub
#1060 (comment).

[nicoddemus]

@bukzor does this fix #680 as well?

[RonnyPfannschmidt]

I suppose that needs a test case as well

We can't merge the pr as is tho, I can't do a cleanup today

[hpk42]

So what is the original issue? That "py.test -c some-ini -n1" does not work?

[bukzor]

migrated to #1124