fix: enable clippy on tests (#870)

https://github.com/openai/codex/pull/855 added the clippy warning to
disallow `unwrap()`, but apparently we were not verifying that tests
were "clippy clean" in CI, so I ended up with a lot of local errors in
VS Code.

This turns on the check in CI and fixes the offenders.
This commit is contained in:
Michael Bolin
2025-05-08 16:02:56 -07:00
committed by GitHub
parent 699ec5a87f
commit a9adb4175c
13 changed files with 31 additions and 11 deletions

View File

@@ -81,7 +81,7 @@ jobs:
run: echo "FAILED=" >> $GITHUB_ENV run: echo "FAILED=" >> $GITHUB_ENV
- name: cargo clippy - name: cargo clippy
run: cargo clippy --target ${{ matrix.target }} --all-features -- -D warnings || echo "FAILED=${FAILED:+$FAILED, }cargo clippy" >> $GITHUB_ENV run: cargo clippy --target ${{ matrix.target }} --all-features --tests -- -D warnings || echo "FAILED=${FAILED:+$FAILED, }cargo clippy" >> $GITHUB_ENV
# Running `cargo build` from the workspace root builds the workspace using # Running `cargo build` from the workspace root builds the workspace using
# the union of all features from third-party crates. This can mask errors # the union of all features from third-party crates. This can mask errors

View File

@@ -194,6 +194,7 @@ fn is_valid_sed_n_arg(arg: Option<&str>) -> bool {
} }
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
#![allow(clippy::unwrap_used)]
use super::*; use super::*;
fn vec_str(args: &[&str]) -> Vec<String> { fn vec_str(args: &[&str]) -> Vec<String> {

View File

@@ -179,7 +179,9 @@ fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(),
} }
#[cfg(test)] #[cfg(test)]
mod tests_linux { mod tests {
#![allow(clippy::unwrap_used)]
use super::*; use super::*;
use crate::exec::ExecParams; use crate::exec::ExecParams;
use crate::exec::SandboxType; use crate::exec::SandboxType;

View File

@@ -163,6 +163,7 @@ impl std::ops::Deref for FunctionCallOutputPayload {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
#![allow(clippy::unwrap_used)]
use super::*; use super::*;
#[test] #[test]

View File

@@ -189,6 +189,7 @@ fn is_write_patch_constrained_to_writable_paths(
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
#![allow(clippy::unwrap_used)]
use super::*; use super::*;
#[test] #[test]

View File

@@ -20,6 +20,7 @@ pub(crate) enum UserNotification {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
#![allow(clippy::unwrap_used)]
use super::*; use super::*;
#[test] #[test]

View File

@@ -19,6 +19,7 @@ use std::time::Duration;
use codex_core::Codex; use codex_core::Codex;
use codex_core::config::Config; use codex_core::config::Config;
use codex_core::error::CodexErr;
use codex_core::protocol::EventMsg; use codex_core::protocol::EventMsg;
use codex_core::protocol::InputItem; use codex_core::protocol::InputItem;
use codex_core::protocol::Op; use codex_core::protocol::Op;
@@ -32,7 +33,7 @@ fn api_key_available() -> bool {
/// Helper that spawns a fresh Agent and sends the mandatory *ConfigureSession* /// Helper that spawns a fresh Agent and sends the mandatory *ConfigureSession*
/// submission. The caller receives the constructed [`Agent`] plus the unique /// submission. The caller receives the constructed [`Agent`] plus the unique
/// submission id used for the initialization message. /// submission id used for the initialization message.
async fn spawn_codex() -> Codex { async fn spawn_codex() -> Result<Codex, CodexErr> {
assert!( assert!(
api_key_available(), api_key_available(),
"OPENAI_API_KEY must be set for live tests" "OPENAI_API_KEY must be set for live tests"
@@ -53,11 +54,9 @@ async fn spawn_codex() -> Codex {
} }
let config = Config::load_default_config_for_test(); let config = Config::load_default_config_for_test();
let (agent, _init_id) = Codex::spawn(config, std::sync::Arc::new(Notify::new())) let (agent, _init_id) = Codex::spawn(config, std::sync::Arc::new(Notify::new())).await?;
.await
.unwrap();
agent Ok(agent)
} }
/// Verifies that the agent streams incremental *AgentMessage* events **before** /// Verifies that the agent streams incremental *AgentMessage* events **before**
@@ -66,12 +65,13 @@ async fn spawn_codex() -> Codex {
#[ignore] #[ignore]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn live_streaming_and_prev_id_reset() { async fn live_streaming_and_prev_id_reset() {
#![allow(clippy::unwrap_used)]
if !api_key_available() { if !api_key_available() {
eprintln!("skipping live_streaming_and_prev_id_reset OPENAI_API_KEY not set"); eprintln!("skipping live_streaming_and_prev_id_reset OPENAI_API_KEY not set");
return; return;
} }
let codex = spawn_codex().await; let codex = spawn_codex().await.unwrap();
// ---------- Task 1 ---------- // ---------- Task 1 ----------
codex codex
@@ -140,12 +140,13 @@ async fn live_streaming_and_prev_id_reset() {
#[ignore] #[ignore]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn live_shell_function_call() { async fn live_shell_function_call() {
#![allow(clippy::unwrap_used)]
if !api_key_available() { if !api_key_available() {
eprintln!("skipping live_shell_function_call OPENAI_API_KEY not set"); eprintln!("skipping live_shell_function_call OPENAI_API_KEY not set");
return; return;
} }
let codex = spawn_codex().await; let codex = spawn_codex().await.unwrap();
const MARKER: &str = "codex_live_echo_ok"; const MARKER: &str = "codex_live_echo_ok";

View File

@@ -15,6 +15,7 @@ fn require_api_key() -> String {
/// Helper that spawns the binary inside a TempDir with minimal flags. Returns (Assert, TempDir). /// Helper that spawns the binary inside a TempDir with minimal flags. Returns (Assert, TempDir).
fn run_live(prompt: &str) -> (assert_cmd::assert::Assert, TempDir) { fn run_live(prompt: &str) -> (assert_cmd::assert::Assert, TempDir) {
#![allow(clippy::unwrap_used)]
use std::io::Read; use std::io::Read;
use std::io::Write; use std::io::Write;
use std::thread; use std::thread;
@@ -110,6 +111,7 @@ fn run_live(prompt: &str) -> (assert_cmd::assert::Assert, TempDir) {
#[ignore] #[ignore]
#[test] #[test]
fn live_create_file_hello_txt() { fn live_create_file_hello_txt() {
#![allow(clippy::unwrap_used)]
if std::env::var("OPENAI_API_KEY").is_err() { if std::env::var("OPENAI_API_KEY").is_err() {
eprintln!("skipping live_create_file_hello_txt OPENAI_API_KEY not set"); eprintln!("skipping live_create_file_hello_txt OPENAI_API_KEY not set");
return; return;

View File

@@ -48,6 +48,8 @@ data: {{\"type\":\"response.completed\",\"response\":{{\"id\":\"{}\",\"output\":
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn keeps_previous_response_id_between_tasks() { async fn keeps_previous_response_id_between_tasks() {
#![allow(clippy::unwrap_used)]
// Mock server // Mock server
let server = MockServer::start().await; let server = MockServer::start().await;

View File

@@ -32,6 +32,8 @@ data: {{\"type\":\"response.completed\",\"response\":{{\"id\":\"{}\",\"output\":
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn retries_on_early_close() { async fn retries_on_early_close() {
#![allow(clippy::unwrap_used)]
let server = MockServer::start().await; let server = MockServer::start().await;
struct SeqResponder; struct SeqResponder;

View File

@@ -140,6 +140,7 @@ fn is_executable_file(path: &str) -> bool {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
#![allow(clippy::unwrap_used)]
use tempfile::TempDir; use tempfile::TempDir;
use super::*; use super::*;

View File

@@ -67,7 +67,10 @@ fn test_head_one_flag_one_file() -> Result<()> {
exec: ValidExec { exec: ValidExec {
program: "head".to_string(), program: "head".to_string(),
flags: vec![], flags: vec![],
opts: vec![MatchedOpt::new("-n", "100", ArgType::PositiveInteger).unwrap()], opts: vec![
MatchedOpt::new("-n", "100", ArgType::PositiveInteger)
.expect("should validate")
],
args: vec![MatchedArg::new( args: vec![MatchedArg::new(
2, 2,
ArgType::ReadableFile, ArgType::ReadableFile,

View File

@@ -47,7 +47,10 @@ fn test_sed_print_specific_lines_with_e_flag() -> Result<()> {
exec: ValidExec { exec: ValidExec {
program: "sed".to_string(), program: "sed".to_string(),
flags: vec![MatchedFlag::new("-n")], flags: vec![MatchedFlag::new("-n")],
opts: vec![MatchedOpt::new("-e", "122,202p", ArgType::SedCommand).unwrap()], opts: vec![
MatchedOpt::new("-e", "122,202p", ArgType::SedCommand)
.expect("should validate")
],
args: vec![MatchedArg::new(3, ArgType::ReadableFile, "hello.txt")?], args: vec![MatchedArg::new(3, ArgType::ReadableFile, "hello.txt")?],
system_path: vec!["/usr/bin/sed".to_string()], system_path: vec!["/usr/bin/sed".to_string()],
} }