Unable to run tests with +REQUIRES(--flag) using pytest

[karenc]

When using pytest to run doctests, I can't figure out a way to run the tests that are marked +REQUIRES(--flag). For example, I was trying to run the tests with +REQUIRES(--web):

(env3) root@5af053f4ef0e:/wbia/wildbook-ia# pytest --web                 
ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]         
pytest: error: unrecognized arguments: --web 
  inifile: /wbia/wildbook-ia/setup.cfg 
  rootdir: /wbia/wildbook-ia

I looked into the xdoctest code and came up with something a bit different: (Edit: This isn't the right thing to do)

(env3) root@5af053f4ef0e:/wbia/wildbook-ia# pytest --xdoctest-options='+REQUIRES(--web)' wbia/web/apis.py
...
            elif action == 'assign':                                     
                state[key] = value
            elif action == 'set.add':                                    
>               state[key].add(value)                                    
E               AttributeError: 'bool' object has no attribute 'add'     

/virtualenv/env3/lib/python3.6/site-packages/xdoctest/directive.py:267: AttributeError

but at least that appears to run the right number of tests, it's just that the tests aren't actually run because of the attribute error in xdoctest.

This is our pytest config in setup.cfg:

[tool:pytest]
minversion = 5.4
addopts = -p no:doctest --xdoctest --xdoctest-style=google
testpaths =
    wbia
filterwarnings =
    default
    ignore:.*No cfgstr given in Cacher constructor or call.*:Warning
    ignore:.*Define the __nice__ method for.*:Warning

[Erotemic]

It should be as simple as adding --web to the command line when you run pytest. EG: pytest --web. However, pytest will reject any unrecognized arguments by default. However, there is a simple workaround. Add a file conftest.py to the root of the repository and add the contents:

import pytest  # NOQA


def pytest_addoption(parser):
    # Allow --network to be passed in as an option on sys.argv
    parser.addoption("--web", action="store_true")

This will force pytest to recognize the CLI argument and prevent it from raising: error: unrecognized arguments: --web

I do this in ubelt with the --network flag.

For example in ubelt, the doctests in ubelt/util_download.py contain the directive >>> # xdoctest: +REQUIRES(--network). When I run pytest ubelt/util_download.py it skips all of the tests because I don't have the required CLI flag.

(py38) joncrall@Ooo:~/code/ubelt$ pytest ubelt/util_download.py
======================================================== test session starts =========================================================
platform linux -- Python 3.8.3, pytest-5.4.3, py-1.9.0, pluggy-0.13.1
rootdir: /home/joncrall/code/ubelt, inifile: pytest.ini
plugins: cov-2.10.0, timeout-1.4.1, xdoctest-0.13.0
collected 6 items                                                                                                                    

ubelt/util_download.py ssssss                                                                                                  [100%]

========================================================= 6 skipped in 0.19s =========================================================

But if I run pytest ubelt/util_download.py --network it runs everything:

(py38) joncrall@Ooo:~/code/ubelt$ pytest ubelt/util_download.py --network
======================================================== test session starts =========================================================
platform linux -- Python 3.8.3, pytest-5.4.3, py-1.9.0, pluggy-0.13.1
rootdir: /home/joncrall/code/ubelt, inifile: pytest.ini
plugins: cov-2.10.0, timeout-1.4.1, xdoctest-0.13.0
collected 6 items                                                                                                                    

ubelt/util_download.py ......                                                                                                  [100%]

========================================================= 6 passed in 2.92s ==========================================================

But unfortunately it is important to have that conftest.py file.

An improvement to xdoctest might be to allow the REQUIRES directive to inspect environment variables via some syntax like: # xdoctest: +REQUIRES(env:WEB) or # xdoctest: +REQUIRES(env:WEB==1), so then you could call pytest like WEB=1 pytest and prevent the need for a conftest.py file.

[karenc]

Thanks for your help!

[karenc]

Would something like this be ok?

pytest --xdoctest-flag=web wbia/web/apis.py::hello_world:0

equivalent to:

xdoctest --web wbia/web/apis.py::hello_world:0

So there's no need to add each flag to conftest.py.

The changes that I made to make this work (based on xdoctest 0.13.0):

diff --git a/doctest_example.py b/doctest_example.py
index 0dec694..f50ec53 100644               
--- a/doctest_example.py                    
+++ b/doctest_example.py                    
@@ -57,6 +57,9 @@ class Config(dict):       
             for optpart in directive_optstr.split(','):
                 directive = parse_directive_optstr(optpart)
                 default_runtime_state[directive.name] = directive.positive
+        for flag in ns.get('flag', []):    
+            sys.argv.append('--{}'.format(flag))
+                                           
         _examp_conf = {                    
             'default_runtime_state': default_runtime_state,
             'offset_linenos': ns['offset_linenos'],
diff --git a/plugin.py b/plugin.py          
index 0c0d4c9..ffdc258 100644               
--- a/plugin.py                             
+++ b/plugin.py
@@ -103,6 +103,11 @@ def pytest_addoption(parser):
                     choices=['static', 'dynamic', 'auto'],
                     dest='xdoctest_analysis')

+    group.addoption('--xdoctest-flag', '--xdoc-flag',
+                    action='append', default=[], metavar='XDOCTEST_FLAG',
+                    help='Run tests marked with +REQUIRES(--XDOCTEST_FLAG)',
+                    dest='xdoctest_flag')
+    
     from xdoctest import doctest_example
     doctest_example.Config()._update_argparse_cli(
         group.addoption, prefix=['xdoctest', 'xdoc'],
@@ -201,6 +206,8 @@ class _XDoctestBase(pytest.Module):
                 return self.config.getvalue('xdoctest_' + attr)
             def __getattr__(self, attr):
                 return self.config.getvalue('xdoctest_' + attr)
+            def get(self, attr, default=None):
+                return self.config.getvalue('xdoctest_' + attr, default)
         
         ns = NamespaceLike(self.config)

(Feel free to change the name --xdoctest-flag.)

[Erotemic]

The only thing I don't like about this is modifying sys.argv.

I've made it an explicit point to ensure that xdoctest doesn't modify any global state, and I really don't want to add code that does that.

I made a small proof of concept change in #77 where I implement respect for environment variables in the REQUIRES directive.

While doing this I thought of another workaround that you can do. It's hacky, but it will work.

Use the --xdoctest-global-exec flag to specify: "--xdoctest-global-exec=import sys; sys.argv.append('--web')", and that code will run before every doctest, which means that when the requirements are checked it will be satisfied.

[karenc]

I added to sys.argv because that's what directive.py is checking. It's probably possible to change it so directive.py checks some config instead of sys.argv, then we can avoid modifying sys.argv.

[Erotemic]

The way I think about sys.argv is that it should be completely defined by the user. Its ok for a program to read from it, but in general I'm vary wary of when a program writes to it. Given that xdoctest is a non-project specific tool with many different users I don't want to introduce a code path that might have any potential of messing with a variable that the user should have complete control over.

I'm certainly open to adding another config that can be checked that would have an effect on how doctests are executed at runtime. That's sort of what I'm doing in #77. Remember that the only reason this is a problem in the first place is because pytest errors when it sees unrecognized arguments. In this case instead of checking sys.argv, when a REQUIRES expression starts with env: it will check os.environ instead, which is a similar process, but pytest can't error because of unexpected environment variables.

I'm not sure if there is a better way to do it, or a different config that could be added that might be more intuitive. But I'm open to suggestions.

In the meantime, try using

pytest --xdoctest-global-exec="import sys; _ = None if '--web' in sys.argv else sys.argv.append('--web')"

which is an extension of the hack I had above, but won't make sys.argv crazy long. That will work with the current 0.13.0 release.

---

If you want to try #77, you can check it out and change all

>>> # xdoctest: +REQUIRES(--web)

to

>>> # xdoctest: +REQUIRES(env:WBIA_TEST_WEB)

and run

WBIA_TEST_WEB=1 pytest

(or something like that)

[karenc]

diff --git a/__main__.py b/__main__.py
index 7c2d859..67d1581 100644
--- a/__main__.py
+++ b/__main__.py
@@ -6,6 +6,7 @@ Provides a simple script for running module doctests.
 This should work even if the target module is unaware of xdoctest.
 """
 from __future__ import absolute_import, division, print_function, unicode_literals
+import re
 import sys
 
 
@@ -69,6 +70,7 @@ def main(argv=None):
 
     args, unknown = parser.parse_known_args(args=argv[1:])
     ns = args.__dict__.copy()
+    ns['flag'] = [re.sub('^-+', '', u) for u in unknown if u.startswith('-')]
 
     __DEBUG__ = '--debug' in sys.argv
     if __DEBUG__:
diff --git a/directive.py b/directive.py
index 6230ab5..8874183 100644
--- a/directive.py
+++ b/directive.py
@@ -232,19 +232,20 @@ class RuntimeState(utils.NiceRepr):
                 state[k] = False
         state['REPORT_' + reportchoice.upper()] = True
 
-    def update(self, directives):
+    def update(self, directives, config):
         """
         Update the runtime state given a set of directives
 
         Args:
             directives (List[Directive]): list of directives. The `effect`
                 method is used to update this object.
+            config: config from ini file or command line arguments
         """
         # Clear the previous inline state
         self._inline_state.clear()
         for directive in directives:
 
-            action, key, value = directive.effect()
+            action, key, value = directive.effect(config=config)
 
             if action == 'noop':
                 continue
@@ -364,7 +365,7 @@ class Directive(utils.NiceRepr):
                 'got {}'.format(self.name, num, nargs))
         return self.args
 
-    def effect(self, argv=None):
+    def effect(self, config=None):
         """
         Returns how this directive modifies a RuntimeState object
 
@@ -384,11 +385,11 @@ class Directive(utils.NiceRepr):
             Effect(action='assign', key='SKIP', value=True)
             >>> Directive('SKIP', inline=True).effect()
             Effect(action='assign', key='SKIP', value=True)
-            >>> Directive('REQUIRES', args=['-s']).effect(argv=['-s'])
+            >>> Directive('REQUIRES', args=['-s']).effect(config={'flag': ['s']})
             Effect(action='noop', key='REQUIRES', value=None)
-            >>> Directive('REQUIRES', args=['-s']).effect(argv=[])
+            >>> Directive('REQUIRES', args=['-s']).effect(config=None)
             Effect(action='set.add', key='REQUIRES', value='-s')
-            >>> Directive('ELLIPSIS', args=['-s']).effect(argv=[])
+            >>> Directive('ELLIPSIS', args=['-s']).effect(config=None)
             Effect(action='assign', key='ELLIPSIS', value=True)
 
         Doctest:
@@ -410,10 +411,8 @@ class Directive(utils.NiceRepr):
 
         if self.name == 'REQUIRES':
             # Special handling of REQUIRES
-            if argv is None:
-                argv = sys.argv
             arg, = self._unpack_args(1)
-            if _is_requires_satisfied(arg, argv):
+            if config and _is_requires_satisfied(arg, config.get('flag', [])):
                 # If the requirement is met, then do nothing,
                 action = 'noop'
             else:
@@ -437,7 +436,7 @@ class Directive(utils.NiceRepr):
         return Effect(action, key, value)
 
 
-def _is_requires_satisfied(arg, argv):
+def _is_requires_satisfied(arg, flags):
     """
     Determines if the argument to a REQUIRES directive is satisfied
 
@@ -464,7 +463,8 @@ def _is_requires_satisfied(arg, argv):
     PY_VER_TAGS = ['py2', 'py3']
 
     if arg.startswith('-'):
-        flag = arg in argv
+        # Remove the leading - from arg
+        flag = re.sub('^-+', '', arg) in flags
     elif arg.startswith('module:'):
         parts = arg.split(':')
         if len(parts) != 2:
diff --git a/doctest_example.py b/doctest_example.py
index 0dec694..2bf9e6d 100644
--- a/doctest_example.py
+++ b/doctest_example.py
@@ -57,6 +57,7 @@ class Config(dict):
             for optpart in directive_optstr.split(','):
                 directive = parse_directive_optstr(optpart)
                 default_runtime_state[directive.name] = directive.positive
+
         _examp_conf = {
             'default_runtime_state': default_runtime_state,
             'offset_linenos': ns['offset_linenos'],
@@ -64,6 +65,7 @@ class Config(dict):
             'reportchoice': ns['reportchoice'],
             'global_exec': ns['global_exec'],
             'verbose': ns['verbose'],
+            'flag': ns['flag'],
         }
         return _examp_conf
 
@@ -498,7 +500,7 @@ class DocTest(object):
         with warnings.catch_warnings(record=True) as self.warn_list:
             for partx, part in enumerate(self._parts):
                 # Extract directives and and update runtime state
-                runstate.update(part.directives)
+                runstate.update(part.directives, self.config)
 
                 # Handle runtime actions
                 if runstate['SKIP'] or len(runstate['REQUIRES']) > 0:
diff --git a/plugin.py b/plugin.py
index 0c0d4c9..ffdc258 100644
--- a/plugin.py
+++ b/plugin.py
@@ -103,6 +103,11 @@ def pytest_addoption(parser):
                     choices=['static', 'dynamic', 'auto'],
                     dest='xdoctest_analysis')
 
+    group.addoption('--xdoctest-flag', '--xdoc-flag',
+                    action='append', default=[], metavar='XDOCTEST_FLAG',
+                    help='Run tests marked with +REQUIRES(--XDOCTEST_FLAG)',
+                    dest='xdoctest_flag')
+
     from xdoctest import doctest_example
     doctest_example.Config()._update_argparse_cli(
         group.addoption, prefix=['xdoctest', 'xdoc'],
@@ -201,6 +206,8 @@ class _XDoctestBase(pytest.Module):
                 return self.config.getvalue('xdoctest_' + attr)
             def __getattr__(self, attr):
                 return self.config.getvalue('xdoctest_' + attr)
+            def get(self, attr, default=None):
+                return self.config.getvalue('xdoctest_' + attr, default)
 
         ns = NamespaceLike(self.config)
 

^ This works for me and doesn't modify sys.argv.

[karenc]

Thanks for your time and effort looking into this with me! I think I'll go with the conftest.py solution for now.

I just wanted to provide an alternative implementation and interface for doing this, and to understand xdoctest enough to do a proof of concept that it's possible. As the author of xdoctest, you probably have a vision for the program and may want to do something else. I'm ok with it as long as there's a way to do it (like adding to conftest.py) and it's documented.

[Erotemic]

Sounds good, closing the issue.

Also: the new version of xdoctest 0.14.0 was just released and contains support for the new environment variable based directives.