mirrors/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-05-01 03:42:05 +03:00
Code Issues Packages Projects Releases Wiki Activity

Remove test-support feature from codex-core and replace it with explicit test toggles (#11405)

Browse Source 
## Why

`codex-core` was being built in multiple feature-resolved permutations
because test-only behavior was modeled as crate features. For a large
crate, those permutations increase compile cost and reduce cache reuse.

## Net Change

- Removed the `test-support` crate feature and related feature wiring so
`codex-core` no longer needs separate feature shapes for test consumers.
- Standardized cross-crate test-only access behind
`codex_core::test_support`.
- External test code now imports helpers from
`codex_core::test_support`.
- Underlying implementation hooks are kept internal (`pub(crate)`)
instead of broadly public.

## Outcome

- Fewer `codex-core` build permutations.
- Better incremental cache reuse across test targets.
- No intended production behavior change.
This commit is contained in:
Michael Bolin
2026-02-10 22:44:02 -08:00
committed by GitHub
parent f6dd9e37e7
commit 476c1a7160
36 changed files with 393 additions and 266 deletions
Download Patch File Download Diff File Expand all files Collapse all files

97
codex-rs/core/src/thread_manager.rs
View File

@@ -1,7 +1,5 @@
use crate::AuthManager;
#[cfg(any(test, feature = "test-support"))]
use crate::CodexAuth;
#[cfg(any(test, feature = "test-support"))]
use crate::ModelProviderInfo;
use crate::agent::AgentControl;
use crate::codex::Codex;
@@ -31,25 +29,50 @@ use codex_protocol::protocol::SessionSource;
use std::collections::HashMap;
use std::path::PathBuf;
use std::sync::Arc;
#[cfg(any(test, feature = "test-support"))]
use tempfile::TempDir;
use std::sync::atomic::AtomicBool;
use std::sync::atomic::Ordering;
use tokio::runtime::Handle;
#[cfg(any(test, feature = "test-support"))]
use tokio::runtime::RuntimeFlavor;
use tokio::sync::RwLock;
use tokio::sync::broadcast;
use tracing::warn;
const THREAD_CREATED_CHANNEL_CAPACITY: usize = 1024;
/// Test-only override for enabling thread-manager behaviors used by integration
/// tests.
///
/// In production builds this value should remain at its default (`false`) and
/// must not be toggled.
static FORCE_TEST_THREAD_MANAGER_BEHAVIOR: AtomicBool = AtomicBool::new(false);
type CapturedOps = Vec<(ThreadId, Op)>;
type SharedCapturedOps = Arc<std::sync::Mutex<CapturedOps>>;
pub(crate) fn set_thread_manager_test_mode_for_tests(enabled: bool) {
FORCE_TEST_THREAD_MANAGER_BEHAVIOR.store(enabled, Ordering::Relaxed);
}
fn should_use_test_thread_manager_behavior() -> bool {
FORCE_TEST_THREAD_MANAGER_BEHAVIOR.load(Ordering::Relaxed)
}
struct TempCodexHomeGuard {
path: PathBuf,
}
impl Drop for TempCodexHomeGuard {
fn drop(&mut self) {
let _ = std::fs::remove_dir_all(&self.path);
}
}
fn build_file_watcher(codex_home: PathBuf, skills_manager: Arc<SkillsManager>) -> Arc<FileWatcher> {
#[cfg(any(test, feature = "test-support"))]
if let Ok(handle) = Handle::try_current()
if should_use_test_thread_manager_behavior()
&& let Ok(handle) = Handle::try_current()
&& handle.runtime_flavor() == RuntimeFlavor::CurrentThread
{
// The real watcher spins background tasks that can starve the
// current-thread test runtime and cause event waits to time out.
// Integration tests compile with the `test-support` feature.
warn!("using noop file watcher under current-thread test runtime");
return Arc::new(FileWatcher::noop());
}
@@ -95,8 +118,7 @@ pub struct NewThread {
/// them in memory.
pub struct ThreadManager {
state: Arc<ThreadManagerState>,
#[cfg(any(test, feature = "test-support"))]
_test_codex_home_guard: Option<TempDir>,
_test_codex_home_guard: Option<TempCodexHomeGuard>,
}
/// Shared, `Arc`-owned state for [`ThreadManager`]. This `Arc` is required to have a single
@@ -110,10 +132,8 @@ pub(crate) struct ThreadManagerState {
skills_manager: Arc<SkillsManager>,
file_watcher: Arc<FileWatcher>,
session_source: SessionSource,
#[cfg(any(test, feature = "test-support"))]
#[allow(dead_code)]
// Captures submitted ops for testing purpose.
ops_log: Arc<std::sync::Mutex<Vec<(ThreadId, Op)>>>,
// Captures submitted ops for testing purpose when test mode is enabled.
ops_log: Option<SharedCapturedOps>,
}
impl ThreadManager {
@@ -134,33 +154,40 @@ impl ThreadManager {
file_watcher,
auth_manager,
session_source,
#[cfg(any(test, feature = "test-support"))]
ops_log: Arc::new(std::sync::Mutex::new(Vec::new())),
ops_log: should_use_test_thread_manager_behavior()
.then(|| Arc::new(std::sync::Mutex::new(Vec::new()))),
}),
#[cfg(any(test, feature = "test-support"))]
_test_codex_home_guard: None,
}
}
#[cfg(any(test, feature = "test-support"))]
/// Construct with a dummy AuthManager containing the provided CodexAuth.
/// Used for integration tests: should not be used by ordinary business logic.
pub fn with_models_provider(auth: CodexAuth, provider: ModelProviderInfo) -> Self {
let temp_dir = tempfile::tempdir().unwrap_or_else(|err| panic!("temp codex home: {err}"));
let codex_home = temp_dir.path().to_path_buf();
let mut manager = Self::with_models_provider_and_home(auth, provider, codex_home);
manager._test_codex_home_guard = Some(temp_dir);
pub(crate) fn with_models_provider_for_tests(
auth: CodexAuth,
provider: ModelProviderInfo,
) -> Self {
set_thread_manager_test_mode_for_tests(true);
let codex_home = std::env::temp_dir().join(format!(
"codex-thread-manager-test-{}",
uuid::Uuid::new_v4()
));
std::fs::create_dir_all(&codex_home)
.unwrap_or_else(|err| panic!("temp codex home dir create failed: {err}"));
let mut manager =
Self::with_models_provider_and_home_for_tests(auth, provider, codex_home.clone());
manager._test_codex_home_guard = Some(TempCodexHomeGuard { path: codex_home });
manager
}
#[cfg(any(test, feature = "test-support"))]
/// Construct with a dummy AuthManager containing the provided CodexAuth and codex home.
/// Used for integration tests: should not be used by ordinary business logic.
pub fn with_models_provider_and_home(
pub(crate) fn with_models_provider_and_home_for_tests(
auth: CodexAuth,
provider: ModelProviderInfo,
codex_home: PathBuf,
) -> Self {
set_thread_manager_test_mode_for_tests(true);
let auth_manager = AuthManager::from_auth_for_testing(auth);
let (thread_created_tx, _) = broadcast::channel(THREAD_CREATED_CHANNEL_CAPACITY);
let skills_manager = Arc::new(SkillsManager::new(codex_home.clone()));
@@ -169,7 +196,7 @@ impl ThreadManager {
state: Arc::new(ThreadManagerState {
threads: Arc::new(RwLock::new(HashMap::new())),
thread_created_tx,
models_manager: Arc::new(ModelsManager::with_provider(
models_manager: Arc::new(ModelsManager::with_provider_for_tests(
codex_home,
auth_manager.clone(),
provider,
@@ -178,8 +205,8 @@ impl ThreadManager {
file_watcher,
auth_manager,
session_source: SessionSource::Exec,
#[cfg(any(test, feature = "test-support"))]
ops_log: Arc::new(std::sync::Mutex::new(Vec::new())),
ops_log: should_use_test_thread_manager_behavior()
.then(|| Arc::new(std::sync::Mutex::new(Vec::new()))),
}),
_test_codex_home_guard: None,
}
@@ -340,13 +367,12 @@ impl ThreadManager {
AgentControl::new(Arc::downgrade(&self.state))
}
#[cfg(any(test, feature = "test-support"))]
#[allow(dead_code)]
#[cfg(test)]
pub(crate) fn captured_ops(&self) -> Vec<(ThreadId, Op)> {
self.state
.ops_log
.lock()
.map(|log| log.clone())
.as_ref()
.and_then(|ops_log| ops_log.lock().ok().map(|log| log.clone()))
.unwrap_or_default()
}
}
@@ -364,11 +390,10 @@ impl ThreadManagerState {
/// Send an operation to a thread by ID.
pub(crate) async fn send_op(&self, thread_id: ThreadId, op: Op) -> CodexResult<String> {
let thread = self.get_thread(thread_id).await?;
#[cfg(any(test, feature = "test-support"))]
if let Some(ops_log) = &self.ops_log
&& let Ok(mut log) = ops_log.lock()
{
if let Ok(mut log) = self.ops_log.lock() {
log.push((thread_id, op.clone()));
}
log.push((thread_id, op.clone()));
}
thread.submit(op).await
}