Added allow-expect-in-tests / allow-unwrap-in-tests (#2328)
This PR: * Added the clippy.toml to configure allowable expect / unwrap usage in tests * Removed as many expect/allow lines as possible from tests * moved a bunch of allows to expects where possible Note: in integration tests, non `#[test]` helper functions are not covered by this so we had to leave a few lingering `expect(expect_used` checks around
This commit is contained in:
@@ -132,7 +132,6 @@ fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option<Ve
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
use super::*;
|
||||
|
||||
fn parse_seq(src: &str) -> Option<Vec<Vec<String>>> {
|
||||
|
||||
@@ -609,8 +609,6 @@ fn try_parse_retry_after(err: &Error) -> Option<Duration> {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(clippy::expect_used, clippy::unwrap_used)]
|
||||
|
||||
use super::*;
|
||||
use serde_json::json;
|
||||
use tokio::sync::mpsc;
|
||||
|
||||
@@ -187,7 +187,6 @@ impl Stream for ResponseStream {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(clippy::expect_used)]
|
||||
use crate::model_family::find_family_for_model;
|
||||
|
||||
use super::*;
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
// Poisoned mutex should fail the program
|
||||
#![allow(clippy::unwrap_used)]
|
||||
#![expect(clippy::unwrap_used)]
|
||||
|
||||
use std::borrow::Cow;
|
||||
use std::collections::HashMap;
|
||||
|
||||
@@ -765,7 +765,6 @@ pub fn log_dir(cfg: &Config) -> std::io::Result<PathBuf> {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(clippy::expect_used, clippy::unwrap_used)]
|
||||
use crate::config_types::HistoryPersistence;
|
||||
|
||||
use super::*;
|
||||
|
||||
@@ -70,8 +70,6 @@ where
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(clippy::unwrap_used, clippy::expect_used)]
|
||||
|
||||
use super::*;
|
||||
use crate::config_types::ShellEnvironmentPolicyInherit;
|
||||
use maplit::hashmap;
|
||||
|
||||
@@ -99,9 +99,6 @@ async fn run_git_command_with_timeout(args: &[&str], cwd: &Path) -> Option<std::
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(clippy::expect_used)]
|
||||
#![allow(clippy::unwrap_used)]
|
||||
|
||||
use super::*;
|
||||
|
||||
use std::fs;
|
||||
|
||||
@@ -162,7 +162,6 @@ fn is_valid_sed_n_arg(arg: Option<&str>) -> bool {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
use super::*;
|
||||
|
||||
fn vec_str(args: &[&str]) -> Vec<String> {
|
||||
|
||||
@@ -281,7 +281,6 @@ fn is_valid_mcp_server_name(server_name: &str) -> bool {
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[allow(clippy::unwrap_used)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use mcp_types::ToolInputSchema;
|
||||
|
||||
@@ -322,7 +322,6 @@ pub fn create_oss_provider_with_base_url(base_url: &str) -> ModelProviderInfo {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
|
||||
@@ -266,7 +266,6 @@ impl std::ops::Deref for FunctionCallOutputPayload {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -470,7 +470,6 @@ pub(crate) fn get_openai_tools(
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[allow(clippy::expect_used)]
|
||||
mod tests {
|
||||
use crate::model_family::find_family_for_model;
|
||||
use mcp_types::ToolInputSchema;
|
||||
|
||||
@@ -134,8 +134,6 @@ async fn load_first_candidate(
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(clippy::expect_used, clippy::unwrap_used)]
|
||||
|
||||
use super::*;
|
||||
use crate::config::ConfigOverrides;
|
||||
use crate::config::ConfigToml;
|
||||
|
||||
@@ -697,7 +697,6 @@ pub struct Chunk {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
use super::*;
|
||||
|
||||
/// Serialize Event to verify that its JSON representation has the expected
|
||||
|
||||
@@ -245,7 +245,6 @@ fn is_write_patch_constrained_to_writable_paths(
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -122,7 +122,6 @@ fn create_seatbelt_command_args(
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![expect(clippy::expect_used)]
|
||||
use super::MACOS_SEATBELT_BASE_POLICY;
|
||||
use super::create_seatbelt_command_args;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
|
||||
@@ -98,7 +98,6 @@ mod tests {
|
||||
use std::process::Command;
|
||||
|
||||
#[tokio::test]
|
||||
#[expect(clippy::unwrap_used)]
|
||||
async fn test_current_shell_detects_zsh() {
|
||||
let shell = Command::new("sh")
|
||||
.arg("-c")
|
||||
@@ -129,7 +128,6 @@ mod tests {
|
||||
assert_eq!(actual_cmd, None);
|
||||
}
|
||||
|
||||
#[expect(clippy::unwrap_used)]
|
||||
#[tokio::test]
|
||||
async fn test_run_with_profile_escaping_and_execution() {
|
||||
let shell_path = "/bin/zsh";
|
||||
|
||||
@@ -466,7 +466,6 @@ fn is_windows_drive_or_unc_root(p: &std::path::Path) -> bool {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
|
||||
@@ -13,7 +13,6 @@ pub fn get_codex_user_agent(originator: Option<&str>) -> String {
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[allow(clippy::unwrap_used)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
|
||||
@@ -20,7 +20,6 @@ pub(crate) enum UserNotification {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
#![expect(clippy::unwrap_used)]
|
||||
|
||||
use assert_cmd::Command as AssertCommand;
|
||||
use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR;
|
||||
use std::time::Duration;
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
#![allow(clippy::expect_used, clippy::unwrap_used)]
|
||||
|
||||
use codex_core::ConversationManager;
|
||||
use codex_core::ModelProviderInfo;
|
||||
use codex_core::NewConversation;
|
||||
@@ -27,10 +25,12 @@ fn sse_completed(id: &str) -> String {
|
||||
load_sse_fixture_with_id("tests/fixtures/completed_template.json", id)
|
||||
}
|
||||
|
||||
#[expect(clippy::unwrap_used)]
|
||||
fn assert_message_role(request_body: &serde_json::Value, role: &str) {
|
||||
assert_eq!(request_body["role"].as_str().unwrap(), role);
|
||||
}
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
fn assert_message_starts_with(request_body: &serde_json::Value, text: &str) {
|
||||
let content = request_body["content"][0]["text"]
|
||||
.as_str()
|
||||
@@ -42,6 +42,7 @@ fn assert_message_starts_with(request_body: &serde_json::Value, text: &str) {
|
||||
);
|
||||
}
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
fn assert_message_ends_with(request_body: &serde_json::Value, text: &str) {
|
||||
let content = request_body["content"][0]["text"]
|
||||
.as_str()
|
||||
@@ -55,8 +56,6 @@ fn assert_message_ends_with(request_body: &serde_json::Value, text: &str) {
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn includes_session_id_and_model_headers_in_request() {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
|
||||
if std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() {
|
||||
println!(
|
||||
"Skipping test because it cannot execute when network is disabled in a Codex sandbox."
|
||||
@@ -129,8 +128,6 @@ async fn includes_session_id_and_model_headers_in_request() {
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn includes_base_instructions_override_in_request() {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
|
||||
// Mock server
|
||||
let server = MockServer::start().await;
|
||||
|
||||
@@ -187,8 +184,6 @@ async fn includes_base_instructions_override_in_request() {
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn originator_config_override_is_used() {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
|
||||
// Mock server
|
||||
let server = MockServer::start().await;
|
||||
|
||||
@@ -238,8 +233,6 @@ async fn originator_config_override_is_used() {
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn chatgpt_auth_sends_correct_request() {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
|
||||
if std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() {
|
||||
println!(
|
||||
"Skipping test because it cannot execute when network is disabled in a Codex sandbox."
|
||||
@@ -320,8 +313,6 @@ async fn chatgpt_auth_sends_correct_request() {
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn includes_user_instructions_message_in_request() {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
|
||||
let server = MockServer::start().await;
|
||||
|
||||
let first = ResponseTemplate::new(200)
|
||||
@@ -382,8 +373,6 @@ async fn includes_user_instructions_message_in_request() {
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn azure_overrides_assign_properties_used_for_responses_url() {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
|
||||
let existing_env_var_with_random_value = if cfg!(windows) { "USERNAME" } else { "USER" };
|
||||
|
||||
// Mock server
|
||||
@@ -460,8 +449,6 @@ async fn azure_overrides_assign_properties_used_for_responses_url() {
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn env_var_overrides_loaded_auth() {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
|
||||
let existing_env_var_with_random_value = if cfg!(windows) { "USERNAME" } else { "USER" };
|
||||
|
||||
// Mock server
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
#![allow(clippy::expect_used)]
|
||||
#![expect(clippy::expect_used)]
|
||||
|
||||
use tempfile::TempDir;
|
||||
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
#![cfg(target_os = "macos")]
|
||||
#![expect(clippy::unwrap_used, clippy::expect_used)]
|
||||
|
||||
use std::collections::HashMap;
|
||||
|
||||
@@ -24,6 +23,7 @@ fn skip_test() -> bool {
|
||||
false
|
||||
}
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
async fn run_test_cmd(tmp: TempDir, cmd: Vec<&str>) -> Result<ExecToolCallOutput> {
|
||||
let sandbox_type = get_platform_sandbox().expect("should be able to get sandbox type");
|
||||
assert_eq!(sandbox_type, SandboxType::MacosSeatbelt);
|
||||
@@ -68,7 +68,6 @@ async fn truncates_output_lines() {
|
||||
let tmp = TempDir::new().expect("should be able to create temp dir");
|
||||
let cmd = vec!["seq", "300"];
|
||||
|
||||
#[expect(clippy::unwrap_used)]
|
||||
let output = run_test_cmd(tmp, cmd).await.unwrap();
|
||||
|
||||
let expected_output = (1..=256)
|
||||
|
||||
@@ -17,7 +17,7 @@ fn require_api_key() -> String {
|
||||
|
||||
/// Helper that spawns the binary inside a TempDir with minimal flags. Returns (Assert, TempDir).
|
||||
fn run_live(prompt: &str) -> (assert_cmd::assert::Assert, TempDir) {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
#![expect(clippy::unwrap_used)]
|
||||
use std::io::Read;
|
||||
use std::io::Write;
|
||||
use std::thread;
|
||||
@@ -113,7 +113,6 @@ fn run_live(prompt: &str) -> (assert_cmd::assert::Assert, TempDir) {
|
||||
#[ignore]
|
||||
#[test]
|
||||
fn live_create_file_hello_txt() {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
if std::env::var("OPENAI_API_KEY").is_err() {
|
||||
eprintln!("skipping live_create_file_hello_txt – OPENAI_API_KEY not set");
|
||||
return;
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
#![allow(clippy::expect_used, clippy::unwrap_used)]
|
||||
|
||||
use codex_core::ConversationManager;
|
||||
use codex_core::ModelProviderInfo;
|
||||
use codex_core::built_in_model_providers;
|
||||
@@ -24,7 +22,6 @@ fn sse_completed(id: &str) -> String {
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn prefixes_context_and_instructions_once_and_consistently_across_requests() {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
let server = MockServer::start().await;
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
#![cfg(target_os = "macos")]
|
||||
#![expect(clippy::expect_used)]
|
||||
|
||||
//! Tests for the macOS sandboxing that are specific to Seatbelt.
|
||||
//! Tests that apply to both Mac and Linux sandboxing should go in sandbox.rs.
|
||||
@@ -160,6 +159,7 @@ async fn read_only_forbids_all_writes() {
|
||||
.await;
|
||||
}
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
fn create_test_scenario(tmp: &TempDir) -> TestScenario {
|
||||
let repo_parent = tmp.path().to_path_buf();
|
||||
let repo_root = repo_parent.join("repo");
|
||||
@@ -177,6 +177,7 @@ fn create_test_scenario(tmp: &TempDir) -> TestScenario {
|
||||
}
|
||||
}
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
/// Note that `path` must be absolute.
|
||||
async fn touch(path: &Path, policy: &SandboxPolicy) -> bool {
|
||||
assert!(path.is_absolute(), "Path must be absolute: {path:?}");
|
||||
|
||||
@@ -25,7 +25,6 @@ fn sse_completed(id: &str) -> String {
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn continue_after_stream_error() {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
if std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() {
|
||||
println!(
|
||||
"Skipping test because it cannot execute when network is disabled in a Codex sandbox."
|
||||
|
||||
@@ -33,8 +33,6 @@ fn sse_completed(id: &str) -> String {
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn retries_on_early_close() {
|
||||
#![allow(clippy::unwrap_used)]
|
||||
|
||||
if std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() {
|
||||
println!(
|
||||
"Skipping test because it cannot execute when network is disabled in a Codex sandbox."
|
||||
|
||||
Reference in New Issue
Block a user