Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed
* Fixed a regression introduced in #1176 where the testrunner couldn't handle relative paths in `sys.path`, causing `basilisp test` to fail when no arugments were provided (#1204)

## [v0.3.6]
### Added
* Added support for the `:decorators` meta key in anonymous `fn`s (#1178)
Expand Down
6 changes: 4 additions & 2 deletions src/basilisp/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,7 @@ def test(
"Cannot run tests without dependency PyTest. Please install PyTest and try again.",
)
else:
pytest.main(args=list(extra))
sys.exit(pytest.main(args=list(extra)))


@_subcommand(
Expand All @@ -768,7 +768,9 @@ def test(
If all options are unambiguous (e.g. they are only either used by Basilisp
or by PyTest), then you can omit the `--`:

`basilisp test -k vector -p other_dir`"""
`basilisp test -k vector -p other_dir`

Returns the PyTest exit code as the exit code."""
),
handler=test,
allows_extra=True,
Expand Down
12 changes: 10 additions & 2 deletions src/basilisp/contrib/pytest/testrunner.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
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")
Expand Down Expand Up @@ -183,9 +184,10 @@ def _get_fully_qualified_module_names(file: Path) -> list[str]:
there, we derive a Python module name referring to the given module path."""
paths = []
for pth in sys.path:
root = Path(pth)
root = Path(pth).resolve()
if file.is_relative_to(root):
elems = list(file.with_suffix("").relative_to(pth).parts)
elems = list(file.with_suffix("").relative_to(root).parts)

if elems[-1] == "__init__":
elems.pop()
paths.append(".".join(elems))
Expand Down Expand Up @@ -269,6 +271,12 @@ def collect(self):
filename = self.path.name
module = self._import_module()
ns = module.__basilisp_namespace__

# Ensure the test module was loaded because it was directly
# relative to an entry in `sys.path`.
if module.__name__ != munge(str(ns)):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this cause problems with the changes introduced by #1176 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and in particular it was breaking the test_ns_not_in_syspath test.

Consider the following file relative to the project directory:

./test/a/test_path.lpy, declared with the namespace a.test-path.

Running basilisp test will locate and load a.test-path due to "" in sys.path, but this is inconsistent because the namespace doesn't match the load path (it’s not test.a.test-path). Thus, an exception is thrown in this case.

(To make this work, the user must add the -p test option to the cli tool)

raise ModuleNotFoundError(f"Module named '{ns}' is not in sys.path")

once_fixtures, each_fixtures = self._collected_fixtures(ns)
self._fixture_manager = FixtureManager(once_fixtures)
for test in self._collected_tests(ns):
Expand Down
39 changes: 36 additions & 3 deletions tests/basilisp/testrunner_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import platform
import shutil
import subprocess
import sys

import pytest
Expand Down Expand Up @@ -263,6 +265,30 @@ def test_fixtures_with_errors(
result.assert_outcomes(passed=passes, failed=failures, errors=errors)


def test_basilisp_test_noargs(pytester: pytest.Pytester):
runtime.Namespace.remove(sym.symbol("a.test-path"))

code = """
(ns tests.test-path
(:require
[basilisp.test :refer [deftest is]]))
(deftest passing-test
(is true))
"""
pytester.makefile(".lpy", **{"./tests/test_path": code})

# I couldn't find a way to directly manipulate the pytester's
# `sys.path` with the precise control needed by this test, so we're
# invoking `basilisp test` directly as a subprocess instead ...
basilisp = shutil.which("basilisp")
cmd = [basilisp, "test"]
result = subprocess.run(cmd, capture_output=True, text=True, cwd=pytester.path)
Comment on lines +280 to +285
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it not use the same sys.path as the running executable? By which I mean are you not able to monkeypatch sys.path to the value you need?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, there's no way to replicate the desired behavior exactly. We need a test case where sys.path starts with "", followed by the minimal system Python paths. I couldn't achieve this using pytester.runpytest with monkeypatch or pytester.runpytest_subprocess, as the latter always adds the test CWD to the path


assert "==== 1 passed" in result.stdout.strip()

assert result.returncode == 0


def test_ns_in_syspath(pytester: pytest.Pytester, monkeypatch: pytest.MonkeyPatch):
runtime.Namespace.remove(sym.symbol("a.test-path"))

Expand Down Expand Up @@ -324,11 +350,18 @@ def test_ns_not_in_syspath(pytester: pytest.Pytester):
(:require
[basilisp.test :refer [deftest is]]))
"""
pytester.makefile(".lpy", **{"./test/a/test_path": code})
# In this test, we use a `testabc` directory instead of `test`, as
# the latter can cause issues on macOS. Specifically, macOS has a
# `/Library/Frameworks/Python.framework/Versions/3.xx/lib/python3.13/test`
# directory is picked up, resulting in a slightly different error
# message.
pytester.makefile(".lpy", **{"./testabc/a/test_path": code})
pytester.syspathinsert()
result: pytest.RunResult = pytester.runpytest("test")
result: pytest.RunResult = pytester.runpytest("testabc")
assert result.ret != 0
result.stdout.fnmatch_lines(["*ModuleNotFoundError: No module named 'test.a'"])
result.stdout.fnmatch_lines(
["*ModuleNotFoundError: Module named 'a.test-path' is not in sys.path"]
)


def test_ns_with_underscore(pytester: pytest.Pytester):
Expand Down
Loading