From b692353e98794804bd3584e3df648e4b726d0b23 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Fri, 5 Jun 2026 22:45:14 -0400 Subject: [PATCH] docs(spec+plan): wait_for_ready_test_pattern (replace time.sleep with polling) --- .../2026-06-05-wait-for-ready-test-pattern.md | 191 ++++++++++++++++++ ...6-05-wait-for-ready-test-pattern-design.md | 112 ++++++++++ 2 files changed, 303 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-05-wait-for-ready-test-pattern.md create mode 100644 docs/superpowers/specs/2026-06-05-wait-for-ready-test-pattern-design.md diff --git a/docs/superpowers/plans/2026-06-05-wait-for-ready-test-pattern.md b/docs/superpowers/plans/2026-06-05-wait-for-ready-test-pattern.md new file mode 100644 index 00000000..f85c5991 --- /dev/null +++ b/docs/superpowers/plans/2026-06-05-wait-for-ready-test-pattern.md @@ -0,0 +1,191 @@ +# wait_for_ready_test_pattern_20260605 Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace `time.sleep(N)` in `test_workspace_profiles_sim.py` and `test_auto_switch_sim.py` with polling helpers that wait for the operation to complete. Tests should pass consistently across machines. + +**Architecture:** Inline polling helpers (or extracted to `tests/helpers.py` if 3+ tests need them). 100ms poll interval, 5s default timeout. + +**Tech Stack:** Python 3.11+, pytest 9.0, time-based polling. + +--- + +## File Structure + +| File | Change | Purpose | +|---|---|---| +| `tests/test_workspace_profiles_sim.py` | Modify | Replace time.sleep with polling | +| `tests/test_auto_switch_sim.py` | Modify | Replace time.sleep with polling | + +No production code changes. No new shared module (helpers are inlined for now). + +--- + +## Task 1: Migrate `test_workspace_profiles_sim.py` + +**Files:** +- Modify: `tests/test_workspace_profiles_sim.py` + +- [ ] **Step 1.1: Pre-edit checkpoint** + +```powershell +cd C:\projects\manual_slop; git status --short +``` + +- [ ] **Step 1.2: Read the test** + +Read `tests/test_workspace_profiles_sim.py` to see the current `time.sleep` calls. + +- [ ] **Step 1.3: Add the polling helpers at the top of the file** + +After the existing imports, add: + +```python +import time + +def wait_for_save_completion(client, profile_name, timeout=5.0): + """Poll until the saved profile appears in the workspace profiles.""" + deadline = time.time() + timeout + while time.time() < deadline: + profiles = client.get_value('workspace_profiles') or {} + if profile_name in profiles: + return + time.sleep(0.1) + raise TimeoutError(f"Profile '{profile_name}' did not appear in workspace_profiles within {timeout}s") + +def wait_for_load_completion(client, item, expected, timeout=5.0): + """Poll until the item's value matches expected.""" + deadline = time.time() + timeout + while time.time() < deadline: + if client.get_value(item) == expected: + return + time.sleep(0.1) + raise TimeoutError(f"Item '{item}' did not become {expected!r} within {timeout}s") +``` + +Use exactly 1-space indentation. No comments. + +- [ ] **Step 1.4: Replace the `time.sleep` calls** + +In the test body, replace: +- `time.sleep(2.0)` after `save_workspace_profile` → `wait_for_save_completion(client, "test_restore")` +- `time.sleep(2.0)` after `load_workspace_profile` → `wait_for_load_completion(client, 'ui_separate_tier1', True)` +- The other `time.sleep(1.0)` calls after `set_value` can stay (set_value is synchronous in the controller) OR be replaced with `wait_for_load_completion` for consistency. + +**Recommended:** keep the `set_value` sleeps for now (set_value writes to controller synchronously; the sleep is for the GUI to process the change), but replace the save/load ones. + +- [ ] **Step 1.5: Run the test** + +```powershell +cd C:\projects\manual_slop; uv run pytest tests/test_workspace_profiles_sim.py -v --timeout=30 +``` + +Expected: 1 passed. + +- [ ] **Step 1.6: Commit** + +```powershell +cd C:\projects\manual_slop; git add tests/test_workspace_profiles_sim.py +git -C C:\projects\manual_slop commit -m "test(workspace_profiles): replace time.sleep with wait_for_X polling helpers" +$h = git -C C:\projects\manual_slop log -1 --format='%H' +git -C C:\projects\manual_slop notes add -m "Replaced time.sleep(2.0) with wait_for_save_completion and wait_for_load_completion polling helpers. 100ms poll interval, 5s default timeout. Per the Authoring Robust live_gui Tests rules in docs/guide_testing.md: use wait-for-ready pattern, not fixed sleeps." $h +``` + +--- + +## Task 2: Migrate `test_auto_switch_sim.py` + +**Files:** +- Modify: `tests/test_auto_switch_sim.py` + +- [ ] **Step 2.1: Read the test** + +Read `tests/test_auto_switch_sim.py` to see the current `time.sleep` calls. + +- [ ] **Step 2.2: Add the polling helpers at the top of the file** + +Same as Task 1 Step 1.3 (or import from a shared location if extracted in the future). + +- [ ] **Step 2.3: Replace the `time.sleep(1)` calls after each `trigger_tier(...)` call** + +The test triggers a tier-2 then tier-3 transition. After each trigger, wait for `show_windows['Diagnostics']` to reach the expected value: + +```python +trigger_tier('Tier 2 (Tech Lead)') +wait_for_load_completion(client, 'show_windows', {'Diagnostics': False}) +assert client.get_value('show_windows').get('Diagnostics', False) == False + +trigger_tier('Tier 3 (Worker): task-1') +wait_for_load_completion(client, 'show_windows', {'Diagnostics': True}) +assert client.get_value('show_windows').get('Diagnostics', False) == True +``` + +- [ ] **Step 2.4: Run the test** + +```powershell +cd C:\projects\manual_slop; uv run pytest tests/test_auto_switch_sim.py -v --timeout=60 +``` + +Expected: 1 passed. + +- [ ] **Step 2.5: Commit** + +```powershell +cd C:\projects\manual_slop; git add tests/test_auto_switch_sim.py +git -C C:\projects\manual_slop commit -m "test(auto_switch): replace time.sleep with wait_for_load_completion polling" +$h = git -C C:\projects\manual_slop log -1 --format='%H' +git -C C:\projects\manual_slop notes add -m "Replaced time.sleep(1) after each trigger_tier with wait_for_load_completion. The auto-switch applies a workspace profile; the test now polls until the expected show_windows state is observed." $h +``` + +--- + +## Task 3: Verify both tests pass in the full batched suite + +**Files:** (no file changes; verification only) + +- [ ] **Step 3.1: Run both tests** + +```powershell +cd C:\projects\manual_slop; uv run pytest tests/test_workspace_profiles_sim.py tests/test_auto_switch_sim.py -v --timeout=60 +``` + +Expected: 2 passed. + +- [ ] **Step 3.2: Commit (no-op)** + +```powershell +cd C:\projects\manual_slop; git -c core.autocrlf=false commit --allow-empty -m "verify: wait_for_ready migration unblocks 2 tests" +``` + +--- + +## Task 4: Update tracks.md + +**Files:** +- Modify: `conductor/tracks.md` + +- [ ] **Step 4.1: Add a brief note** + +Find the live_gui_test_hardening_v2 entry and add: "Sub-track `wait_for_ready_test_pattern_20260605` complete: time.sleep replaced with polling helpers in test_workspace_profiles_sim and test_auto_switch_sim." + +- [ ] **Step 4.2: Commit** + +```powershell +cd C:\projects\manual_slop; git add conductor/tracks.md +git -C C:\projects\manual_slop commit -m "conductor: wait_for_ready_test_pattern sub-track complete" +``` + +--- + +## Self-Review + +- **Spec coverage:** 2 tests migrated; polling helpers defined; fixed sleeps replaced. +- **Placeholders:** None. +- **Type consistency:** Polling helpers return None on success, raise TimeoutError on failure. Test assertions unchanged. +- **Risk:** Low — only test files change. + +--- + +## Execution Handoff + +Inline execution. 4 tasks, atomic commits. User runs the full batched suite to confirm. diff --git a/docs/superpowers/specs/2026-06-05-wait-for-ready-test-pattern-design.md b/docs/superpowers/specs/2026-06-05-wait-for-ready-test-pattern-design.md new file mode 100644 index 00000000..b764985b --- /dev/null +++ b/docs/superpowers/specs/2026-06-05-wait-for-ready-test-pattern-design.md @@ -0,0 +1,112 @@ +# wait_for_ready_test_pattern_20260605 — Design + +**Date:** 2026-06-05 +**Status:** Draft +**Track:** wait_for_ready_test_pattern_20260605 (sub-project of v2) + +## Problem Statement + +Two failing live_gui tests use `time.sleep(N)` to wait for asynchronous GUI operations to complete: + +- `tests/test_workspace_profiles_sim.py` — `time.sleep(2.0)` after save and after load; `time.sleep(1.0)` after each set_value. +- `tests/test_auto_switch_sim.py` — `time.sleep(1)` after each `push_event`. + +Fixed sleeps are a fragile test pattern: +- On slow machines the sleep may be insufficient; the assertion runs before the operation completes. +- On fast machines the sleep is wasted; the test takes longer than necessary. +- Tests that pass with `time.sleep(2.0)` in CI may fail on a developer machine with different load. + +After the state-sync fix (`live_gui_state_sync_20260605`) lands, these tests should pass at the current 2-second sleep. **But the test pattern is still wrong** — the tests should poll for completion, not assume timing. + +## Design + +### Approach: Migrate `time.sleep` to a wait-for-ready helper + +`src/api_hook_client.py` already exposes `wait_for_event(event_type, timeout)` and `get_value(item)`. The tests can use these directly. + +**Hypothetical example — the current pattern:** + +```python +client.set_value('ui_separate_tier1', True) +time.sleep(1.0) +client.push_event("custom_callback", {"callback": "save_workspace_profile", "args": ["test_restore", "project"]}) +time.sleep(2.0) # HOPE the save completes within 2s +client.set_value('ui_separate_tier1', False) +time.sleep(1.0) +client.push_event("custom_callback", {"callback": "load_workspace_profile", "args": ["test_restore"]}) +time.sleep(2.0) # HOPE the load completes within 2s +assert client.get_value('ui_separate_tier1') is True +``` + +**Migrated pattern:** + +```python +def wait_for_save_completion(client, profile_name, timeout=5.0): + """Poll until the saved profile appears in the workspace profiles.""" + import time + deadline = time.time() + timeout + while time.time() < deadline: + profiles = client.get_value('workspace_profiles') or {} + if profile_name in profiles: + return + time.sleep(0.1) + raise TimeoutError(f"Save did not complete within {timeout}s") + +def wait_for_load_completion(client, item, expected, timeout=5.0): + """Poll until the item's value matches expected.""" + import time + deadline = time.time() + timeout + while time.time() < deadline: + if client.get_value(item) == expected: + return + time.sleep(0.1) + raise TimeoutError(f"Load did not apply {item}={expected} within {timeout}s") + +client.set_value('ui_separate_tier1', True) +# No sleep needed; set_value returns when the value is set on the controller +client.push_event("custom_callback", {"callback": "save_workspace_profile", "args": ["test_restore", "project"]}) +wait_for_save_completion(client, "test_restore") +client.set_value('ui_separate_tier1', False) +client.push_event("custom_callback", {"callback": "load_workspace_profile", "args": ["test_restore"]}) +wait_for_load_completion(client, 'ui_separate_tier1', True) +``` + +### Why this approach + +- **Polling, not fixed sleeps**: 100ms poll interval is responsive without busy-waiting. +- **Generous timeouts**: 5s default is well over the typical ~100ms operation; catches genuine hangs. +- **Reusable helpers**: `wait_for_save_completion` and `wait_for_load_completion` are simple and can be added to a shared test helper module. +- **Failure messages are clear**: TimeoutError explicitly says which operation timed out. + +### Alternatives considered + +- **A2: Add wait_for_X helpers to ApiHookClient itself.** Rejected: ApiHookClient should remain a thin transport; test-helper logic doesn't belong there. Keep helpers in `tests/conftest.py` or a `tests/helpers.py` module. +- **A3: Use `wait_for_event` exclusively.** The Hook API's `wait_for_event` listens for events the GUI emits. save/load may not emit events in a way the test can match. Polling `get_value` is more direct. + +## File Changes + +### Modify: `tests/test_workspace_profiles_sim.py` + +Replace `time.sleep(...)` with `wait_for_save_completion` and `wait_for_load_completion` calls. Add the helper functions at the top of the file (or import from a shared helper). + +### Modify: `tests/test_auto_switch_sim.py` + +Replace `time.sleep(...)` with similar polling helpers. + +### Optionally: Create: `tests/helpers.py` + +If multiple tests need the same helpers, extract them to a shared module. For now, keep them inline (2 tests, ~30 lines of helpers total). + +## Risk Assessment + +| Risk | Likelihood | Impact | Mitigation | +|---|---|---|---| +| The polling masks a slow operation that's now flaky | Low | Medium | Generous 5s timeout; if a test times out, the test message points to which operation | +| Helper functions added in 2 places diverge | Medium | Low | If 3+ tests need the same helper, extract to `tests/helpers.py` | + +## Out of Scope + +- **State sync fix** — separate track (`live_gui_state_sync_20260605`). +- **prior_session test** — separate track (`prior_session_test_harden_20260605`). +- **Migrating other live_gui tests that use `time.sleep`** — out of scope for now. Track as a follow-up if more flakes appear. +- **Replacing `time.sleep` with `asyncio.sleep`** — out of scope; the live_gui tests are sync, and the GUI event queue is sync.