Skip to content

test: harden process and TUI synchronization - #4

Merged
dejay-vu merged 2 commits into
mainfrom
agent/harden-process-exit-check
Jul 14, 2026
Merged

test: harden process and TUI synchronization#4
dejay-vu merged 2 commits into
mainfrom
agent/harden-process-exit-check

Conversation

@dejay-vu

@dejay-vu dejay-vu commented Jul 14, 2026

Copy link
Copy Markdown
Owner

Summary

  • reject non-positive process IDs before probing for process exit
  • treat only ProcessLookupError as evidence that the process is gone
  • fail explicitly when os.kill(pid, 0) raises another OSError
  • wait for the asynchronous GC preview callback before inspecting its modal

Why

The process-exit helper follows up on Copilot feedback from PR #3: catching every
OSError could incorrectly pass when a process still existed but could not be
inspected. The Python 3.14 check also exposed a separate TUI race where the test
could inspect the screen before the GC callback had pushed the confirmation
modal.

Impact

Production behavior is unchanged. These tests now distinguish a missing process
from a failed process probe and synchronize with both the GC callback and modal
mounting before making assertions.

Validation

  • GC preview test passed 20 consecutive runs
  • targeted background login-build test passed
  • conda run -n slurmdeck-dev ruff format .
  • conda run -n slurmdeck-dev ruff check .
  • conda run -n slurmdeck-dev mypy
  • conda run -n slurmdeck-dev python -m pytest -q (510 passed)
  • git diff --check

@dejay-vu dejay-vu changed the title test: harden process exit checks test: harden process and TUI synchronization Jul 14, 2026
@dejay-vu
dejay-vu marked this pull request as ready for review July 14, 2026 22:19
Copilot AI review requested due to automatic review settings July 14, 2026 22:19
@dejay-vu
dejay-vu merged commit c3a9205 into main Jul 14, 2026
7 checks passed
@dejay-vu
dejay-vu deleted the agent/harden-process-exit-check branch July 14, 2026 22:19

Copilot AI 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.

Pull request overview

This PR hardens timing- and OS-error-sensitive test helpers by making process-exit probing distinguish “process is gone” from “probe failed”, and by synchronizing a TUI test with the GC preview confirmation modal before asserting on it.

Changes:

  • Reject non-positive PIDs and treat only ProcessLookupError as a definitive “process exited” signal during os.kill(pid, 0) probing.
  • Fail explicitly when os.kill(pid, 0) raises other OSError subclasses (e.g., permission/probing errors) instead of silently passing.
  • Add an explicit async wait for the ConfirmModal screen before inspecting/clicking it in the GC preview TUI test.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/unit/test_env_executors.py Tightens process-exit polling to avoid false positives and unsafe PID values in unit tests.
tests/tui/test_pilot.py Adds explicit modal synchronization to prevent a race when asserting against the GC confirmation UI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

2 participants