Refactor and simplify the testrunner (#495)

* Refactor and simplify the testrunner

* Changelog

Author: chrisrink10

CHANGELOG.md

@@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
  * Changed the Basilisp module type from `types.ModuleType` to a custom subtype with support for custom attributes (#482)
  * Basilisp's runtime function `Namespace.get_or_create` no longer refers `basilisp.core` by default, which allows callers to exclude `basilisp.core` names in the `ns` macro (#481)
  * Namespaces now use a single internal lock rather than putting each property inside of an Atom (#494)
+ * Refactor the testrunner to use fewer `atom`s in `basilisp.test` (#495)
 
 ### Fixed
  * Fixed a reader bug where no exception was being thrown splicing reader conditional forms appeared outside of valid splicing contexts (#470)

src/basilisp/importer.py

@@ -14,7 +14,8 @@
 import basilisp.lang.reader as reader
 import basilisp.lang.runtime as runtime
 import basilisp.lang.symbol as sym
-from basilisp.lang.typing import BasilispModule, ReaderForm
+from basilisp.lang.runtime import BasilispModule
+from basilisp.lang.typing import ReaderForm
 from basilisp.lang.util import demunge
 from basilisp.util import timed
 
@@ -275,10 +276,7 @@ def _exec_module(
             # During compilation, bytecode objects are added to the list via the closure
             # add_bytecode below, which is passed to the compiler. The collected bytecodes
             # will be used to generate an .lpyc file for caching the compiled file.
-            all_bytecode = []
-
-            def add_bytecode(bytecode: types.CodeType):
-                all_bytecode.append(bytecode)
+            all_bytecode: List[types.CodeType] = []
 
             logger.debug(f"Reading and compiling Basilisp module '{fullname}'")
             # Cast to basic ReaderForm since the reader can never return a reader conditional
@@ -292,7 +290,7 @@ def add_bytecode(bytecode: types.CodeType):
                 forms,
                 compiler.CompilerContext(filename=filename),
                 ns,
-                collect_bytecode=add_bytecode,
+                collect_bytecode=all_bytecode.append,
             )
 
         # Cache the bytecode that was collected through the compilation run.
@@ -324,6 +322,7 @@ def exec_module(self, module):
         ns_name = demunge(fullname)
         ns: runtime.Namespace = runtime.Namespace.get_or_create(sym.symbol(ns_name))
         ns.module = module
+        module.__basilisp_namespace__ = ns
 
         # Check if a valid, cached version of this Basilisp namespace exists and, if so,
         # load it and bypass the expensive compilation process below.
@@ -351,7 +350,7 @@ def hook_imports():
 
     Once this is called, Basilisp code may be called from within Python code
     using standard `import module.submodule` syntax."""
-    if any([isinstance(o, BasilispImporter) for o in sys.meta_path]):
+    if any(isinstance(o, BasilispImporter) for o in sys.meta_path):
         return
     sys.meta_path.insert(
         0, BasilispImporter()  # pylint:disable=abstract-class-instantiated

src/basilisp/lang/runtime.py

@@ -47,7 +47,7 @@
     ISeqable,
 )
 from basilisp.lang.reference import ReferenceBase
-from basilisp.lang.typing import BasilispModule, LispNumber
+from basilisp.lang.typing import LispNumber
 from basilisp.lang.util import munge
 from basilisp.logconfig import TRACE
 from basilisp.util import Maybe
@@ -122,6 +122,11 @@
 CompletionTrimmer = Callable[[Tuple[sym.Symbol, Any]], str]
 
 
+class BasilispModule(types.ModuleType):
+    __basilisp_namespace__: "Namespace"
+    __basilisp_bootstrapped__: bool = False
+
+
 def _new_module(name: str, doc=None) -> BasilispModule:
     """Create a new empty Basilisp Python module.
     Modules are created for each Namespace when it is created."""

src/basilisp/lang/typing.py

@@ -2,7 +2,6 @@
 from datetime import datetime
 from decimal import Decimal
 from fractions import Fraction
-from types import ModuleType
 from typing import Pattern, Union
 
 import basilisp.lang.keyword as kw
@@ -37,7 +36,3 @@
 PyCollectionForm = Union[dict, list, set, tuple]
 ReaderForm = Union[LispForm, IRecord, ISeq, IType, PyCollectionForm]
 SpecialForm = Union[llist.List, ISeq]
-
-
-class BasilispModule(ModuleType):
-    __basilisp_bootstrapped__: bool = False

src/basilisp/test.lpy

@@ -4,16 +4,8 @@
   (:require
    [basilisp.core.template :as template]))
 
-(def ^:private collected-tests
-  (atom []))
-
-(def ^:private current-ns
-  (atom nil))
-
-(defn ^:private add-test!
-  "Add the test named by test-var to the test suite for ns."
-  [test-var]
-  (swap! collected-tests conj test-var))
+(def ^:private current-test-number
+  (atom 0))
 
 (def ^:dynamic *test-name* nil)
 (def ^:dynamic *test-section* nil)
@@ -134,18 +126,18 @@
   Tests defined by deftest will be run by default by the PyTest test
   runner using Basilisp's builtin PyTest hook."
   [name-sym & body]
-  (let [test-name-sym (vary-meta name-sym assoc :test true)
+  (let [test-num      (swap! current-test-number inc)
+        test-name-sym (vary-meta name-sym
+                                 assoc
+                                 :basilisp.test/test true
+                                 :basilisp.test/order test-num)
         test-name-str (name test-name-sym)
         test-ns-name  `(quote ~(symbol (name *ns*)))]
-    `(do
-       (defn ~test-name-sym
-         []
-         (binding [*ns*            (the-ns ~test-ns-name)
-                   *test-name*     ~test-name-str
-                   *test-section*  nil
-                   *test-failures* (atom [])]
-           ~@body
-           {:failures (deref *test-failures*)}))
-
-       (add-test! (var ~test-name-sym))
-       (reset! current-ns (the-ns ~test-ns-name)))))
+    `(defn ~test-name-sym
+       []
+       (binding [*ns*            (the-ns ~test-ns-name)
+                 *test-name*     ~test-name-str
+                 *test-section*  nil
+                 *test-failures* (atom [])]
+         ~@body
+         {:failures (deref *test-failures*)}))))

src/basilisp/testrunner.py

@@ -1,6 +1,6 @@
 import importlib
 import traceback
-from typing import Callable, Optional
+from typing import Callable, Iterable, Optional
 
 import pytest
 
@@ -13,8 +13,9 @@
 from basilisp.lang.obj import lrepr
 from basilisp.util import Maybe
 
-_COLLECTED_TESTS_SYM = sym.symbol("collected-tests", ns="basilisp.test")
 _CURRENT_NS_SYM = sym.symbol("current-ns", ns="basilisp.test")
+_TEST_META_KW = kw.keyword("test", "basilisp.test")
+_TEST_NUM_META_KW = kw.keyword("order", "basilisp.test")
 
 
 # pylint: disable=unused-argument
@@ -31,37 +32,6 @@ def pytest_collect_file(parent, path):
     return None
 
 
-def _collected_tests() -> Optional[vec.Vector]:
-    """Fetch the collected tests for the namespace ns from
-    basilisp.test/collected-tests atom. If no tests are found, return
-    None."""
-    var = Maybe(runtime.Var.find(_COLLECTED_TESTS_SYM)).or_else_raise(
-        lambda: runtime.RuntimeException(
-            f"Unable to find test Var {_COLLECTED_TESTS_SYM}."
-        )
-    )
-    return var.value.deref()
-
-
-def _current_ns() -> str:
-    """Fetch the current namespace from basilisp.test/current-ns."""
-    var = Maybe(runtime.Var.find(_CURRENT_NS_SYM)).or_else_raise(
-        lambda: runtime.RuntimeException(f"Unable to find test Var {_CURRENT_NS_SYM}.")
-    )
-    ns = var.value.deref()
-    return ns.name
-
-
-def _reset_collected_tests() -> None:
-    """Reset the collected tests."""
-    var = Maybe(runtime.Var.find(_COLLECTED_TESTS_SYM)).or_else_raise(
-        lambda: runtime.RuntimeException(
-            f"Unable to find test Var {_COLLECTED_TESTS_SYM}."
-        )
-    )
-    return var.value.reset(vec.Vector.empty())
-
-
 class TestFailuresInfo(Exception):
     __slots__ = ("_msg", "_data")
 
@@ -91,21 +61,43 @@ def message(self) -> str:
 class BasilispFile(pytest.File):
     """Files represent a test module in Python or a test namespace in Basilisp."""
 
+    @staticmethod
+    def _collected_tests(ns: runtime.Namespace) -> Iterable[runtime.Var]:
+        """Return the set of collected tests from the Namespace `ns`.
+
+        Tests defined by `deftest` are annotated with `:basilisp.test/test` metadata
+        and `:basilisp.test/order` is a monotonically increasing integer added by
+        `deftest` at compile-time to run tests in the order they are defined (which
+        matches the default behavior of PyTest)."""
+
+        def _test_num(var: runtime.Var) -> int:
+            assert var.meta is not None
+            order = var.meta.val_at(_TEST_NUM_META_KW)
+            assert isinstance(order, int)
+            return order
+
+        return sorted(
+            (
+                var
+                for _, var in ns.interns.items()
+                if var.meta is not None and var.meta.val_at(_TEST_META_KW)
+            ),
+            key=_test_num,
+        )
+
     def collect(self):
         """Collect all of the tests in the namespace (module) given.
 
         Basilisp's test runner imports the namespace which will (as a side
         effect) collect all of the test functions in a namespace (represented
-        by `deftest` forms in Basilisp) into an atom in `basilisp.test`.
-        BasilispFile.collect fetches those test functions and generates
-        BasilispTestItems for PyTest to run the tests."""
-        _reset_collected_tests()
+        by `deftest` forms in Basilisp). BasilispFile.collect fetches those
+        test functions and generates BasilispTestItems for PyTest to run the
+        tests."""
         filename = self.fspath.basename
-        self.fspath.pyimport()
-        ns = _current_ns()
-        tests = _collected_tests()
-        assert tests is not None, "Must have collected tests"
-        for test in tests:
+        module = self.fspath.pyimport()
+        assert isinstance(module, runtime.BasilispModule)
+        ns = module.__basilisp_namespace__
+        for test in self._collected_tests(ns):
             f: TestFunction = test.value
             yield BasilispTestItem(test.name.name, self, f, ns, filename)
 
@@ -137,7 +129,7 @@ def __init__(  # pylint: disable=too-many-arguments
         name: str,
         parent: BasilispFile,
         run_test: TestFunction,
-        namespace: str,
+        namespace: runtime.Namespace,
         filename: str,
     ) -> None:
         super(BasilispTestItem, self).__init__(name, parent)