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: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,13 @@ print(result)
#> SubprocessResult(id='e9f2d29acb4011ee8957320319d7541c', stdout='hello, world!\n', stderr='', returncode=0, killed_by_token=False)
```

It returns an object of the `SubprocessResult` class, which contains the following fields:
It returns an object of the `SubprocessResult` class, which contains the following public attributes:

- **id**: a unique string that allows you to distinguish one result of calling the same command from another.
- **stdout**: a string containing the entire output of the command being run.
- **stderr**: a string containing the entire stderr output of the command being run. If the subprocess fails to start at all, this field remains empty because no process stderr existed yet.
- **returncode**: an integer indicating the return code of the subprocess. `0` means that the process was completed successfully; other values usually indicate an error.
- **success**: a read-only boolean property that is `True` exactly when `returncode == 0`.
- **killed_by_token**: a boolean flag indicating whether the subprocess was killed due to [token](https://cantok.readthedocs.io/en/latest/the_pattern/) cancellation.


Expand Down
104 changes: 104 additions & 0 deletions docs/plans/1.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
# Добавление read-only свойства `SubprocessResult.success`

Добавить вычисляемое свойство `success`, возвращающее `True` только при `returncode == 0`. Явные setter и deleter должны запрещать прямое изменение свойства. Разработку провести по циклу RED → GREEN → REFACTOR.

> **Примечание.** Была исследована документация для [Linux/POSIX](https://man7.org/linux/man-pages/man3/exit_success.3const.html), [macOS](https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/sysexits.3.html), [Windows](https://learn.microsoft.com/en-us/cpp/c-runtime-library/exit-success-exit-failure?view=msvc-170), а также [стандартная реализация `subprocess` в Python](https://docs.python.org/3/library/subprocess.html#subprocess.CompletedProcess.returncode). Исследование подтвердило, что проверка `returncode == 0` достаточна для определения стандартного успеха процесса на поддерживаемых платформах; отдельная логика для разных ОС не нужна.

## Порядок работы

1. До изменений тестов и исходного кода создать `docs/plans/1.md` и сохранить в нём этот план.
2. RED: добавить и расширить перечисленные ниже тесты, запустить их и убедиться, что они падают из-за отсутствующего контракта `success`.
3. GREEN: реализовать минимально необходимый getter, setter и deleter.
4. REFACTOR: проверить читаемость реализации и тестов без изменения поведения.
5. Обновить README и выполнить полный набор проверок проекта.

## Публичный API и реализация

- Добавить в `SubprocessResult` свойство `success: bool`.
- Getter вычисляет значение при каждом обращении:
- `returncode == 0` → `True`;
- `returncode is None` → `False`;
- любой положительный или отрицательный ненулевой код → `False`.
- Setter при любом присваивании выбрасывает `AttributeError` с текстом `The success property is read-only and cannot be assigned.`
- Deleter при удалении выбрасывает `AttributeError` с текстом `The success property is read-only and cannot be deleted.`
- Не добавлять `success` в dataclass-поля: конструктор, `repr()` и `dataclasses.asdict()` остаются неизменными.
- В README описать тип, вычисление и read-only поведение свойства.

## TDD и конкретные тесты

1. `test_default_values` (существующий тест)

- Суть: расширить проверку начального состояния `SubprocessResult`.
- Фиксируем: объект с `returncode is None` не считается успешно завершившимся.
- Ассерты: добавить `SubprocessResult().success is False`.

2. `test_success_reflects_returncode`

- Суть: параметризованно проверить полную таблицу вычисления свойства при обоих значениях соседнего статусного флага `killed_by_token`.
- Фиксируем: `success` определяется только точным сравнением `returncode == 0`, а не истинностью самого кода или значением `killed_by_token`; чтение свойства не изменяет оба статусных поля.
- Ассерты: при `killed_by_token` со значениями `False` и `True` для `None`, `1` и `-1` результат равен `False`, а для `0` — `True`; после чтения свойства исходные значение и тип `returncode`, а также `killed_by_token` сохранены. Использовать `is`, чтобы подтвердить точные boolean-значения.

3. `test_success_is_recomputed_after_returncode_changes`

- Суть: при обоих значениях `killed_by_token` последовательно менять `returncode` одного объекта.
- Фиксируем: `success` вычисляется при каждом обращении, не кэшируется и сохраняет независимость от `killed_by_token` после изменения `returncode`, не меняя оба статусных поля.
- Ассерты: при `None` получить `False`, после установки `0` — `True`, после установки ненулевого кода — снова `False`; последовательность одинакова для `killed_by_token=False` и `killed_by_token=True`, а установленные значение и тип `returncode` и исходный `killed_by_token` сохранены после каждого чтения.

4. `test_success_assignment_is_forbidden`

- Суть: при обоих значениях `killed_by_token` попытаться напрямую присвоить значение `result.success`.
- Фиксируем: setter существует, но свойство остаётся read-only независимо от текущего и присваиваемого boolean-значения.
- Ассерты: для `returncode` со значениями `0` и `1`, `killed_by_token` со значениями `False` и `True` и присваиваемого `success` со значениями `True` и `False` выброшен именно стандартный `AttributeError`, а не его подкласс, с точным сообщением `The success property is read-only and cannot be assigned.`; исходные значение и тип `returncode`, `killed_by_token` и вычисляемый `success` не изменились.

5. `test_success_deletion_is_forbidden`

- Суть: при обоих значениях `killed_by_token` попытаться выполнить `del result.success`.
- Фиксируем: deleter существует и запрещает удаление свойства независимо от его текущего значения.
- Ассерты: для `returncode` со значениями `0` и `1` и `killed_by_token` со значениями `False` и `True` выброшен именно стандартный `AttributeError`, а не его подкласс, с точным сообщением `The success property is read-only and cannot be deleted.`; исходные значение и тип `returncode`, `killed_by_token` и вычисляемый `success` не изменились.

6. `test_run_result_success_matches_process_exit_code`

- Суть: параметризованно проверить свойство на реальных результатах `run()`.
- Фиксируем: публичный объект, возвращаемый `run()`, сообщает об успехе для нулевого кода и о неуспехе для ненулевого.
- Ассерты: при `catch_exceptions=True` команда с кодом `0` возвращает `returncode == 0` и `success is True`, а команда с кодом `1` — `returncode == 1` и `success is False`.

7. `test_success_is_not_a_dataclass_field`

- Суть: проверить, что вычисляемое свойство не меняет структуру dataclass.
- Фиксируем: `success` не становится dataclass-полем, не попадает в сериализацию и не добавляется в сгенерированный конструктор.
- Ассерты: точные списки dataclass-полей, ключей и значений `asdict()` и параметров сигнатуры конструктора содержат только пять исходных полей.

8. `test_run_result_and_its_attributes_have_expected_types` (существующий тест)

- Суть: расширить статическую проверку типа нового свойства.
- Фиксируем: для mypy `result.success` всегда имеет тип `bool`, в отличие от опционального `returncode`.
- Ассерты: добавить ожидаемый `reveal_type(result.success) == builtins.bool` и корректное присваивание значения свойства переменной типа `bool`.

9. `test_run_hello_world_and_result_contract` (существующий тест)

- Суть: дополнить исполняемый пример из README проверкой нового публичного свойства.
- Фиксируем: документированный успешный запуск возвращает `success is True`.
- Ассерты: добавить проверку `result.success is True`; существующую точную проверку `repr()` сохранить, подтверждая, что `success` туда не добавился.

10. `test_repr_format`

- Суть: существующая проверка запускается без изменений как регрессия формата dataclass.
- Фиксируем: добавление property не меняет публичное строковое представление объекта.
- Ассерты: существующее полное регулярное выражение по-прежнему должно совпадать с `repr(result)` и не содержать `success`.

После добавления тестов сначала запустить целевые unit-, documentation- и typing-тесты и зафиксировать ожидаемую RED-стадию. После реализации повторить целевой запуск до GREEN.

## Финальная проверка

- Запустить весь `pytest`.
- Запустить Ruff для `suby` и `tests`.
- Запустить строгий mypy для библиотеки и отдельную проверку тестов.
- Выполнить CI-команду покрытия с требованием 100% branch coverage.
- Убедиться, что сохранённый `docs/plans/1.md` соответствует этому плану.

## Принятые допущения

- Используется стандартный `AttributeError`; новый класс исключения не добавляется.
- `success` остаётся вычисляемым property и не участвует в dataclass-сериализации.
- `returncode == 0` используется одинаково на всех поддерживаемых ОС без платформенных ветвлений.
- Версия пакета не повышается; коммиты и публикация релиза не входят в задачу.
4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ build-backend = 'flit_core.buildapi'

[project]
name = 'suby'
version = '0.0.12'
version = '0.0.13'
authors = [
{ name='Evgeniy Blinov', email='zheni-b@yandex.ru' },
]
Expand All @@ -13,7 +13,7 @@ readme = 'README.md'
requires-python = '>=3.8'
dependencies = [
'emptylog>=0.0.12',
'cantok>=0.0.40',
'cantok>=0.0.42',
'microbenchmark>=0.0.3',
'sigmatch>=0.0.10',
]
Expand Down
3 changes: 2 additions & 1 deletion requirements_dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ pytest-codspeed==2.2.1; python_version < '3.9'
pytest-codspeed==4.3.0; python_version >= '3.9'
pytest-xdist==3.6.1; python_version < '3.9'
pytest-xdist==3.8.0; python_version >= '3.9'
coverage==7.6.1
coverage==7.6.1; python_version < '3.14'
coverage==7.15.2; python_version >= '3.14'
coverage-pyver-pragma==0.4.0
build==1.2.2.post1
mypy==1.14.1
Expand Down
12 changes: 12 additions & 0 deletions suby/subprocess_result.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,15 @@ class SubprocessResult:
stderr: Optional[str] = None
returncode: Optional[int] = None
killed_by_token: bool = False

@property
def success(self) -> bool:
return self.returncode == 0

@success.setter
def success(self, _value: bool) -> None:
raise AttributeError('The success property is read-only and cannot be assigned.')

@success.deleter
def success(self) -> None:
raise AttributeError('The success property is read-only and cannot be deleted.')
5 changes: 3 additions & 2 deletions tests/documentation/test_readme.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ def _read_child_environment(*names: str, **run_kwargs):
return json.loads(result.stdout)


def test_run_hello_world_and_result_repr_format():
"""The hello-world command is echoed to stdout, captured in result.stdout, and keeps the documented SubprocessResult repr format."""
def test_run_hello_world_and_result_contract():
"""The README hello-world call forwards child output to stdout and captures it in result.stdout. The result reports success and retains the documented repr."""
stderr_buffer = StringIO()
stdout_buffer = StringIO()

Expand All @@ -48,6 +48,7 @@ def test_run_hello_world_and_result_repr_format():
assert result.stderr == ''
assert result.returncode == 0
assert not result.killed_by_token
assert result.success is True

assert re.fullmatch(
r"SubprocessResult\(id='[0-9a-f]{32}', stdout='hello, world!\\n', stderr='', returncode=0, killed_by_token=False\)",
Expand Down
6 changes: 4 additions & 2 deletions tests/typing/test_typing_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -355,21 +355,23 @@ def check(self) -> None:


@pytest.mark.mypy_testing
def test_run_result_and_result_fields_have_expected_types() -> None:
"""run() returns SubprocessResult, and its fields keep their Optional/boolean static types."""
def test_run_result_and_its_attributes_have_expected_types() -> None:
"""run() returns SubprocessResult; stdout, stderr, and returncode are Optional, while killed_by_token and success are bool."""
result = run('python -c pass')

reveal_type(result) # R: suby.subprocess_result.SubprocessResult
reveal_type(result.stdout) # R: Union[builtins.str, None]
reveal_type(result.stderr) # R: Union[builtins.str, None]
reveal_type(result.returncode) # R: Union[builtins.int, None]
reveal_type(result.killed_by_token) # R: builtins.bool
reveal_type(result.success) # R: builtins.bool

_explicit_result: SubprocessResult = run('python -c pass')
_optional_stdout: Optional[str] = result.stdout
_optional_stderr: Optional[str] = result.stderr
_optional_returncode: Optional[int] = result.returncode
_killed_by_token: bool = result.killed_by_token
_success: bool = result.success

if result.stdout is not None:
result.stdout.upper()
Expand Down
33 changes: 22 additions & 11 deletions tests/units/test_benchmarks.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import time
from pathlib import Path

from microbenchmark import Scenario, ScenarioGroup
Expand Down Expand Up @@ -161,32 +160,44 @@ def test_delayed_condition_token_cancellation_timer_starts_after_subprocess_mark
"""
Verify delayed token cancellation is based on subprocess-start marker age.

A fake `run` observes the token before marker creation, shortly after marker creation, and after the 10 ms cancellation delay.
A fake `run` observes repeated pre-marker polls, a post-marker poll before 10 ms, and cancellation exactly at the 10 ms boundary.
"""
observed_states = []
marker_file = None
pre_marker_time_reads = 0
times = iter(())

def fake_time_ns():
nonlocal pre_marker_time_reads

if marker_file is None or not marker_file.exists():
pre_marker_time_reads += 1
return 0
return next(times)

def fake_run(*arguments, **kwargs):
nonlocal marker_file, times

token = kwargs['token']
marker_file = arguments[-1]

time.sleep(0.02)
observed_states.append(bool(token))
assert bool(token) is True
assert bool(token) is True

marker_file.touch()
marker_mtime_ns = marker_file.stat().st_mtime_ns
times = iter(
(
marker_mtime_ns + 1_000_000,
marker_mtime_ns + 20_000_000,
marker_mtime_ns + 9_999_999,
marker_mtime_ns + 10_000_000,
),
)
monkeypatch.setattr(benchmarks, 'time_ns', lambda: next(times))

observed_states.append(bool(token))
observed_states.append(bool(token))
assert bool(token) is True
assert bool(token) is False

monkeypatch.setattr(benchmarks, 'run', fake_run)
monkeypatch.setattr(benchmarks, 'time_ns', fake_time_ns)

benchmarks.run_with_delayed_condition_token_cancellation()

assert observed_states == [True, True, False]
assert pre_marker_time_reads == 0
Loading
Loading