feat pytest: clean up CONFTEST_LOAD_POLICY_LOCAL implementation

* Add docs for `CONFTEST_LOAD_POLICY_LOCAL` and `CONFTEST_LOAD_POLICY_LEGACY_GLOBAL`
* Use `ya_` prefixes for custom fields of `LoadedModule` to differentiate from fields used by pytest
* Clean up `LoadedModule.__init__` parameters
* Move resfs modules lookup from `main.py` to `collection.py` to gather module name lookup and loading in the same place
* Turn `collection.py` into a plugin instead of using a class plugin. As a benefit, it is now displayed as `library.python.pytest.plugins.collection` in the plugin list (previously it was displayed as `<unnamed plugin>`)
commit_hash:be61134f075fbc9b645cc98635ab33fe7bf8a979

3 files changed, 96 insertions, 90 deletions

diff --git a/library/python/pytest/main.py b/library/python/pytest/main.py
index 5f7caa1725c..4d976b14df3 100644
--- a/library/python/pytest/main.py
+++ b/library/python/pytest/main.py
@@ -6,8 +6,6 @@ import yatest.common
 
 import yatest_lib.ya
 
-import __res
-
 FORCE_EXIT_TESTSFAILED_ENV = 'FORCE_EXIT_TESTSFAILED'
 
 
@@ -59,35 +57,6 @@ def main():
 
     yatest.common.runtime._set_ya_config(ya=yatest_lib.ya.Ya())
 
-    prefix = '__tests__.'
-
-    test_modules = [
-        # fmt: off
-        name
-        for name in sys.extra_modules
-        if name.startswith(prefix) and not name.endswith('.conftest')
-        # fmt: on
-    ]
-
-    doctest_packages = __res.find("PY_DOCTEST_PACKAGES") or ""
-    if isinstance(doctest_packages, bytes):
-        doctest_packages = doctest_packages.decode('utf-8')
-    doctest_packages = doctest_packages.split()
-
-    def is_doctest_module(name):
-        for package in doctest_packages:
-            if name == package or name.startswith(str(package) + "."):
-                return True
-        return False
-
-    doctest_modules = [
-        # fmt: off
-        name
-        for name in sys.extra_modules
-        if is_doctest_module(name)
-        # fmt: on
-    ]
-
     def remove_user_site(paths):
         site_paths = ('site-packages', 'site-python')
 
@@ -105,9 +74,10 @@ def main():
         return new_paths
 
     sys.path = remove_user_site(sys.path)
