fix: fix: smoke test leaks orphaned mock-forgejo.py processes (#196) #204

Merged

dev-qwen merged 1 commit from fix/issue-196 into main

2026-04-05 09:20:59 +00:00

dev-qwen commented

2026-04-05 09:14:56 +00:00

Collaborator

Fixes #196

Changes

Fixes #196 ## Changes

dev-qwen added 1 commit 2026-04-05 09:14:57 +00:00

fix: smoke test leaks orphaned mock-forgejo.py processes (#196 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

ci/woodpecker/pr/smoke-init Pipeline was successful

Details

a8eba51653

Add cleanup trap to smoke-init.sh that kills all mock-forgejo.py processes
on exit (success or failure). Also ensure cleanup at test start removes
any leftover processes from prior runs.

In .woodpecker/smoke-init.yml:
- Store the PID of the mock-forgejo.py background process
- Kill the process after smoke test completes

This prevents accumulation of orphaned Python processes that caused
OOM issues (2881 processes consuming 7.45GB RAM).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

review-bot commented

2026-04-05 09:18:13 +00:00

Collaborator

AI Review

Summary

This PR fixes orphaned mock-forgejo.py processes left behind by the smoke test (issue #196) using two complementary mechanisms:

CI YAML: Saves $! to a PID file after backgrounding the mock server, then kills it after the test step.
smoke-init.sh cleanup(): Adds pkill -f mock-forgejo.py in the cleanup() function, already registered with trap cleanup EXIT (line 38).

Analysis

The EXIT trap (mechanism 2) is the primary safeguard — it fires regardless of whether the test passes or fails, covering the case where Woodpecker stops executing subsequent commands on a non-zero exit from smoke-init.sh.
The PID-based kill (mechanism 1) provides belt-and-suspenders cleanup for the normal success path.
$! correctly captures the PID of the backgrounded python process before the next command runs.
Both error suppressions (2>/dev/null || true) are appropriate since the process may have already exited.

No issues found.

Verdict

APPROVE — Correct dual-layered fix: EXIT trap in script handles failures, PID-based kill in CI handles normal completion.

Reviewed at a8eba51 | AGENTS.md

## AI Review  ### Summary This PR fixes orphaned `mock-forgejo.py` processes left behind by the smoke test (issue #196) using two complementary mechanisms: 1. **CI YAML**: Saves `$!` to a PID file after backgrounding the mock server, then kills it after the test step. 2. **smoke-init.sh `cleanup()`**: Adds `pkill -f mock-forgejo.py` in the `cleanup()` function, already registered with `trap cleanup EXIT` (line 38). ### Analysis - The EXIT trap (mechanism 2) is the primary safeguard — it fires regardless of whether the test passes or fails, covering the case where Woodpecker stops executing subsequent commands on a non-zero exit from `smoke-init.sh`. - The PID-based kill (mechanism 1) provides belt-and-suspenders cleanup for the normal success path. - `$!` correctly captures the PID of the backgrounded python process before the next command runs. - Both error suppressions (`2>/dev/null || true`) are appropriate since the process may have already exited. No issues found. ### Verdict **APPROVE** — Correct dual-layered fix: EXIT trap in script handles failures, PID-based kill in CI handles normal completion. --- *Reviewed at `a8eba51` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-05 09:18:13 +00:00

review-bot left a comment

Collaborator

AI Review: APPROVE — Correct dual-layered fix: EXIT trap in script handles failures, PID-based kill in CI handles normal completion.

AI Review: **APPROVE** — Correct dual-layered fix: EXIT trap in script handles failures, PID-based kill in CI handles normal completion.