Derive module (test) names from ns declarations

CHANGELOG.md

@@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
  * `alter-var-root` now returns the new value to align with Clojure behavior. Updated the docstring to highlight side effects of direct linking optimization (#1166)
+ * The test runner now supports loading test namespaces from any directory relative to `sys.path` without requiring any `__init__.py` files (#1155)
+
+### Fixed
+ * Fix an issue where test runner couldn't load test namespaces with underscores in their names (#1165)
 
 ## [v0.3.4]
 ### Added

docs/testing.rst

@@ -41,28 +41,56 @@ For asserting repeatedly against different inputs, you can use the :lpy:fn:`are`
 Testing and ``PYTHONPATH``
 --------------------------
 
-Typical Clojure projects will have parallel ``src/`` and ``tests/`` folders in the project root.
+Typical Clojure projects will have parallel ``src/`` and ``test/`` folders in the project root.
 Project management tooling typically constructs the Java classpath to include both parallel trees for development and only ``src/`` for deployed software.
-Basilisp does not currently have such tooling (though it is planned!) and the recommended Python tooling is not configurable to allow for this distinction.
+Basilisp does not currently have such tooling, though it is planned.
 
-Due to this limitation, the easiest solution to facilitate test discovery with Pytest (Basilisp's default test runner) is to include a single, empty ``__init__.py`` file in the top-level ``tests`` directory:
+The easiest solution to facilitate test discovery with Pytest (Basilisp's default test runner) is to create a ``tests`` directory:
 
 .. code-block:: text
 
    tests
-   ├── __init__.py
    └── myproject
        └── core_test.lpy
 
 Test namespaces can then be created as if they are part of a giant ``tests`` package:
 
-.. code-block::
+.. code-block:: clojure
 
    (ns tests.myproject.core-test)
 
+Tests can be run with:
+
+.. code-block:: shell
+
+   $ basilisp test
+
+----
+
+Alternatively, you can follow the more traditional Clojure project structure by creating a `test` directory for your test namespaces:
+
+.. code-block:: text
+
+   test
+   └── myproject
+       └── core_test.lpy
+
+In this case, the test namespace can start at ``myproject``:
+
+.. code-block:: clojure
+
+   (ns myproject.core-test)
+
+
+However, the ``test`` directory must be explicitly added to the `PYTHONPATH` using the ``--include-path`` (or ``-p``) option when running the tests:
+
+.. code-block:: shell
+
+   $ basilisp test --include-path test
+
 .. note::
 
-   The project maintainers acknowledge that this is not an ideal solution and would like to provide a more Clojure-like solution in the future.
+   Test directory names can be arbitrary. By default, the test runner searches all subdirectories for tests. In the first example above (``tests``, a Python convention), the top-level directory is already in the `PYTHONPATH`, allowing ``tests.myproject.core-test`` to be resolvable. In the second example (``test``, a Clojure convention), the test directory is explicitly added to the `PYTHONPATH`, enabling ``myproject.core-test`` to be resolvable.
 
 .. _test_fixtures:
 
@@ -107,4 +135,4 @@ You can see below that the fixture uses a :ref:`dynamic Var <dynamic_vars>` to c
 
 .. warning::
 
-   Basilisp test fixtures are not related to PyTest fixtures and they cannot be used interchangeably.
+   Basilisp test fixtures are not related to PyTest fixtures and they cannot be used interchangeably.

src/basilisp/contrib/pytest/testrunner.py

@@ -11,12 +11,14 @@
 import pytest
 
 from basilisp import main as basilisp
+from basilisp.importer import read_namespace_name
 from basilisp.lang import keyword as kw
 from basilisp.lang import map as lmap
 from basilisp.lang import runtime as runtime
 from basilisp.lang import symbol as sym
 from basilisp.lang import vector as vec
 from basilisp.lang.obj import lrepr
+from basilisp.lang.util import munge
 from basilisp.util import Maybe
 
 _EACH_FIXTURES_META_KW = kw.keyword("each-fixtures", "basilisp.test")
@@ -57,9 +59,7 @@ def pytest_unconfigure(config):
         runtime.pop_thread_bindings()
 
 
-def pytest_collect_file(  # pylint: disable=unused-argument
-    file_path: Path, path, parent
-):
+def pytest_collect_file(file_path: Path, parent):
     """Primary PyTest hook to identify Basilisp test files."""
     if file_path.suffix == ".lpy":
         if file_path.name.startswith("test_") or file_path.stem.endswith("_test"):
@@ -177,29 +177,6 @@ def _is_package(path: Path) -> bool:
     return False
 
 
-def _get_fully_qualified_module_name(file: Path) -> str:
-    """Return the fully qualified module name (from the import root) for a module given
-    its location.
-
-    This works by traversing up the filesystem looking for the top-most package. From
-    there, we derive a Python module name referring to the given module path."""
-    top = None
-    for p in file.parents:
-        if _is_package(p):
-            top = p
-        else:
-            break
-
-    if top is None or top == file.parent:
-        return file.stem
-
-    root = top.parent
-    elems = list(file.with_suffix("").relative_to(root).parts)
-    if elems[-1] == "__init__":
-        elems.pop()
-    return ".".join(elems)
-
-
 class BasilispFile(pytest.File):
     """Files represent a test module in Python or a test namespace in Basilisp."""
 
@@ -251,7 +228,18 @@ def teardown(self) -> None:
         self._fixture_manager.teardown()
 
     def _import_module(self) -> runtime.BasilispModule:
-        modname = _get_fully_qualified_module_name(self.path)
+        """Import the Basilisp module at `self.path` and return it.
+
+        Raises ImportError if the Basilisp module does not declare a
+        namespace name.
+
+        """
+        ns_name = read_namespace_name(self.path)
+        if ns_name is not None:
+            modname = munge(ns_name)
+        else:
+            raise ImportError(f"Can't find Basilisp namespace name in {self.path}")
+
         module = importlib.import_module(modname)
         assert isinstance(module, runtime.BasilispModule)
         return module

src/basilisp/importer.py

@@ -9,11 +9,13 @@
 from functools import lru_cache
 from importlib.abc import MetaPathFinder, SourceLoader
 from importlib.machinery import ModuleSpec
+from pathlib import Path
 from typing import Any, Optional, cast
 
 from typing_extensions import TypedDict
 
 from basilisp.lang import compiler as compiler
+from basilisp.lang import list as lst
 from basilisp.lang import reader as reader
 from basilisp.lang import runtime as runtime
 from basilisp.lang import symbol as sym
@@ -127,6 +129,23 @@ def _is_namespace_package(path: str) -> bool:
     return no_inits and has_basilisp_files
 
 
+def read_namespace_name(file: Path) -> Optional[str]:
+    """Read the top-level `ns` form from Basilisp `file` and return
+    its name. Return None if the the top sexp is not a valid `ns`
+    form.
+
+    """
+    ns_form = next(iter(reader.read_file(str(file))))
+    if (
+        (isinstance(ns_form, lst.PersistentList) and len(ns_form) > 1)
+        and str(ns_form[0]) == "ns"
+        and isinstance(ns_form[1], sym.Symbol)
+    ):
+        return ns_form[1].name
+    else:
+        return None
+
+
 class ImporterCacheEntry(TypedDict, total=False):
     spec: ModuleSpec
     module: BasilispModule
@@ -155,7 +174,6 @@ def find_spec(
             module_name = package_components
         else:
             module_name = [package_components[-1]]
-
         for entry in path:
             root_path = os.path.join(entry, *module_name)
             filenames = [
@@ -394,7 +412,7 @@ def exec_module(self, module: types.ModuleType) -> None:
         # a blank module. If we do not replace the module here with the module we are
         # generating, then we will not be able to use advanced compilation features such
         # as direct Python variable access to functions and other def'ed values.
-        ns_name = demunge(fullname)
+        ns_name = name if (name := read_namespace_name(filename)) else demunge(fullname)
         ns: runtime.Namespace = runtime.Namespace.get_or_create(sym.symbol(ns_name))
         ns.module = module
         module.__basilisp_namespace__ = ns

tests/basilisp/testrunner_test.py

@@ -261,3 +261,107 @@ def test_fixtures_with_errors(
     pytester.syspathinsert()
     result: pytest.RunResult = pytester.runpytest()
     result.assert_outcomes(passed=passes, failed=failures, errors=errors)
+
+
+def test_ns_in_syspath(pytester: pytest.Pytester, monkeypatch: pytest.MonkeyPatch):
+    code = """
+    (ns a.test-path
+      (:require
+       [basilisp.test :refer [deftest is]]))
+
+    (deftest passing-test
+      (is true))
+
+    (deftest failing-test
+      (is false))
+    """
+    pytester.makefile(".lpy", **{"./test/a/test_path": code})
+    pytester.syspathinsert()
+    # ensure `a` namespace is in sys.path
+    monkeypatch.syspath_prepend(pytester.path / "test")
+    result: pytest.RunResult = pytester.runpytest("test")
+    result.assert_outcomes(passed=1, failed=1)
+
+
+def test_ns_in_syspath_w_src(
+    pytester: pytest.Pytester, monkeypatch: pytest.MonkeyPatch
+):
+    code_src = """
+    (ns a.src)
+    (def abc 5)
+    """
+
+    code = """
+    (ns a.test-path
+      (:require
+       a.src
+       [basilisp.test :refer [deftest is]]))
+
+    (deftest a-test (is (= a.src/abc 5)))
+
+    (deftest passing-test
+      (is true))
+
+    (deftest failing-test
+      (is false))
+    """
+    # a slightly more complicated setup where packages under namespace
+    # `a` are both in src and test.
+    pytester.makefile(".lpy", **{"./test/a/test_path": code, "./src/a/src": code_src})
+    pytester.syspathinsert()
+    # ensure src and test is in sys.path
+    monkeypatch.syspath_prepend(pytester.path / "test")
+    monkeypatch.syspath_prepend(pytester.path / "src")
+    result: pytest.RunResult = pytester.runpytest("test")
+    result.assert_outcomes(passed=2, failed=1)
+
+
+def test_ns_not_in_syspath(pytester: pytest.Pytester):
+    code = """
+    (ns a.test-path
+      (:require
+       [basilisp.test :refer [deftest is]]))
+    """
+    pytester.makefile(".lpy", **{"./test/a/test_path": code})
+    pytester.syspathinsert()
+    result: pytest.RunResult = pytester.runpytest("test")
+    assert result.ret != 0
+    result.stdout.fnmatch_lines(["*ModuleNotFoundError: No module named 'a'"])
+
+
+def test_ns_with_underscore(pytester: pytest.Pytester):
+    code = """
+    (ns test_underscore
+      (:require
+       [basilisp.test :refer [deftest is]]))
+
+    (deftest passing-test
+      (is true))
+
+    (deftest failing-test
+      (is false))
+    """
+    pytester.makefile(".lpy", test_underscore=code)
+    pytester.syspathinsert()
+    result: pytest.RunResult = pytester.runpytest()
+    result.assert_outcomes(passed=1, failed=1)
+
+
+def test_no_ns(pytester: pytest.Pytester):
+    code = """
+    (in-ns 'abc)
+    (require '[basilisp.test :refer [deftest is]]))
+
+    (deftest passing-test
+      (is true))
+
+    (deftest failing-test
+      (is false))
+    """
+    pytester.makefile(".lpy", test_under=code)
+    pytester.syspathinsert()
+    result: pytest.RunResult = pytester.runpytest()
+    assert result.ret != 0
+    result.stdout.fnmatch_lines(
+        ["*ImportError: Can't find Basilisp namespace name in*"]
+    )