diff --git a/CHANGELOG.md b/CHANGELOG.md index be3f642a3..61f780de2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed * `import` now returns nil instead of the last module's string representation (#1174) * The `:decorators` key now works in `defn` when passed as a metadata name key, expanding its support to `defn`-derived macros like `defasync` (#1178). + * Change Python import machinery to be centered around Python code, rather than Basilisp namespaces (#1155, #1165) ### Fixed * Fix a bug in `defn` where the `attr-map?` and function metdata were merged into a seq instead of a map, causing `macroexpand` to fail in some cases (#1186) diff --git a/docs/testing.rst b/docs/testing.rst index aeee0beb8..e4df31b04 100644 --- a/docs/testing.rst +++ b/docs/testing.rst @@ -41,28 +41,59 @@ 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 +138,4 @@ You can see below that the fixture uses a :ref:`dynamic Var ` to c .. warning:: - Basilisp test fixtures are not related to PyTest fixtures and they cannot be used interchangeably. \ No newline at end of file + Basilisp test fixtures are not related to PyTest fixtures and they cannot be used interchangeably. diff --git a/src/basilisp/contrib/pytest/testrunner.py b/src/basilisp/contrib/pytest/testrunner.py index d2985e076..4f0cb23d8 100644 --- a/src/basilisp/contrib/pytest/testrunner.py +++ b/src/basilisp/contrib/pytest/testrunner.py @@ -57,9 +57,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,27 +175,21 @@ def _is_package(path: Path) -> bool: return False -def _get_fully_qualified_module_name(file: Path) -> str: +def _get_fully_qualified_module_names(file: Path) -> list[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) + paths = [] + for pth in sys.path: + root = Path(pth) + if file.is_relative_to(root): + elems = list(file.with_suffix("").relative_to(pth).parts) + if elems[-1] == "__init__": + elems.pop() + paths.append(".".join(elems)) + return paths class BasilispFile(pytest.File): @@ -251,10 +243,21 @@ def teardown(self) -> None: self._fixture_manager.teardown() def _import_module(self) -> runtime.BasilispModule: - modname = _get_fully_qualified_module_name(self.path) - module = importlib.import_module(modname) - assert isinstance(module, runtime.BasilispModule) - return module + modnames = _get_fully_qualified_module_names(self.path) + assert modnames, "Must have at least one module name" + + exc: Optional[ModuleNotFoundError] = None + for modname in modnames: + try: + module = importlib.import_module(modname) + except ModuleNotFoundError as e: + exc = e + else: + assert isinstance(module, runtime.BasilispModule) + return module + + assert exc is not None, "Must have an exception or module" + raise exc def collect(self): """Collect all tests from the namespace (module) given. diff --git a/src/basilisp/importer.py b/src/basilisp/importer.py index 22de90e15..6501ff183 100644 --- a/src/basilisp/importer.py +++ b/src/basilisp/importer.py @@ -297,7 +297,7 @@ def _exec_cached_module( fullname: str, loader_state: Mapping[str, str], path_stats: Mapping[str, int], - ns: runtime.Namespace, + module: BasilispModule, ) -> None: """Load and execute a cached Basilisp module.""" filename = loader_state["filename"] @@ -319,7 +319,7 @@ def _exec_cached_module( filename=filename, opts=runtime.get_compiler_opts() ), compiler.PythonASTOptimizer(), - ns, + module, ) def _exec_module( @@ -327,7 +327,7 @@ def _exec_module( fullname: str, loader_state: Mapping[str, str], path_stats: Mapping[str, int], - ns: runtime.Namespace, + module: BasilispModule, ) -> None: """Load and execute a non-cached Basilisp module.""" filename = loader_state["filename"] @@ -356,7 +356,7 @@ def _exec_module( compiler.CompilerContext( filename=filename, opts=runtime.get_compiler_opts() ), - ns, + module, collect_bytecode=all_bytecode.append, ) @@ -394,21 +394,29 @@ 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: 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. - if os.getenv(_NO_CACHE_ENVVAR, "").lower() == "true": - self._exec_module(fullname, spec.loader_state, path_stats, ns) - else: - try: - self._exec_cached_module(fullname, spec.loader_state, path_stats, ns) - except (EOFError, ImportError, OSError) as e: - logger.debug(f"Failed to load cached Basilisp module: {e}") - self._exec_module(fullname, spec.loader_state, path_stats, ns) + if fullname == runtime.CORE_NS: + ns: runtime.Namespace = runtime.Namespace.get_or_create(runtime.CORE_NS_SYM) + ns.module = module + module.__basilisp_namespace__ = ns + + # Set the currently importing module so it can be attached to the namespace when + # it is created. + with runtime.bindings( + {runtime.Var.find_safe(runtime.IMPORT_MODULE_VAR_SYM): module} + ): + + # Check if a valid, cached version of this Basilisp namespace exists and, if so, + # load it and bypass the expensive compilation process below. + if os.getenv(_NO_CACHE_ENVVAR, "").lower() == "true": + self._exec_module(fullname, spec.loader_state, path_stats, module) + else: + try: + self._exec_cached_module( + fullname, spec.loader_state, path_stats, module + ) + except (EOFError, ImportError, OSError) as e: + logger.debug(f"Failed to load cached Basilisp module: {e}") + self._exec_module(fullname, spec.loader_state, path_stats, module) def hook_imports() -> None: diff --git a/src/basilisp/lang/compiler/__init__.py b/src/basilisp/lang/compiler/__init__.py index a0e95b35d..7c5fcc968 100644 --- a/src/basilisp/lang/compiler/__init__.py +++ b/src/basilisp/lang/compiler/__init__.py @@ -37,6 +37,7 @@ from basilisp.lang.compiler.generator import statementize as _statementize from basilisp.lang.compiler.optimizer import PythonASTOptimizer from basilisp.lang.interfaces import ISeq +from basilisp.lang.runtime import BasilispModule from basilisp.lang.typing import CompilerOpts, ReaderForm from basilisp.lang.util import genname from basilisp.util import Maybe @@ -109,7 +110,6 @@ def compiler_opts( # pylint: disable=too-many-arguments def _emit_ast_string( - ns: runtime.Namespace, module: ast.AST, ) -> None: # pragma: no cover """Emit the generated Python AST string either to standard out or to the @@ -125,7 +125,7 @@ def _emit_ast_string( if runtime.print_generated_python(): print(to_py_str(module)) else: - runtime.add_generated_python(to_py_str(module), which_ns=ns) + runtime.add_generated_python(to_py_str(module)) def _flatmap_forms(forms: Iterable[ReaderForm]) -> Iterable[ReaderForm]: @@ -156,7 +156,7 @@ def compile_and_exec_form( return None if not ns.module.__basilisp_bootstrapped__: - _bootstrap_module(ctx.generator_context, ctx.py_ast_optimizer, ns) + _bootstrap_module(ctx.generator_context, ctx.py_ast_optimizer, ns.module) last = _sentinel for unrolled_form in _flatmap_forms([form]): @@ -181,7 +181,7 @@ def compile_and_exec_form( ast_module = ctx.py_ast_optimizer.visit(ast_module) ast.fix_missing_locations(ast_module) - _emit_ast_string(ns, ast_module) + _emit_ast_string(ast_module) bytecode = compile(ast_module, ctx.filename, "exec") if collect_bytecode: @@ -199,7 +199,7 @@ def compile_and_exec_form( def _incremental_compile_module( optimizer: PythonASTOptimizer, py_ast: GeneratedPyAST, - ns: runtime.Namespace, + module: BasilispModule, source_filename: str, collect_bytecode: Optional[BytecodeCollector] = None, ) -> None: @@ -213,39 +213,39 @@ def _incremental_compile_module( map(_statementize, itertools.chain(py_ast.dependencies, [py_ast.node])) ) - module = ast.Module(body=list(module_body), type_ignores=[]) - module = optimizer.visit(module) - ast.fix_missing_locations(module) + module_ast = ast.Module(body=list(module_body), type_ignores=[]) + module_ast = optimizer.visit(module_ast) + ast.fix_missing_locations(module_ast) - _emit_ast_string(ns, module) + _emit_ast_string(module_ast) - bytecode = compile(module, source_filename, "exec") + bytecode = compile(module_ast, source_filename, "exec") if collect_bytecode: collect_bytecode(bytecode) - exec(bytecode, ns.module.__dict__) # pylint: disable=exec-used # nosec 6102 + exec(bytecode, module.__dict__) # pylint: disable=exec-used # nosec 6102 def _bootstrap_module( gctx: GeneratorContext, optimizer: PythonASTOptimizer, - ns: runtime.Namespace, + module: BasilispModule, collect_bytecode: Optional[BytecodeCollector] = None, ) -> None: """Bootstrap a new module with imports and other boilerplate.""" _incremental_compile_module( optimizer, - py_module_preamble(ns), - ns, + py_module_preamble(), + module, source_filename=gctx.filename, collect_bytecode=collect_bytecode, ) - ns.module.__basilisp_bootstrapped__ = True + module.__basilisp_bootstrapped__ = True def compile_module( forms: Iterable[ReaderForm], ctx: CompilerContext, - ns: runtime.Namespace, + module: BasilispModule, collect_bytecode: Optional[BytecodeCollector] = None, ) -> None: """Compile an entire Basilisp module into Python bytecode which can be @@ -255,7 +255,7 @@ def compile_module( Basilisp import machinery, to allow callers to import Basilisp modules from Python code. """ - _bootstrap_module(ctx.generator_context, ctx.py_ast_optimizer, ns) + _bootstrap_module(ctx.generator_context, ctx.py_ast_optimizer, module) for form in _flatmap_forms(forms): nodes = gen_py_ast( @@ -264,7 +264,7 @@ def compile_module( _incremental_compile_module( ctx.py_ast_optimizer, nodes, - ns, + module, source_filename=ctx.filename, collect_bytecode=collect_bytecode, ) @@ -274,7 +274,7 @@ def compile_bytecode( code: list[types.CodeType], gctx: GeneratorContext, optimizer: PythonASTOptimizer, - ns: runtime.Namespace, + module: BasilispModule, ) -> None: """Compile cached bytecode into the given module. @@ -282,9 +282,9 @@ def compile_bytecode( namespaces. When the cached bytecode is reloaded from disk, it needs to be compiled within a bootstrapped module. This function bootstraps the module and then proceeds to compile a collection of bytecodes into the module.""" - _bootstrap_module(gctx, optimizer, ns) + _bootstrap_module(gctx, optimizer, module) for bytecode in code: - exec(bytecode, ns.module.__dict__) # pylint: disable=exec-used # nosec 6102 + exec(bytecode, module.__dict__) # pylint: disable=exec-used # nosec 6102 _LOAD_SYM = sym.symbol("load", ns=runtime.CORE_NS) diff --git a/src/basilisp/lang/compiler/generator.py b/src/basilisp/lang/compiler/generator.py index f775fe721..26b128b30 100644 --- a/src/basilisp/lang/compiler/generator.py +++ b/src/basilisp/lang/compiler/generator.py @@ -3916,13 +3916,13 @@ def gen_py_ast(ctx: GeneratorContext, lisp_ast: Node) -> GeneratedPyAST[ast.expr ############################# -def _module_imports(ns: runtime.Namespace) -> Iterable[ast.Import]: +def _module_imports() -> Iterable[ast.Import]: """Generate the Python Import AST node for importing all required language support modules.""" # Yield `import basilisp` so code attempting to call fully qualified # `basilisp.lang...` modules don't result in compiler errors yield ast.Import(names=[ast.alias(name="basilisp", asname=None)]) - for s in sorted(ns.imports.keys(), key=lambda s: s.name): + for s in sorted(runtime.Namespace.DEFAULT_IMPORTS, key=lambda s: s.name): name = s.name alias = _MODULE_ALIASES.get(name, None) yield ast.Import(names=[ast.alias(name=name, asname=alias)]) @@ -3970,10 +3970,10 @@ def _ns_var( ) -def py_module_preamble(ns: runtime.Namespace) -> GeneratedPyAST: +def py_module_preamble() -> GeneratedPyAST: """Bootstrap a new module with imports and other boilerplate.""" preamble: list[PyASTNode] = [] - preamble.extend(_module_imports(ns)) + preamble.extend(_module_imports()) preamble.extend(_from_module_imports()) preamble.append(_ns_var()) return GeneratedPyAST(node=ast.Constant(None), dependencies=preamble) diff --git a/src/basilisp/lang/runtime.py b/src/basilisp/lang/runtime.py index 9debe3a38..b8526b9a4 100644 --- a/src/basilisp/lang/runtime.py +++ b/src/basilisp/lang/runtime.py @@ -62,6 +62,8 @@ CORE_NS_SYM = sym.symbol(CORE_NS) NS_VAR_NAME = "*ns*" NS_VAR_SYM = sym.symbol(NS_VAR_NAME, ns=CORE_NS) +IMPORT_MODULE_VAR_NAME = "*import-module*" +IMPORT_MODULE_VAR_SYM = sym.symbol(IMPORT_MODULE_VAR_NAME, ns=CORE_NS) NS_VAR_NS = CORE_NS REPL_DEFAULT_NS = "basilisp.user" SUPPORTED_PYTHON_VERSIONS = frozenset({(3, 9), (3, 10), (3, 11), (3, 12), (3, 13)}) @@ -826,9 +828,15 @@ def __get_or_create( if ns is not None: return ns_cache new_ns = Namespace(name, module=module) - # The `ns` macro is important for setting up an new namespace, - # but it becomes available only after basilisp.core has been - # loaded. + + # If this is a new namespace and we're given a module (typically from the + # importer), attach the namespace to the module. + if module is not None: + logger.debug(f"Setting module '{module}' namespace to '{new_ns}'") + module.__basilisp_namespace__ = new_ns + + # The `ns` macro is important for setting up a new namespace, but it becomes + # available only after basilisp.core has been loaded. ns_var = Var.find_in_ns(CORE_NS_SYM, sym.symbol("ns")) if ns_var: new_ns.add_refer(sym.symbol("ns"), ns_var) @@ -842,6 +850,12 @@ def get_or_create( """Get the namespace bound to the symbol `name` in the global namespace cache, creating it if it does not exist. Return the namespace.""" + if ( + module is None + and (module_var := Var.find(IMPORT_MODULE_VAR_SYM)) is not None + ): + module = module_var.value + assert module is None or isinstance(module, BasilispModule) return cls._NAMESPACES.swap(Namespace.__get_or_create, name, module=module)[ name ] @@ -2399,6 +2413,17 @@ def in_ns(s: sym.Symbol): Var.intern( CORE_NS_SYM, sym.symbol("in-ns"), in_ns, meta=lmap.map({_REDEF_META_KEY: True}) ) + Var.intern( + CORE_NS_SYM, + sym.symbol(IMPORT_MODULE_VAR_NAME), + None, + dynamic=True, + meta=lmap.map( + { + _DOC_META_KEY: "If not ``nil``, corresponds to the module which is currently being imported." + } + ), + ) # Dynamic Var examined by the compiler when importing new Namespaces Var.intern( diff --git a/tests/basilisp/testrunner_test.py b/tests/basilisp/testrunner_test.py index ae6577a9e..8791543d6 100644 --- a/tests/basilisp/testrunner_test.py +++ b/tests/basilisp/testrunner_test.py @@ -261,3 +261,111 @@ 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): + runtime.Namespace.remove(sym.symbol("a.test-path")) + + 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 +): + runtime.Namespace.remove(sym.symbol("a.src")) + runtime.Namespace.remove(sym.symbol("a.test-path")) + + 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): + runtime.Namespace.remove(sym.symbol("a.test-path")) + + 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 'test.a'"]) + + +def test_ns_with_underscore(pytester: pytest.Pytester): + runtime.Namespace.remove(sym.symbol("test_underscore")) + + 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): + runtime.Namespace.remove(sym.symbol("abc")) + + 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( + [ + "*basilisp.lang.compiler.exception.CompilerException: unable to resolve symbol 'require'*" + ] + )