Skip to content

Fix test runner to use current Python and report failures - #652

Open
xdong99 wants to merge 1 commit into
elapouya:masterfrom
xdong99:fix-test-runner-python
Open

Fix test runner to use current Python and report failures#652
xdong99 wants to merge 1 commit into
elapouya:masterfrom
xdong99:fix-test-runner-python

Conversation

@xdong99

@xdong99 xdong99 commented Jun 27, 2026

Copy link
Copy Markdown

Summary

  • Run test scripts with sys.executable so the active virtualenv/CI Python is preserved.
  • Resolve test files relative to the tests/ directory, allowing python tests/runtests.py from the repo root.
  • Exit non-zero when any test script fails.
  • Replace os.system("python -m docxtpl ...") with subprocess.run(..., check=True).

Verification

  • python tests/runtests.py
  • python -m flake8 docxtpl tests setup.py --count --max-line-length=127 --show-source --statistics

@arturict

Copy link
Copy Markdown

Good docs-oriented PR. Could you include a short negative test case for the new placeholder support to ensure error behavior remains stable?

@JackSpiece JackSpiece left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Checked this locally from the repository root with the project environment. All 36 test scripts completed and the runner returned 0. I also reran it with the project import deliberately unavailable; it collected the failing script names and returned 1 as intended. The module CLI test used the same interpreter and generated its output successfully. The full flake8 target passes too. No blocking issues found.

@yangfan-yf-yf yangfan-yf-yf left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The new path handling still breaks the documented invocation on supported Python 3.7 and 3.8. Before Python 3.9, __main__.__file__ retains the relative script path; Python 3.9 changed it to an absolute path (Python 3.9 notes). With cd tests && python runtests.py, os.path.dirname(__file__) is therefore "", and the first subprocess.run(..., cwd=tests_dir) raises instead of recording a test result.

I reproduced the relative-__file__ branch on 9721512 with runpy.run_path("runtests.py", run_name="__main__"): it prints cellbg.py ... and then raises OSError: [WinError 123] at the first child launch. The repository documentation still gives cd tests/; python runtests.py, and the package still advertises Python 3.7 and 3.8.

Could tests_dir be normalized to an absolute path first (for example, os.path.dirname(os.path.abspath(__file__))) and the documented invocation be covered on one of the oldest supported versions? The root invocation otherwise ran all 36 test scripts successfully, and a synthetic failing script was aggregated and returned exit 1 as intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants