Require absolute paths for test SQLite configuration (#34411)

## Why

`SqliteConfig` stores its home as an `AbsolutePathBuf`, but its test constructor
previously accepted a `PathBuf` and checked the absolute-path invariant at
runtime.

## What changed

- Make `SqliteConfig::new_for_testing` accept an `AbsolutePathBuf` directly,
  removing its fallible conversion and `expect`.
- Update SQLite test setup to convert temporary directory paths with
  `PathExt::abs` at each call site.

GitOrigin-RevId: 93585b9aea805e2449b3465ac20eeb39417ed555
This commit is contained in:
Adam Perry @ OpenAI authored and copyberry committed 2026-07-20 20:54:08 +00:00
1 parent 44481a1c45
commit 81e89fa5af
8 files changed
+26 -27

No files matched your search

+5 -5
View File
@@ -745,6 +745,7 @@ where
mod tests { mod tests {
use super::*; use super::*;
use codex_protocol::ThreadId; use codex_protocol::ThreadId;
use codex_utils_absolute_path::test_support::PathExt;
use pretty_assertions::assert_eq; use pretty_assertions::assert_eq;
use tempfile::TempDir; use tempfile::TempDir;
@@ -1348,11 +1349,10 @@ mod tests {
async fn insert_thread_row(&self, id: &str, rollout_path: &Path, archived: bool) { async fn insert_thread_row(&self, id: &str, rollout_path: &Path, archived: bool) {
let state_db_path = codex_state::state_db_path(self.sqlite_home.path()); let state_db_path = codex_state::state_db_path(self.sqlite_home.path());
let pool = let pool = codex_state::SqliteConfig::new_for_testing(self.sqlite_home.path().abs())
codex_state::SqliteConfig::new_for_testing(self.sqlite_home.path().to_path_buf()) .open_read_write_pool(&state_db_path)
.open_read_write_pool(&state_db_path) .await
.await .expect("sqlite pool");
.expect("sqlite pool");
sqlx::query( sqlx::query(
r#" r#"
INSERT INTO threads ( INSERT INTO threads (
+3 -2
View File
@@ -4,6 +4,7 @@ use anyhow::Result;
use codex_state::StateRuntime; use codex_state::StateRuntime;
use codex_state::memories_db_path; use codex_state::memories_db_path;
use codex_state::state_db_path; use codex_state::state_db_path;
use codex_utils_absolute_path::test_support::PathExt;
use predicates::str::contains; use predicates::str::contains;
use tempfile::TempDir; use tempfile::TempDir;
@@ -16,7 +17,7 @@ fn codex_command(codex_home: &Path) -> Result<assert_cmd::Command> {
#[tokio::test] #[tokio::test]
async fn debug_clear_memories_resets_state_and_removes_memory_dir() -> Result<()> { async fn debug_clear_memories_resets_state_and_removes_memory_dir() -> Result<()> {
let codex_home = TempDir::new()?; let codex_home = TempDir::new()?;
let sqlite = codex_state::SqliteConfig::new_for_testing(codex_home.path().to_path_buf()); let sqlite = codex_state::SqliteConfig::new_for_testing(codex_home.path().abs());
let runtime = let runtime =
StateRuntime::init(codex_home.path().to_path_buf(), "test-provider".to_string()).await?; StateRuntime::init(codex_home.path().to_path_buf(), "test-provider".to_string()).await?;
drop(runtime); drop(runtime);
@@ -139,7 +140,7 @@ INSERT INTO jobs (
#[tokio::test] #[tokio::test]
async fn debug_clear_memories_resets_memories_db_without_state_db() -> Result<()> { async fn debug_clear_memories_resets_memories_db_without_state_db() -> Result<()> {
let codex_home = TempDir::new()?; let codex_home = TempDir::new()?;
let sqlite = codex_state::SqliteConfig::new_for_testing(codex_home.path().to_path_buf()); let sqlite = codex_state::SqliteConfig::new_for_testing(codex_home.path().abs());
let runtime = let runtime =
StateRuntime::init(codex_home.path().to_path_buf(), "test-provider".to_string()).await?; StateRuntime::init(codex_home.path().to_path_buf(), "test-provider".to_string()).await?;
runtime.close().await; runtime.close().await;
+4 -3
View File
@@ -1,3 +1,4 @@
use codex_utils_absolute_path::test_support::PathExt;
use sqlx::Connection; use sqlx::Connection;
use sqlx::Row; use sqlx::Row;
use sqlx::migrate::Migration; use sqlx::migrate::Migration;
@@ -35,7 +36,7 @@ async fn recency_migration_backfills_and_seeds_old_binary_inserts() {
let _cleanup = scopeguard::guard(sqlite_home.clone(), |sqlite_home| { let _cleanup = scopeguard::guard(sqlite_home.clone(), |sqlite_home| {
let _ = std::fs::remove_dir_all(sqlite_home); let _ = std::fs::remove_dir_all(sqlite_home);
}); });
let sqlite = crate::SqliteConfig::new_for_testing(sqlite_home.clone()); let sqlite = crate::SqliteConfig::new_for_testing(sqlite_home.as_path().abs());
let pool = sqlite let pool = sqlite
.open_read_write_pool(&state_db_path(&sqlite_home)) .open_read_write_pool(&state_db_path(&sqlite_home))
.await .await
@@ -148,7 +149,7 @@ async fn repairs_recency_migration_that_was_applied_as_version_38() {
let _cleanup = scopeguard::guard(sqlite_home.clone(), |sqlite_home| { let _cleanup = scopeguard::guard(sqlite_home.clone(), |sqlite_home| {
let _ = std::fs::remove_dir_all(sqlite_home); let _ = std::fs::remove_dir_all(sqlite_home);
}); });
let sqlite = crate::SqliteConfig::new_for_testing(sqlite_home.clone()); let sqlite = crate::SqliteConfig::new_for_testing(sqlite_home.as_path().abs());
let pool = sqlite let pool = sqlite
.open_read_write_pool(&state_db_path(&sqlite_home)) .open_read_write_pool(&state_db_path(&sqlite_home))
.await .await
@@ -224,7 +225,7 @@ async fn repair_recency_migration_succeeds_while_another_connection_holds_writer
let _cleanup = scopeguard::guard(sqlite_home.clone(), |sqlite_home| { let _cleanup = scopeguard::guard(sqlite_home.clone(), |sqlite_home| {
let _ = std::fs::remove_dir_all(sqlite_home); let _ = std::fs::remove_dir_all(sqlite_home);
}); });
let sqlite = crate::SqliteConfig::new_for_testing(sqlite_home.clone()); let sqlite = crate::SqliteConfig::new_for_testing(sqlite_home.as_path().abs());
let state_path = state_db_path(&sqlite_home); let state_path = state_db_path(&sqlite_home);
let pool = sqlite let pool = sqlite
.open_read_write_pool(&state_path) .open_read_write_pool(&state_path)
+5 -4
View File
@@ -578,6 +578,7 @@ mod tests {
use crate::DB_INIT_METRIC; use crate::DB_INIT_METRIC;
use crate::DbTelemetry; use crate::DbTelemetry;
use crate::migrations::STATE_MIGRATOR; use crate::migrations::STATE_MIGRATOR;
use codex_utils_absolute_path::test_support::PathExt;
use pretty_assertions::assert_eq; use pretty_assertions::assert_eq;
use sqlx::SqlitePool; use sqlx::SqlitePool;
use sqlx::migrate::MigrateError; use sqlx::migrate::MigrateError;
@@ -638,7 +639,7 @@ mod tests {
} }
async fn open_db_pool(path: &Path) -> SqlitePool { async fn open_db_pool(path: &Path) -> SqlitePool {
crate::SqliteConfig::new_for_testing(path.parent().unwrap_or(path).to_path_buf()) crate::SqliteConfig::new_for_testing(path.parent().unwrap_or(path).abs())
.open_read_write_pool(path) .open_read_write_pool(path)
.await .await
.expect("open sqlite pool") .expect("open sqlite pool")
@@ -651,7 +652,7 @@ mod tests {
.await .await
.expect("create codex home"); .expect("create codex home");
let path = state_db_path(codex_home.as_path()); let path = state_db_path(codex_home.as_path());
let pool = crate::SqliteConfig::new_for_testing(codex_home.clone()) let pool = crate::SqliteConfig::new_for_testing(codex_home.as_path().abs())
.open_read_write_pool(&path) .open_read_write_pool(&path)
.await .await
.expect("open sqlite db"); .expect("open sqlite db");
@@ -676,7 +677,7 @@ mod tests {
.await .await
.expect("create codex home"); .expect("create codex home");
let state_path = state_db_path(codex_home.as_path()); let state_path = state_db_path(codex_home.as_path());
let pool = crate::SqliteConfig::new_for_testing(codex_home.clone()) let pool = crate::SqliteConfig::new_for_testing(codex_home.as_path().abs())
.open_read_write_pool(&state_path) .open_read_write_pool(&state_path)
.await .await
.expect("open state db"); .expect("open state db");
@@ -707,7 +708,7 @@ mod tests {
let tolerant_migrator = runtime_state_migrator(); let tolerant_migrator = runtime_state_migrator();
let tolerant_pool = open_state_sqlite( let tolerant_pool = open_state_sqlite(
&crate::SqliteConfig::new_for_testing(codex_home.clone()), &crate::SqliteConfig::new_for_testing(codex_home.as_path().abs()),
state_path.as_path(), state_path.as_path(),
&tolerant_migrator, &tolerant_migrator,
/*telemetry_override*/ None, /*telemetry_override*/ None,
+2 -1
View File
@@ -112,6 +112,7 @@ mod tests {
use super::StateRuntime; use super::StateRuntime;
use super::test_support::unique_temp_dir; use super::test_support::unique_temp_dir;
use chrono::Utc; use chrono::Utc;
use codex_utils_absolute_path::test_support::PathExt;
use pretty_assertions::assert_eq; use pretty_assertions::assert_eq;
use sqlx::Connection; use sqlx::Connection;
@@ -174,7 +175,7 @@ mod tests {
let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string()) let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string())
.await .await
.expect("initialize runtime"); .expect("initialize runtime");
let write_pool = crate::SqliteConfig::new_for_testing(codex_home.clone()) let write_pool = crate::SqliteConfig::new_for_testing(codex_home.as_path().abs())
.open_read_write_pool(&crate::state_db_path(codex_home.as_path())) .open_read_write_pool(&crate::state_db_path(codex_home.as_path()))
.await .await
.expect("open write pool"); .expect("open write pool");
+3 -2
View File
@@ -548,6 +548,7 @@ mod tests {
use crate::logs_db_path; use crate::logs_db_path;
use crate::migrations::LOGS_MIGRATOR; use crate::migrations::LOGS_MIGRATOR;
use chrono::Utc; use chrono::Utc;
use codex_utils_absolute_path::test_support::PathExt;
use pretty_assertions::assert_eq; use pretty_assertions::assert_eq;
use sqlx::SqlitePool; use sqlx::SqlitePool;
use sqlx::migrate::Migrator; use sqlx::migrate::Migrator;
@@ -555,7 +556,7 @@ mod tests {
use std::path::Path; use std::path::Path;
async fn open_db_pool(path: &Path) -> SqlitePool { async fn open_db_pool(path: &Path) -> SqlitePool {
crate::SqliteConfig::new_for_testing(path.parent().unwrap_or(path).to_path_buf()) crate::SqliteConfig::new_for_testing(path.parent().unwrap_or(path).abs())
.open_read_write_pool(path) .open_read_write_pool(path)
.await .await
.expect("open sqlite pool") .expect("open sqlite pool")
@@ -617,7 +618,7 @@ mod tests {
table_name: LOGS_MIGRATOR.table_name.clone(), table_name: LOGS_MIGRATOR.table_name.clone(),
create_schemas: LOGS_MIGRATOR.create_schemas.clone(), create_schemas: LOGS_MIGRATOR.create_schemas.clone(),
}; };
let pool = crate::SqliteConfig::new_for_testing(codex_home.clone()) let pool = crate::SqliteConfig::new_for_testing(codex_home.as_path().abs())
.open_read_write_pool(&logs_path) .open_read_write_pool(&logs_path)
.await .await
.expect("open old logs db"); .expect("open old logs db");
+2 -1
View File
@@ -158,6 +158,7 @@ mod tests {
use super::test_support::unique_temp_dir; use super::test_support::unique_temp_dir;
use crate::migrations::STATE_MIGRATOR; use crate::migrations::STATE_MIGRATOR;
use crate::state_db_path; use crate::state_db_path;
use codex_utils_absolute_path::test_support::PathExt;
use pretty_assertions::assert_eq; use pretty_assertions::assert_eq;
use sqlx::migrate::Migrator; use sqlx::migrate::Migrator;
use std::borrow::Cow; use std::borrow::Cow;
@@ -336,7 +337,7 @@ mod tests {
table_name: STATE_MIGRATOR.table_name.clone(), table_name: STATE_MIGRATOR.table_name.clone(),
create_schemas: STATE_MIGRATOR.create_schemas.clone(), create_schemas: STATE_MIGRATOR.create_schemas.clone(),
}; };
let pool = crate::SqliteConfig::new_for_testing(codex_home.clone()) let pool = crate::SqliteConfig::new_for_testing(codex_home.as_path().abs())
.open_read_write_pool(&state_db_path(codex_home.as_path())) .open_read_write_pool(&state_db_path(codex_home.as_path()))
.await .await
.expect("open old state db"); .expect("open old state db");
+2 -9
View File
@@ -11,7 +11,6 @@ use sqlx::sqlite::SqliteJournalMode;
use sqlx::sqlite::SqlitePoolOptions; use sqlx::sqlite::SqlitePoolOptions;
use sqlx::sqlite::SqliteSynchronous; use sqlx::sqlite::SqliteSynchronous;
use std::path::Path; use std::path::Path;
use std::path::PathBuf;
use std::time::Duration; use std::time::Duration;
/// Resolved configuration shared by all Codex SQLite connections. /// Resolved configuration shared by all Codex SQLite connections.
@@ -25,14 +24,8 @@ impl SqliteConfig {
Self { sqlite_home } Self { sqlite_home }
} }
#[expect( pub fn new_for_testing(sqlite_home: AbsolutePathBuf) -> Self {
clippy::expect_used, Self::from_sqlite_home(sqlite_home)
reason = "test sqlite homes must already be absolute"
)]
pub fn new_for_testing(sqlite_home: PathBuf) -> Self {
Self::from_sqlite_home(
AbsolutePathBuf::try_from(sqlite_home).expect("sqlite home should be absolute"),
)
} }
pub fn home(&self) -> &Path { pub fn home(&self) -> &Path {