Add pre-suite persona cleanup fixture#788
Conversation
Add a session-scoped autouse pytest fixture in tests/integration/sdk/conftest.py that deletes all non-important active personas before the test session starts. This prevents accumulated leaked personas from previous CI runs from exhausting the staging account's 10-persona limit (HTTP 429). - Create conftest.py with cleanup_all_personas fixture - Preserve important persona IDs used by frontend and other test suites - Remove redundant test_does_nothing_expect_cleanup_personas test
|
| Filename | Overview |
|---|---|
| tests/integration/sdk/conftest.py | New session-scoped autouse fixture that deletes non-important personas before and after the test session, with proper error logging and symmetric teardown — previous review issues (silent suppression, missing teardown) are addressed. |
| tests/integration/sdk/test_personas.py | Removes the ad-hoc cleanup pseudo-test test_does_nothing_expect_cleanup_personas; its responsibility is now correctly handled by the conftest fixture. |
Prompt To Fix All With AI
This is a comment left during a code review.
Path: tests/integration/sdk/conftest.py
Line: 24-56
Comment:
**No new integration tests added**
This PR is a bug fix (HTTP 429 from leaked personas) but removes a test without adding a replacement. The fixture itself is infrastructure — it doesn't assert any new behavior. Per the team's guideline, bug-fix PRs should include at least one new integration test (e.g., a test that verifies the fixture's cleanup logic actually removes a persona it shouldn't keep).
**Context Used:** # Test guidelines
- Add a comment if the PR does n... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
How can I resolve this? If you propose a fix, please make it concise.Reviews (2): Last reviewed commit: "fix: add error logging and post-session ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if persona.persona_id not in IMPORTANT_PERSONAS: | ||
| try: | ||
| client.personas.delete(persona.persona_id) | ||
| except Exception: | ||
| pass # best-effort cleanup | ||
| except Exception: |
There was a problem hiding this comment.
Both except Exception: pass blocks silently discard all errors, making it impossible to distinguish between "cleanup succeeded" and "cleanup silently failed" in CI logs. A warnings.warn or print on the outer block would at least surface API connectivity issues without blocking the suite.
| if persona.persona_id not in IMPORTANT_PERSONAS: | |
| try: | |
| client.personas.delete(persona.persona_id) | |
| except Exception: | |
| pass # best-effort cleanup | |
| except Exception: | |
| except Exception as e: | |
| print(f"[cleanup_all_personas] persona listing failed, skipping cleanup: {e}") # best-effort |
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/integration/sdk/conftest.py
Line: 42-47
Comment:
**Silent error suppression hides cleanup failures**
Both `except Exception: pass` blocks silently discard all errors, making it impossible to distinguish between "cleanup succeeded" and "cleanup silently failed" in CI logs. A `warnings.warn` or `print` on the outer block would at least surface API connectivity issues without blocking the suite.
```suggestion
except Exception as e:
print(f"[cleanup_all_personas] persona listing failed, skipping cleanup: {e}") # best-effort
```
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Addressed — both except blocks now log the error with a [cleanup_all_personas] prefix and the phase (setup/teardown), so failures are visible in CI logs without blocking the suite. Also extracted the cleanup logic into a _delete_non_important_personas helper to keep it DRY with the new teardown pass.
| for persona in client.personas.list(page_size=100): | ||
| if persona.persona_id not in IMPORTANT_PERSONAS: | ||
| try: | ||
| client.personas.delete(persona.persona_id) | ||
| except Exception: | ||
| pass # best-effort cleanup | ||
| except Exception: | ||
| pass # don't block the test suite if cleanup itself fails | ||
|
|
There was a problem hiding this comment.
The fixture only runs cleanup before the suite (pre-yield) and does nothing after. Personas leaked by a crash in the current run won't be cleaned up until the next CI run, which may still hit the 10-persona cap if several runs overlap. Adding a symmetric teardown pass after yield would keep the account clean regardless of which run crashed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/integration/sdk/conftest.py
Line: 41-49
Comment:
**No post-session cleanup**
The fixture only runs cleanup *before* the suite (pre-`yield`) and does nothing after. Personas leaked by a crash in the *current* run won't be cleaned up until the next CI run, which may still hit the 10-persona cap if several runs overlap. Adding a symmetric teardown pass after `yield` would keep the account clean regardless of which run crashed.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Addressed — the fixture now runs a symmetric teardown pass after yield, so personas created during the current run are cleaned up even if individual test teardowns fail. Extracted the cleanup into a _delete_non_important_personas(phase) helper that runs in both setup and teardown phases.
Address review feedback: - Log errors instead of silently suppressing them in except blocks - Add symmetric teardown pass after yield to clean up personas leaked by the current run - Extract cleanup logic into helper to avoid duplication
Summary
test_does_nothing_expect_cleanup_personastest since the fixture now handles cleanup automatically before any tests runDetails
Problem: All 9 tests in
tests/integration/sdk/test_personas.pywere failing with HTTP 429 (Max active personas limit exceeded) because prior CI runs left personas open without cleaning them up, eventually hitting the 10-persona cap.Fix: A new
tests/integration/sdk/conftest.pywith acleanup_all_personassession-scoped autouse fixture that:Related insight: https://app.mendral.com/insights/01KQ0X2XXZ0CFVGJMBDPTHMN7B
Note
Created by Mendral. Tag @mendral-app with feedback or questions.