Skip to content

Commit

Permalink
Force UTF-8 when reading subprocess output
Browse files Browse the repository at this point in the history
Subprocess execution was ensuring that Python in the
subprocess used UTF-8 encoding for its standard streams,
but it wasn't ensuring that the *current* process also
did so when communicating with the subprocess via pipes.

Closes #42
  • Loading branch information
ncoghlan committed Oct 28, 2024
1 parent 2c7ec5b commit 14d23af
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 13 deletions.
8 changes: 7 additions & 1 deletion src/venvstacks/_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,18 @@ def run_python_command_unchecked(
env: Mapping[str, str] | None = None,
**kwds: Any,
) -> subprocess.CompletedProcess[str]:
# Ensure required env vars are passed down on Windows,
# and run Python in isolated mode with UTF-8 as the text encoding
run_env = os.environ.copy()
if env is not None:
run_env.update(env)
run_env.update(_SUBPROCESS_PYTHON_CONFIG)
# Default to running in text mode,
# but allow it to be explicitly switched off
text = kwds.pop("text", True)
encoding = "utf-8" if text else None
result: subprocess.CompletedProcess[str] = subprocess.run(
command, env=env, text=True, **kwds
command, env=env, text=text, encoding=encoding, **kwds
)
return result

Expand Down
18 changes: 18 additions & 0 deletions tests/support.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@
import json
import os
import subprocess
import sys
import tomllib

from dataclasses import dataclass, fields
from pathlib import Path
from typing import Any, cast, Mapping
from unittest.mock import create_autospec

import pytest

from venvstacks._util import run_python_command
from venvstacks.stacks import (
BuildEnvironment,
Expand All @@ -20,6 +23,21 @@

_THIS_DIR = Path(__file__).parent

##################################
# Marking test cases
##################################

# Basic marking uses the pytest.mark API directly
# See pyproject.toml and tests/README.md for the defined marks

def requires_venv(description: str) -> pytest.MarkDecorator:
"""Skip test case when running tests outside a virtual environment"""
return pytest.mark.skipif(
sys.prefix == sys.base_prefix,
reason=f"{description} requires test execution in venv",
)


##################################
# Exporting test artifacts
##################################
Expand Down
40 changes: 28 additions & 12 deletions tests/test_cli_invocation.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from venvstacks.stacks import BuildEnvironment, EnvironmentLock, IndexConfig
from venvstacks._util import run_python_command_unchecked

from support import requires_venv

def report_traceback(exc: BaseException | None) -> str:
if exc is None:
Expand Down Expand Up @@ -148,26 +149,41 @@ def test_implicit_help(self, mocked_runner: MockedRunner) -> None:
assert result.exception is None, report_traceback(result.exception)
assert result.exit_code == 0

# See https://github.com/lmstudio-ai/venvstacks/issues/42
@pytest.mark.xfail(
sys.platform == "win32", reason="UnicodeDecodeError parsing output"
)
@requires_venv("Entry point test")
def test_entry_point_help_raw(self) -> None:
expected_entry_point = Path(sys.executable).parent / "venvstacks"
command = [str(expected_entry_point), "--help"]
result = run_python_command_unchecked(
command, text=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE
)
if result.stdout is not None:
# Usage message should suggest direct execution
assert b"Usage: venvstacks [" in result.stdout
# Top-level callback docstring is used as the overall CLI help text
cli_help = cli.handle_app_options.__doc__
assert cli_help is not None
assert cli_help.strip().encode() in result.stdout
# Check operation result last to ensure test results are as informative as possible
assert result.returncode == 0
assert result.stdout is not None

@requires_venv("Entry point test")
def test_entry_point_help(self) -> None:
if sys.prefix == sys.base_prefix:
pytest.skip("Entry point test requires test execution in venv")
expected_entry_point = Path(sys.executable).parent / "venvstacks"
command = [str(expected_entry_point), "--help"]
result = run_python_command_unchecked(
command, stdout=subprocess.PIPE, stderr=subprocess.PIPE
)
# Usage message should suggest direct execution
assert "Usage: venvstacks [" in result.stdout
# Top-level callback docstring is used as the overall CLI help text
cli_help = cli.handle_app_options.__doc__
assert cli_help is not None
assert cli_help.strip() in result.stdout
if result.stdout is not None:
# Usage message should suggest direct execution
assert "Usage: venvstacks [" in result.stdout
# Top-level callback docstring is used as the overall CLI help text
cli_help = cli.handle_app_options.__doc__
assert cli_help is not None
assert cli_help.strip() in result.stdout
# Check operation result last to ensure test results are as informative as possible
assert result.returncode == 0
assert result.stdout is not None


EXPECTED_USAGE_PREFIX = "Usage: python -m venvstacks "
Expand Down

0 comments on commit 14d23af

Please sign in to comment.