fix(tui): resolve format, AmbiguousStep, and coverage gaps in session persistence
- Apply ruff format to store.py (split multi-arg conn.execute calls) and
steps file (collapse single-arg execute to one line)
- Fix AmbiguousStep: add literal quotes to @given/@then patterns so
'a session with id "{session_id}"' is unambiguous vs the
'in the database' variant; update all dependent then-steps consistently
- Mark if TYPE_CHECKING block with # pragma: no cover (never executed)
- Add scenario + step for default db_path to cover store.py lines 32-34
(uses unittest.mock.patch on Path.home to avoid touching real homedir)
ISSUES CLOSED: #10648
This commit is contained in:
@@ -42,15 +42,13 @@ def step_database_has_tables(context: object) -> None:
|
||||
import sqlite3
|
||||
|
||||
with sqlite3.connect(context.db_path) as conn: # type: ignore
|
||||
cursor = conn.execute(
|
||||
"SELECT name FROM sqlite_master WHERE type='table'"
|
||||
)
|
||||
cursor = conn.execute("SELECT name FROM sqlite_master WHERE type='table'")
|
||||
tables = {row[0] for row in cursor.fetchall()}
|
||||
assert "sessions" in tables
|
||||
assert "active_session" in tables
|
||||
|
||||
|
||||
@given("a session with id {session_id} and transcript {transcript}")
|
||||
@given('a session with id "{session_id}" and transcript {transcript}')
|
||||
def step_create_session_with_transcript(
|
||||
context: object, session_id: str, transcript: str
|
||||
) -> None:
|
||||
@@ -63,7 +61,7 @@ def step_create_session_with_transcript(
|
||||
context.session = session # type: ignore
|
||||
|
||||
|
||||
@given("a session with id {session_id}")
|
||||
@given('a session with id "{session_id}"')
|
||||
def step_create_session(context: object, session_id: str) -> None:
|
||||
"""Create a session with the given id."""
|
||||
session = SessionView(session_id=session_id, transcript=[])
|
||||
@@ -97,7 +95,7 @@ def step_set_active_session(context: object) -> None:
|
||||
context.store.set_active_session(context.session.session_id) # type: ignore
|
||||
|
||||
|
||||
@then("the active session id is {session_id}")
|
||||
@then('the active session id is "{session_id}"')
|
||||
def step_verify_active_session(context: object, session_id: str) -> None:
|
||||
"""Verify the active session id."""
|
||||
active = context.store.get_active_session() # type: ignore
|
||||
@@ -130,7 +128,7 @@ def step_verify_list_sessions(context: object) -> None:
|
||||
assert set(listed) == expected_ids
|
||||
|
||||
|
||||
@given("a session with id {session_id} in the database")
|
||||
@given('a session with id "{session_id}" in the database')
|
||||
def step_create_and_save_session(context: object, session_id: str) -> None:
|
||||
"""Create and save a session."""
|
||||
session = SessionView(session_id=session_id, transcript=[])
|
||||
@@ -184,7 +182,7 @@ def step_create_new_store_instance(context: object) -> None:
|
||||
context.new_store = TuiSessionStore(db_path=context.db_path) # type: ignore
|
||||
|
||||
|
||||
@then("the active session is {session_id}")
|
||||
@then('the active session is "{session_id}"')
|
||||
def step_verify_new_store_active_session(context: object, session_id: str) -> None:
|
||||
"""Verify the active session in the new store."""
|
||||
active = context.new_store.get_active_session() # type: ignore
|
||||
@@ -198,3 +196,17 @@ def step_verify_session_restored(context: object) -> None:
|
||||
assert loaded is not None
|
||||
assert loaded.session_id == context.session.session_id # type: ignore
|
||||
assert loaded.transcript == context.session.transcript # type: ignore
|
||||
|
||||
|
||||
@given("the TUI session store is created with default database path")
|
||||
def step_default_db_path(context: object) -> None:
|
||||
"""Initialize store using the default path, patching home to a temp dir."""
|
||||
from unittest.mock import patch
|
||||
|
||||
temp_dir = tempfile.mkdtemp()
|
||||
temp_home = Path(temp_dir)
|
||||
with patch.object(Path, "home", return_value=temp_home):
|
||||
context.store = TuiSessionStore() # type: ignore
|
||||
context.db_path = ( # type: ignore
|
||||
temp_home / ".local" / "state" / "cleveragents" / "tui.db"
|
||||
)
|
||||
|
||||
@@ -45,3 +45,7 @@ Feature: TUI Session Persistence
|
||||
And I create a new session store instance
|
||||
Then the active session is "last-session"
|
||||
And the session data is restored correctly
|
||||
|
||||
Scenario: Store uses default database path when none is specified
|
||||
Given the TUI session store is created with default database path
|
||||
Then the SQLite database file exists at ~/.local/state/cleveragents/tui.db
|
||||
|
||||
@@ -8,7 +8,7 @@ from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
if TYPE_CHECKING:
|
||||
if TYPE_CHECKING: # pragma: no cover
|
||||
from cleveragents.tui.app import SessionView
|
||||
|
||||
|
||||
@@ -64,10 +64,13 @@ class TuiSessionStore:
|
||||
"""
|
||||
with sqlite3.connect(self.db_path) as conn:
|
||||
transcript_json = json.dumps(session.transcript)
|
||||
conn.execute("""
|
||||
conn.execute(
|
||||
"""
|
||||
INSERT OR REPLACE INTO sessions (session_id, transcript, updated_at)
|
||||
VALUES (?, ?, CURRENT_TIMESTAMP)
|
||||
""", (session.session_id, transcript_json))
|
||||
""",
|
||||
(session.session_id, transcript_json),
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
def load_session(self, session_id: str) -> SessionView | None:
|
||||
@@ -80,9 +83,12 @@ class TuiSessionStore:
|
||||
The SessionView if found, None otherwise
|
||||
"""
|
||||
with sqlite3.connect(self.db_path) as conn:
|
||||
cursor = conn.execute("""
|
||||
cursor = conn.execute(
|
||||
"""
|
||||
SELECT session_id, transcript FROM sessions WHERE session_id = ?
|
||||
""", (session_id,))
|
||||
""",
|
||||
(session_id,),
|
||||
)
|
||||
row = cursor.fetchone()
|
||||
|
||||
if row is None:
|
||||
@@ -92,6 +98,7 @@ class TuiSessionStore:
|
||||
transcript = json.loads(transcript_json)
|
||||
# Import here to avoid circular imports
|
||||
from cleveragents.tui.app import SessionView
|
||||
|
||||
return SessionView(session_id=session_id_result, transcript=transcript)
|
||||
|
||||
def set_active_session(self, session_id: str) -> None:
|
||||
@@ -101,10 +108,13 @@ class TuiSessionStore:
|
||||
session_id: The ID of the session to set as active
|
||||
"""
|
||||
with sqlite3.connect(self.db_path) as conn:
|
||||
conn.execute("""
|
||||
conn.execute(
|
||||
"""
|
||||
INSERT OR REPLACE INTO active_session (id, session_id, updated_at)
|
||||
VALUES (1, ?, CURRENT_TIMESTAMP)
|
||||
""", (session_id,))
|
||||
""",
|
||||
(session_id,),
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
def get_active_session(self) -> str | None:
|
||||
@@ -139,9 +149,12 @@ class TuiSessionStore:
|
||||
session_id: The ID of the session to delete
|
||||
"""
|
||||
with sqlite3.connect(self.db_path) as conn:
|
||||
conn.execute("""
|
||||
conn.execute(
|
||||
"""
|
||||
DELETE FROM sessions WHERE session_id = ?
|
||||
""", (session_id,))
|
||||
""",
|
||||
(session_id,),
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
def clear_all(self) -> None:
|
||||
|
||||
Reference in New Issue
Block a user