Merge pull request #2474 from vkarak/refactor/fixture-instantiation

[refactor] Refactor fixture instantiation

Author: Vasileios Karakasis

reframe/core/decorators.py

@@ -22,11 +22,16 @@
 import reframe.core.warnings as warn
 import reframe.core.hooks as hooks
 from reframe.core.exceptions import ReframeSyntaxError, SkipTestError, what
+from reframe.core.fixtures import FixtureRegistry
 from reframe.core.logging import getlogger
 from reframe.core.pipeline import RegressionTest
 from reframe.utility.versioning import VersionValidator
 
 
+# NOTE: we should consider renaming this module in 4.0; it practically takes
+# care of the registration and instantiation of the tests.
+
+
 class TestRegistry:
     '''Regression test registry.
 
@@ -61,14 +66,20 @@ def skip(self, test):
 
     def instantiate_all(self):
         '''Instantiate all the registered tests.'''
-        ret = []
+
+        # We first instantiate the leaf tests and then walk up their
+        # dependencies to instantiate all the fixtures. Fixtures can only
+        # establish their exact dependencies at instantiation time, so the
+        # dependency graph grows dynamically.
+
+        leaf_tests = []
         for test, variants in self._tests.items():
             if test in self._skip_tests:
                 continue
 
             for args, kwargs in variants:
                 try:
-                    ret.append(test(*args, **kwargs))
+                    leaf_tests.append(test(*args, **kwargs))
                 except SkipTestError as e:
                     getlogger().warning(
                         f'skipping test {test.__qualname__!r}: {e}'
@@ -82,7 +93,30 @@ def instantiate_all(self):
                     )
                     getlogger().verbose(traceback.format_exc())
 
-        return ret
+        # Instantiate fixtures
+
+        # Do a level-order traversal of the fixture registries of all leaf
+        # tests, instantiate all fixtures and generate the final set of
+        # candidate tests; the leaf tests are consumed at the end of the
+        # traversal and all instantiated tests (including fixtures) are stored
+        # in `final_tests`.
+        final_tests = []
+        fixture_registry = FixtureRegistry()
+        while leaf_tests:
+            tmp_registry = FixtureRegistry()
+            while leaf_tests:
+                c = leaf_tests.pop()
+                reg = getattr(c, '_rfm_fixture_registry', None)
+                final_tests.append(c)
+                if reg:
+                    tmp_registry.update(reg)
+
+            # Instantiate the new fixtures and update the registry
+            new_fixtures = tmp_registry.difference(fixture_registry)
+            leaf_tests = new_fixtures.instantiate_all()
+            fixture_registry.update(new_fixtures)
+
+        return final_tests
 
     def __iter__(self):
         '''Iterate over the registered test classes.'''

reframe/frontend/loader.py

@@ -18,7 +18,6 @@
 import reframe.utility.osext as osext
 from reframe.core.exceptions import NameConflictError, is_severe, what
 from reframe.core.logging import getlogger
-from reframe.core.fixtures import FixtureRegistry
 
 
 class RegressionCheckValidator(ast.NodeVisitor):
@@ -171,7 +170,7 @@ def load_from_module(self, module):
             return []
 
         self._set_defaults(registry)
-        test_pool = registry.instantiate_all() if registry else []
+        candidate_tests = registry.instantiate_all() if registry else []
         legacy_tests = legacy_registry() if legacy_registry else []
         if self._external_vars and legacy_tests:
             getlogger().warning(
@@ -180,32 +179,11 @@ def load_from_module(self, module):
                 "please use the 'parameter' builtin in your tests"
             )
 
-        # Merge registries
-        test_pool += legacy_tests
-
-        # Do a level-order traversal of the fixture registries of all tests in
-        # the test pool, instantiate all fixtures and generate the final set
-        # of candidate tests to load; the test pool is consumed at the end of
-        # the traversal and all instantiated tests (including fixtures) are
-        # stored in `candidate_tests`.
-        candidate_tests = []
-        fixture_registry = FixtureRegistry()
-        while test_pool:
-            tmp_registry = FixtureRegistry()
-            while test_pool:
-                c = test_pool.pop()
-                reg = getattr(c, '_rfm_fixture_registry', None)
-                candidate_tests.append(c)
-                if reg:
-                    tmp_registry.update(reg)
-
-            # Instantiate the new fixtures and update the registry
-            new_fixtures = tmp_registry.difference(fixture_registry)
-            test_pool = new_fixtures.instantiate_all()
-            fixture_registry.update(new_fixtures)
+        # Merge tests
+        candidate_tests += legacy_tests
 
         # Post-instantiation validation of the candidate tests
-        tests = []
+        final_tests = []
         for c in candidate_tests:
             if not isinstance(c, RegressionTest):
                 continue
@@ -218,15 +196,15 @@ def load_from_module(self, module):
                 conflicted = self._loaded[c.unique_name]
             except KeyError:
                 self._loaded[c.unique_name] = testfile
-                tests.append(c)
+                final_tests.append(c)
             else:
                 raise NameConflictError(
                     f'test {c.unique_name!r} from {testfile!r} '
                     f'is already defined in {conflicted!r}'
                 )
 
-        getlogger().debug(f'  > Loaded {len(tests)} test(s)')
-        return tests
+        getlogger().debug(f'  > Loaded {len(final_tests)} test(s)')
+        return final_tests
 
     def load_from_file(self, filename, force=False):
         if not self._validate_source(filename):