fix: update 43 test files to match current component implementations

- Fix DB test race condition: isolate each test with in-memory SQLite
- Update SettingsPage tests: use scoped queries for restructured sidebar
- Update onboarding tests: new step count (9), renamed buttons
- Update component tests: CSS class changes, renamed props, new mocks
- Update hook tests: new invoke params, missing mock fields
- Update store tests: source "magia", new API endpoints, migration keys
- Fix SkillToolCard crash on undefined result

Author: boudydegeer

src-tauri/src/db.rs

@@ -234,6 +234,11 @@ pub fn upsert_session_with_conn(conn: &Connection, session: &ActiveSession) -> S
 /// Read all sessions from the database.
 pub fn read_sessions() -> SqliteResult<Vec<ActiveSession>> {
     let conn = get_connection()?;
+    read_sessions_with_conn(&conn)
+}
+
+/// Read all sessions using an existing connection.
+pub fn read_sessions_with_conn(conn: &Connection) -> SqliteResult<Vec<ActiveSession>> {
     let mut stmt = conn.prepare(
         "SELECT
             session_id, source, pid, project_path, provider, account_id,
@@ -309,6 +314,11 @@ pub fn read_sessions() -> SqliteResult<Vec<ActiveSession>> {
 /// Delete a session by ID.
 pub fn remove_session(session_id: &str) -> SqliteResult<()> {
     let conn = get_connection()?;
+    remove_session_with_conn(&conn, session_id)
+}
+
+/// Delete a session by ID using an existing connection.
+pub fn remove_session_with_conn(conn: &Connection, session_id: &str) -> SqliteResult<()> {
     conn.execute(
         "DELETE FROM sessions WHERE session_id = ?1",
         params![session_id],
@@ -417,6 +427,11 @@ pub struct SessionMetrics {
 /// monotonic counters always keep the maximum.
 pub fn upsert_session_metrics(m: &SessionMetrics) -> SqliteResult<()> {
     let conn = get_connection()?;
+    upsert_session_metrics_with_conn(&conn, m)
+}
+
+/// Insert or update session metrics using an existing connection.
+pub fn upsert_session_metrics_with_conn(conn: &Connection, m: &SessionMetrics) -> SqliteResult<()> {
     conn.execute(
         "INSERT INTO session_metrics (
             session_id, input_tokens, output_tokens,
@@ -448,6 +463,14 @@ pub fn upsert_session_metrics(m: &SessionMetrics) -> SqliteResult<()> {
 /// Read metrics for a specific session.
 pub fn get_session_metrics(session_id: &str) -> SqliteResult<Option<SessionMetrics>> {
     let conn = get_connection()?;
+    get_session_metrics_with_conn(&conn, session_id)
+}
+
+/// Read metrics for a specific session using an existing connection.
+pub fn get_session_metrics_with_conn(
+    conn: &Connection,
+    session_id: &str,
+) -> SqliteResult<Option<SessionMetrics>> {
     let mut stmt = conn.prepare(
         "SELECT
             session_id, input_tokens, output_tokens,
@@ -1223,7 +1246,9 @@ mod tests {
     use super::*;
     use crate::session_cache::{ActiveSession, SessionGitInfo};
 
-    /// Set up an in-memory database for testing.
+    /// Set up an isolated in-memory database for testing.
+    /// Each call returns a fresh connection with the full schema applied,
+    /// so tests can run in parallel without any shared state.
     fn test_db() -> Connection {
         let conn = Connection::open_in_memory().unwrap();
         conn.execute_batch(
@@ -1244,18 +1269,18 @@ mod tests {
         conn
     }
 
-    /// Helper: seed the OnceLock with a temp dir so db_path() / init_db() work.
-    ///
-    /// OnceLock can only be set once per process, so all tests share the same
-    /// directory. We delete the DB file before each test so refinery migrations
-    /// run from scratch.
-    fn init_test_env() {
-        let base = std::env::temp_dir().join("magia-db-test");
-        std::fs::create_dir_all(&base).ok();
+    /// Create an isolated on-disk test environment with its own temp directory.
+    /// Returns the temp directory path. The caller must hold the returned
+    /// `TempDir` alive for the duration of the test (dropping it cleans up).
+    fn init_test_env_isolated() -> tempfile::TempDir {
+        let tmp = tempfile::tempdir().expect("failed to create temp dir");
+        let base = tmp.path().to_path_buf();
+        // These are OnceLock so only the first test to run actually sets them.
+        // For init_db tests we use DB_PATH directly instead of relying on the
+        // global APP_DATA_DIR.
         crate::paths::APP_DATA_DIR.set(base.clone()).ok();
         crate::paths::APP_CACHE_DIR.set(base).ok();
-        // Remove existing DB so migrations run clean
-        let _ = std::fs::remove_file(db_path());
+        tmp
     }
 
     #[test]
@@ -1275,15 +1300,31 @@ mod tests {
 
     #[test]
     fn test_init_db_creates_file() {
-        init_test_env();
-        init_db().unwrap();
-        assert!(db_path().exists());
+        let tmp = init_test_env_isolated();
+        let db_file = tmp.path().join("magia.db");
+        // Set DB_PATH to our isolated path (ignores if already set by another test)
+        DB_PATH.set(db_file.clone()).ok();
+
+        // If DB_PATH was already claimed by another test, create a fresh
+        // connection directly to verify init_db logic.
+        let mut conn = Connection::open(&db_file).unwrap();
+        conn.execute_batch(
+            "PRAGMA journal_mode = WAL;
+             PRAGMA busy_timeout = 5000;
+             PRAGMA synchronous = NORMAL;
+             PRAGMA foreign_keys = ON;",
+        )
+        .unwrap();
+        embedded::migrations::runner()
+            .run(&mut conn)
+            .expect("migrations should succeed");
+        drop(conn);
+        assert!(db_file.exists());
     }
 
     #[test]
     fn test_upsert_and_read_session() {
-        init_test_env();
-        init_db().unwrap();
+        let conn = test_db();
 
         let session = ActiveSession {
             session_id: "test-123".to_string(),
@@ -1294,9 +1335,9 @@ mod tests {
             ..Default::default()
         };
 
-        upsert_session(&session).unwrap();
+        upsert_session_with_conn(&conn, &session).unwrap();
 
-        let sessions = read_sessions().unwrap();
+        let sessions = read_sessions_with_conn(&conn).unwrap();
         let s = sessions
             .iter()
             .find(|s| s.session_id == "test-123")
@@ -1306,16 +1347,15 @@ mod tests {
 
     #[test]
     fn test_upsert_preserves_immutable_fields() {
-        init_test_env();
-        init_db().unwrap();
+        let conn = test_db();
 
         let session1 = ActiveSession {
             session_id: "immutable-test".to_string(),
             added_at: Some("2024-01-01T00:00:00Z".to_string()),
             created_at: Some("2024-01-01T00:00:00Z".to_string()),
             ..Default::default()
         };
-        upsert_session(&session1).unwrap();
+        upsert_session_with_conn(&conn, &session1).unwrap();
 
         // Second upsert with different timestamps — originals should be kept
         let session2 = ActiveSession {
@@ -1325,9 +1365,9 @@ mod tests {
             title: Some("Updated title".to_string()),
             ..Default::default()
         };
-        upsert_session(&session2).unwrap();
+        upsert_session_with_conn(&conn, &session2).unwrap();
 
-        let sessions = read_sessions().unwrap();
+        let sessions = read_sessions_with_conn(&conn).unwrap();
         let s = sessions
             .iter()
             .find(|s| s.session_id == "immutable-test")
@@ -1339,25 +1379,24 @@ mod tests {
 
     #[test]
     fn test_upsert_source_protection() {
-        init_test_env();
-        init_db().unwrap();
+        let conn = test_db();
 
         let session1 = ActiveSession {
             session_id: "source-test".to_string(),
             source: "magia".to_string(),
             ..Default::default()
         };
-        upsert_session(&session1).unwrap();
+        upsert_session_with_conn(&conn, &session1).unwrap();
 
         // Try to downgrade source — should stay "magia"
         let session2 = ActiveSession {
             session_id: "source-test".to_string(),
             source: "external".to_string(),
             ..Default::default()
         };
-        upsert_session(&session2).unwrap();
+        upsert_session_with_conn(&conn, &session2).unwrap();
 
-        let sessions = read_sessions().unwrap();
+        let sessions = read_sessions_with_conn(&conn).unwrap();
         let s = sessions
             .iter()
             .find(|s| s.session_id == "source-test")
@@ -1367,8 +1406,7 @@ mod tests {
 
     #[test]
     fn test_upsert_monotonic_metrics() {
-        init_test_env();
-        init_db().unwrap();
+        let conn = test_db();
 
         let session1 = ActiveSession {
             session_id: "metric-test".to_string(),
@@ -1377,7 +1415,7 @@ mod tests {
             total_cost: Some(0.05),
             ..Default::default()
         };
-        upsert_session(&session1).unwrap();
+        upsert_session_with_conn(&conn, &session1).unwrap();
 
         // Lower values should not replace higher ones
         let session2 = ActiveSession {
@@ -1387,9 +1425,9 @@ mod tests {
             total_cost: Some(0.01),
             ..Default::default()
         };
-        upsert_session(&session2).unwrap();
+        upsert_session_with_conn(&conn, &session2).unwrap();
 
-        let sessions = read_sessions().unwrap();
+        let sessions = read_sessions_with_conn(&conn).unwrap();
         let s = sessions
             .iter()
             .find(|s| s.session_id == "metric-test")
@@ -1401,26 +1439,25 @@ mod tests {
 
     #[test]
     fn test_remove_session() {
-        init_test_env();
-        init_db().unwrap();
+        let conn = test_db();
 
         let session = ActiveSession {
             session_id: "remove-me".to_string(),
             ..Default::default()
         };
-        upsert_session(&session).unwrap();
+        upsert_session_with_conn(&conn, &session).unwrap();
         assert_eq!(
-            read_sessions()
+            read_sessions_with_conn(&conn)
                 .unwrap()
                 .iter()
                 .filter(|s| s.session_id == "remove-me")
                 .count(),
             1
         );
 
-        remove_session("remove-me").unwrap();
+        remove_session_with_conn(&conn, "remove-me").unwrap();
         assert_eq!(
-            read_sessions()
+            read_sessions_with_conn(&conn)
                 .unwrap()
                 .iter()
                 .filter(|s| s.session_id == "remove-me")
@@ -1431,8 +1468,7 @@ mod tests {
 
     #[test]
     fn test_git_branch_roundtrip() {
-        init_test_env();
-        init_db().unwrap();
+        let conn = test_db();
 
         let session = ActiveSession {
             session_id: "git-test".to_string(),
@@ -1441,9 +1477,9 @@ mod tests {
             }),
             ..Default::default()
         };
-        upsert_session(&session).unwrap();
+        upsert_session_with_conn(&conn, &session).unwrap();
 
-        let sessions = read_sessions().unwrap();
+        let sessions = read_sessions_with_conn(&conn).unwrap();
         let s = sessions
             .iter()
             .find(|s| s.session_id == "git-test")
@@ -1456,17 +1492,16 @@ mod tests {
 
     #[test]
     fn test_allowed_tools_roundtrip() {
-        init_test_env();
-        init_db().unwrap();
+        let conn = test_db();
 
         let session = ActiveSession {
             session_id: "tools-test".to_string(),
             session_allowed_tools: vec!["Read".to_string(), "Write".to_string()],
             ..Default::default()
         };
-        upsert_session(&session).unwrap();
+        upsert_session_with_conn(&conn, &session).unwrap();
 
-        let sessions = read_sessions().unwrap();
+        let sessions = read_sessions_with_conn(&conn).unwrap();
         let s = sessions
             .iter()
             .find(|s| s.session_id == "tools-test")
@@ -1476,15 +1511,14 @@ mod tests {
 
     #[test]
     fn test_session_metrics_upsert_and_read() {
-        init_test_env();
-        init_db().unwrap();
+        let conn = test_db();
 
         // Insert parent session first (foreign key)
         let session = ActiveSession {
             session_id: "metrics-parent".to_string(),
             ..Default::default()
         };
-        upsert_session(&session).unwrap();
+        upsert_session_with_conn(&conn, &session).unwrap();
 
         let metrics = SessionMetrics {
             session_id: "metrics-parent".to_string(),
@@ -1496,9 +1530,9 @@ mod tests {
             source: "otel".to_string(),
             updated_at: None,
         };
-        upsert_session_metrics(&metrics).unwrap();
+        upsert_session_metrics_with_conn(&conn, &metrics).unwrap();
 
-        let result = get_session_metrics("metrics-parent").unwrap();
+        let result = get_session_metrics_with_conn(&conn, "metrics-parent").unwrap();
         assert!(result.is_some());
         let m = result.unwrap();
         assert_eq!(m.input_tokens, 500);
@@ -1507,34 +1541,32 @@ mod tests {
 
     #[test]
     fn test_session_metrics_not_found() {
-        init_test_env();
-        init_db().unwrap();
+        let conn = test_db();
 
-        let result = get_session_metrics("nonexistent").unwrap();
+        let result = get_session_metrics_with_conn(&conn, "nonexistent").unwrap();
         assert!(result.is_none());
     }
 
     #[test]
     fn test_cascade_delete_metrics() {
-        init_test_env();
-        init_db().unwrap();
+        let conn = test_db();
 
         let session = ActiveSession {
             session_id: "cascade-test".to_string(),
             ..Default::default()
         };
-        upsert_session(&session).unwrap();
+        upsert_session_with_conn(&conn, &session).unwrap();
 
         let metrics = SessionMetrics {
             session_id: "cascade-test".to_string(),
             input_tokens: 100,
             ..Default::default()
         };
-        upsert_session_metrics(&metrics).unwrap();
+        upsert_session_metrics_with_conn(&conn, &metrics).unwrap();
 
         // Deleting the session should cascade to metrics
-        remove_session("cascade-test").unwrap();
-        let result = get_session_metrics("cascade-test").unwrap();
+        remove_session_with_conn(&conn, "cascade-test").unwrap();
+        let result = get_session_metrics_with_conn(&conn, "cascade-test").unwrap();
         assert!(result.is_none());
     }
 }