+
     rc = pytest.main(
         plugins=[
-            collection.CollectionPlugin(test_modules, doctest_modules),
+            collection,
             ya,
             conftests,
             fixtures,
diff --git a/library/python/pytest/plugins/collection.py b/library/python/pytest/plugins/collection.py
index e00d9292921..b3b316ade78 100644
--- a/library/python/pytest/plugins/collection.py
+++ b/library/python/pytest/plugins/collection.py
@@ -8,15 +8,16 @@ import py
 import pytest
 import _pytest.doctest
 import six
-from six import reraise
-
-if six.PY3:
-    import pathlib
 
 from library.python.pytest import module_utils
 import library.python.testing.filter.filter as test_filter
 import yatest.common
 
+import __res
+
+if six.PY3:
+    import pathlib
+
 
 def _make_module_with_single_skipped_test(module_name, file_name, exc):
     if exc.msg is not None:
@@ -37,20 +38,19 @@ def _make_module_with_single_skipped_test(module_name, file_name, exc):
 
 
 class LoadedModule(pytest.Module):
-    def __init__(self, parent, name, namespace=True, **kwargs):
-        self.module_name = name
-        if namespace:
-            assert name.startswith('__tests__.')
-            self.display_module_name = name[len('__tests__.') :]
+    def __init__(self, module_name, **kwargs):
+        self.ya_module_name = module_name
+        if module_name.startswith('__tests__.'):
+            self.ya_display_module_name = module_name[len('__tests__.') :]
         else:
-            self.display_module_name = name
-        self.is_fake_module = False
+            self.ya_display_module_name = module_name
         # Always passed to `super().__init__` explicitly.
         kwargs.pop('nodeid', None)
 
-        nodeid = module_utils.get_proper_module_path(self.module_name)
+        nodeid = module_utils.get_proper_module_path(self.ya_module_name)
+        self.ya_is_fake_module = False
         if nodeid is None:
-            self.is_fake_module = True
+            self.ya_is_fake_module = True
             return
 
         # Avoid specializing on `name`. It may change in the future if we add virtual collection nodes for directories.
@@ -62,10 +62,10 @@ class LoadedModule(pytest.Module):
 
         if six.PY3:
             kwargs['path'] = kwargs.get('path') or pathlib.Path(path)
-            super().__init__(parent=parent, name=name, nodeid=nodeid, **kwargs)
+            super().__init__(name=name, nodeid=nodeid, **kwargs)
         else:
             kwargs['fspath'] = kwargs.get('fspath') or py.path.local(path)
-            super(LoadedModule, self).__init__(parent=parent, nodeid=nodeid, **kwargs)
+            super(LoadedModule, self).__init__(nodeid=nodeid, **kwargs)
             self.name = name
 
     @classmethod
@@ -75,7 +75,7 @@ class LoadedModule(pytest.Module):
     def _getobj(self):
         # A simplified version of pytest `importtestmodule` that works with resfs.
         try:
-            __import__(self.module_name)
+            __import__(self.ya_module_name)
         except pytest.skip.Exception as e:
             if not e.allow_module_level:
                 raise RuntimeError(
@@ -83,14 +83,16 @@ class LoadedModule(pytest.Module):
                 )
             # DEVTOOLSSUPPORT-79470: reraising pytest.skip.Exception from here seems to break reporting in ya plugin.
             # Instead, pretend to be a module with a single skipped test.
-            return _make_module_with_single_skipped_test(self.module_name, self.nodeid, e)
+            return _make_module_with_single_skipped_test(self.ya_module_name, self.nodeid, e)
         except Exception as e:
-            msg = 'Failed to load module "{}" and obtain list of tests due to an error'.format(self.display_module_name)
+            msg = 'Failed to load module "{}" and obtain list of tests due to an error'.format(
+                self.ya_display_module_name
+            )
             logging.exception('%s: %s', msg, e)
             etype, exc, tb = sys.exc_info()
-            reraise(etype, type(exc)('{}\n{}'.format(exc, msg)), tb)
+            six.reraise(etype, type(exc)('{}\n{}'.format(exc, msg)), tb)
 
-        return sys.modules[self.module_name]
+        return sys.modules[self.ya_module_name]
 
 
 class DoctestModule(LoadedModule):
@@ -108,7 +110,7 @@ class DoctestModule(LoadedModule):
         runner = doctest.DebugRunner(verbose=0, optionflags=optionflags)
 
         try:
-            for test in finder.find(module, self.display_module_name):
+            for test in finder.find(module, self.ya_display_module_name):
                 if test.examples:  # skip empty doctests
                     yield getattr(_pytest.doctest.DoctestItem, 'from_parent', _pytest.doctest.DoctestItem)(
                         name=test.name, parent=self, runner=runner, dtest=test
@@ -117,18 +119,18 @@ class DoctestModule(LoadedModule):
             logging.exception('DoctestModule failed, probably you can add NO_DOCTESTS() macro to ya.make')
             etype, exc, tb = sys.exc_info()
             msg = 'DoctestModule failed, probably you can add NO_DOCTESTS() macro to ya.make'
-            reraise(etype, type(exc)('{}\n{}'.format(exc, msg)), tb)
+            six.reraise(etype, type(exc)('{}\n{}'.format(exc, msg)), tb)
 
 
 # NOTE: Since we are overriding collect method of pytest session, pytest hooks are not invoked during collection.
-# This function is only used in CollectionPlugin below, this is not an implementation of a pytest hook.
+# This is solely a helper function, not an implementation of a pytest hook.
 def _pytest_ignore_collect(module, config, filenames_from_full_filters, accept_filename_predicate):
-    if module.is_fake_module:
+    if module.ya_is_fake_module:
         return True
 
     # TODO refactor to use meaningful attributes like `path` instead?
     #  test_file_filter would need to be relative to repository root.
-    legacy_name = module.display_module_name + '.py'
+    legacy_name = module.ya_display_module_name + '.py'
 
     if config.option.mode == 'list':
         return not accept_filename_predicate(legacy_name)
@@ -163,47 +165,77 @@ def _patch_set_initial_conftests(pluginmanager):
         pluginmanager._set_initial_conftests = _set_initial_conftests_py2
 
 
-class CollectionPlugin(object):
-    def __init__(self, test_modules, doctest_modules):
-        self._test_modules = test_modules
-        self._doctest_modules = doctest_modules
+def _find_resfs_test_and_doctest_modules():
+    prefix = '__tests__.'
 
-    @pytest.hookimpl(tryfirst=True)
-    def pytest_load_initial_conftests(self, early_config):
-        _patch_set_initial_conftests(early_config.pluginmanager)
+    test_modules = [
+        # fmt: off
+        name
+        for name in sys.extra_modules
+        if name.startswith(prefix) and not name.endswith('.conftest')
+        # fmt: on
+    ]
 
-    def pytest_sessionstart(self, session):
+    doctest_packages = __res.find("PY_DOCTEST_PACKAGES") or ""
+    if isinstance(doctest_packages, bytes):
+        doctest_packages = doctest_packages.decode('utf-8')
+    doctest_packages = doctest_packages.split()
 
-        def collect(*args, **kwargs):
-            config = session.config
+    def is_doctest_module(name):
+        for package in doctest_packages:
+            if name == package or name.startswith(str(package) + "."):
+                return True
+        return False
 
-            # A custom function that is set in conftests.py.
-            config._ya_register_non_initial_conftests()
+    doctest_modules = [
+        # fmt: off
+        name
+        for name in sys.extra_modules
+        if is_doctest_module(name)
+        # fmt: on
+    ]
 
-            accept_filename_predicate = test_filter.make_py_file_filter(config.option.test_filter)
-            full_test_names_file_path = config.option.test_list_path
-            filenames_filter = None
+    return test_modules, doctest_modules
 
-            if full_test_names_file_path and os.path.exists(full_test_names_file_path):
-                with open(full_test_names_file_path, 'r') as afile:
-                    # in afile stored 2 dimensional array such that array[modulo_index] contains tests which should be run in this test suite
-                    full_names_filter = set(json.load(afile)[int(config.option.modulo_index)])
-                    filenames_filter = set(map(lambda x: x.split('::')[0], full_names_filter))
 
-            for test_module in self._test_modules:
-                module = LoadedModule.from_parent(name=test_module, parent=session)
-                if not _pytest_ignore_collect(module, config, filenames_filter, accept_filename_predicate):
-                    yield module
[email protected](tryfirst=True)
+def pytest_load_initial_conftests(early_config):
+    _patch_set_initial_conftests(early_config.pluginmanager)
+
 
-                if os.environ.get('YA_PYTEST_DISABLE_DOCTEST', 'no') == 'no':
-                    module = DoctestModule.from_parent(name=test_module, parent=session)
-                    if not _pytest_ignore_collect(module, config, filenames_filter, accept_filename_predicate):
-                        yield module
+def pytest_sessionstart(session):
+    resfs_test_modules, resfs_doctest_modules = _find_resfs_test_and_doctest_modules()
+
+    def collect(*args, **kwargs):
+        config = session.config
+
+        # A custom function that is set in conftests.py.
+        config._ya_register_non_initial_conftests()
+
+        accept_filename_predicate = test_filter.make_py_file_filter(config.option.test_filter)
+        full_test_names_file_path = config.option.test_list_path
+        filenames_filter = None
+
+        if full_test_names_file_path and os.path.exists(full_test_names_file_path):
+            with open(full_test_names_file_path, 'r') as afile:
+                # in afile stored 2 dimensional array such that array[modulo_index] contains tests which should be run in this test suite
+                full_names_filter = set(json.load(afile)[int(config.option.modulo_index)])
+                filenames_filter = set(map(lambda x: x.split('::')[0], full_names_filter))
+
+        for test_module in resfs_test_modules:
+            module = LoadedModule.from_parent(module_name=test_module, parent=session)
+            if not _pytest_ignore_collect(module, config, filenames_filter, accept_filename_predicate):
+                yield module
 
             if os.environ.get('YA_PYTEST_DISABLE_DOCTEST', 'no') == 'no':
-                for doctest_module in self._doctest_modules:
-                    module = DoctestModule.from_parent(name=doctest_module, parent=session, namespace=False)
-                    if not module.is_fake_module:
-                        yield module
+                module = DoctestModule.from_parent(module_name=test_module, parent=session)
+                if not _pytest_ignore_collect(module, config, filenames_filter, accept_filename_predicate):
+                    yield module
+
+        if os.environ.get('YA_PYTEST_DISABLE_DOCTEST', 'no') == 'no':
+            for doctest_module in resfs_doctest_modules:
+                module = DoctestModule.from_parent(module_name=doctest_module, parent=session)
+                if not module.ya_is_fake_module:
+                    yield module
 
-        session.collect = collect
+    session.collect = collect
diff --git a/library/python/pytest/plugins/conftests.py b/library/python/pytest/plugins/conftests.py
index 667874bb3d8..aa2915fcd59 100644
--- a/library/python/pytest/plugins/conftests.py
+++ b/library/python/pytest/plugins/conftests.py
@@ -99,6 +99,10 @@ def pytest_load_initial_conftests(early_config):
     extra_modules = getattr(sys, 'extra_modules', [])
     conftests = filter(lambda name: name == 'conftest' or name.endswith('.conftest'), extra_modules)
 
+    # CONFTEST_LOAD_POLICY is set by ya.make macros:
+    # * CONFTEST_LOAD_POLICY_LOCAL()
+    # * CONFTEST_LOAD_POLICY_LEGACY_GLOBAL()
+    # Current default is LEGACY_GLOBAL.
     if os.getenv('CONFTEST_LOAD_POLICY') == 'LOCAL':
         test_dir = str(yatest.common.context.project_path)