mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Test repository-wide Rust formatter discovery (#41925)
## What changed - Exercise Rust source discovery through a mocked `git ls-files` call, including files outside `codex-rs`, paths containing spaces, and stale entries that no longer exist. - Assert that the formatter invokes `rustfmt` from `codex-rs` with the repository configuration in both format and check modes. - Keep the multiline argument-comment lint fixture stable under formatting and update affected Rust test files to the expected style. GitOrigin-RevId: 0cfffde1bb91bf620d2118bed1efc68466c0cca6
This commit is contained in:
committed by
copyberry
parent
64c9cde458
commit
9127f21890
@@ -1,7 +1,7 @@
|
||||
use std::any::Any;
|
||||
use std::collections::HashMap;
|
||||
use std::future::Future;
|
||||
use std::fs;
|
||||
use std::future::Future;
|
||||
use std::panic::AssertUnwindSafe;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
@@ -20,9 +20,9 @@ use tokio::io::BufReader;
|
||||
use tokio::process::Command as TokioCommand;
|
||||
use tokio::time::timeout;
|
||||
|
||||
use super::WineRuntimePaths;
|
||||
use super::WineTestCommand;
|
||||
use super::WineTestProcess;
|
||||
use super::WineRuntimePaths;
|
||||
use super::install_powershell_runtime;
|
||||
|
||||
async fn waiting_smoke_process() -> Result<WineTestProcess> {
|
||||
@@ -314,7 +314,10 @@ async fn pinned_powershell_runs_under_wine_with_a_pty() -> Result<()> {
|
||||
let mut env = std::env::vars().collect::<HashMap<_, _>>();
|
||||
env.remove("DISPLAY");
|
||||
env.extend([
|
||||
("HOME".to_string(), prefix.path().to_string_lossy().into_owned()),
|
||||
(
|
||||
"HOME".to_string(),
|
||||
prefix.path().to_string_lossy().into_owned(),
|
||||
),
|
||||
(
|
||||
"XDG_RUNTIME_DIR".to_string(),
|
||||
prefix.path().to_string_lossy().into_owned(),
|
||||
@@ -431,7 +434,11 @@ async fn pinned_powershell_runs_under_wine_with_a_pty() -> Result<()> {
|
||||
.context("PowerShell smoke marker line was incomplete")?
|
||||
.trim_end_matches('\r');
|
||||
let fields = smoke.split('|').collect::<Vec<_>>();
|
||||
assert_eq!(fields.len(), 5, "unexpected PowerShell smoke output: {smoke}");
|
||||
assert_eq!(
|
||||
fields.len(),
|
||||
5,
|
||||
"unexpected PowerShell smoke output: {smoke}"
|
||||
);
|
||||
assert_eq!(fields[0], POWERSHELL_SMOKE_MARKER);
|
||||
assert_eq!(
|
||||
fields[1].split('.').next(),
|
||||
|
||||
@@ -11,10 +11,11 @@ use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EnvironmentConfigState;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::ExecCommandStatus;
|
||||
use codex_protocol::protocol::ThreadSettingsOverrides;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelections;
|
||||
use codex_protocol::protocol::ThreadSettingsOverrides;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_function_call;
|
||||
@@ -22,11 +23,10 @@ use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::mount_sse_sequence;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::test_codex::TurnInputRequest;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::test_codex::turn_permission_fields;
|
||||
use core_test_support::test_codex::TurnInputRequest;
|
||||
use core_test_support::wait_for_event;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use wine_exec_server_test_support::WineExecServer;
|
||||
|
||||
@@ -16,10 +16,10 @@ const DOCKER_CONTAINER_ENV_VAR: &str = "CODEX_TEST_REMOTE_ENV_CONTAINER_NAME";
|
||||
|
||||
#[tokio::main(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn main() -> Result<()> {
|
||||
let test_binary = PathBuf::from(
|
||||
std::env::var_os(TEST_BINARY_ENV_VAR)
|
||||
.with_context(|| format!("{TEST_BINARY_ENV_VAR} must be set by the Bazel test rule"))?,
|
||||
);
|
||||
let test_binary =
|
||||
PathBuf::from(std::env::var_os(TEST_BINARY_ENV_VAR).with_context(|| {
|
||||
format!("{TEST_BINARY_ENV_VAR} must be set by the Bazel test rule")
|
||||
})?);
|
||||
let forwarded_args = std::env::args_os().skip(1).collect::<Vec<_>>();
|
||||
|
||||
if is_terse_list_request(&forwarded_args) {
|
||||
@@ -28,7 +28,10 @@ async fn main() -> Result<()> {
|
||||
.status()
|
||||
.await
|
||||
.context("list integration tests")?;
|
||||
anyhow::ensure!(status.success(), "listing integration tests exited with {status}");
|
||||
anyhow::ensure!(
|
||||
status.success(),
|
||||
"listing integration tests exited with {status}"
|
||||
);
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
|
||||
@@ -33,8 +33,7 @@ fn plain_lines(text: &Text<'_>) -> Vec<String> {
|
||||
|
||||
#[test]
|
||||
fn bare_url_with_tilde_keeps_complete_hyperlink() {
|
||||
let destination =
|
||||
"https://www.cs.tufts.edu/~nr/cs257/archive/olin-shivers/dissertation.pdf";
|
||||
let destination = "https://www.cs.tufts.edu/~nr/cs257/archive/olin-shivers/dissertation.pdf";
|
||||
let lines = render_markdown_lines_with_width_and_cwd(
|
||||
destination,
|
||||
/*width*/ Some(80),
|
||||
@@ -63,8 +62,7 @@ fn bare_url_with_tilde_keeps_complete_hyperlink() {
|
||||
|
||||
#[test]
|
||||
fn table_url_with_tilde_keeps_complete_hyperlink() {
|
||||
let destination =
|
||||
"https://www.cs.tufts.edu/~nr/cs257/archive/olin-shivers/dissertation.pdf";
|
||||
let destination = "https://www.cs.tufts.edu/~nr/cs257/archive/olin-shivers/dissertation.pdf";
|
||||
let markdown = format!("| URL |\n| --- |\n| {destination} |\n");
|
||||
let lines = render_markdown_lines_with_width_and_cwd(
|
||||
&markdown,
|
||||
@@ -793,8 +791,10 @@ fn web_link_labels_use_link_style_and_preserve_inline_formatting() {
|
||||
fn web_link_labels_keep_link_style_in_wrapped_prose_and_tables() {
|
||||
let links =
|
||||
"[plain](https://example.com) [`code`](https://example.com) [<b>](https://example.com)";
|
||||
for markdown in [links.to_string(), format!("| Links |\n| --- |\n| {links} |")]
|
||||
{
|
||||
for markdown in [
|
||||
links.to_string(),
|
||||
format!("| Links |\n| --- |\n| {links} |"),
|
||||
] {
|
||||
for width in [32, 80] {
|
||||
let text = render_markdown_text_with_width(&markdown, Some(width));
|
||||
let labels = text
|
||||
@@ -836,8 +836,9 @@ fn file_link_hides_destination() {
|
||||
"[codex-rs/tui/src/markdown_render.rs](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs)",
|
||||
Path::new("/Users/example/code/codex"),
|
||||
);
|
||||
let expected =
|
||||
Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs".cyan()]));
|
||||
let expected = Text::from(Line::from_iter([
|
||||
"codex-rs/tui/src/markdown_render.rs".cyan()
|
||||
]));
|
||||
assert_eq!(text, expected);
|
||||
}
|
||||
|
||||
@@ -847,9 +848,7 @@ fn file_link_decodes_percent_encoded_bare_path_destination() {
|
||||
"[report](/Users/example/code/codex/Example%20Folder/R%C3%A9sum%C3%A9/report.md)",
|
||||
Path::new("/Users/example/code/codex"),
|
||||
);
|
||||
let expected = Text::from(Line::from_iter([
|
||||
"Example Folder/Résumé/report.md".cyan(),
|
||||
]));
|
||||
let expected = Text::from(Line::from_iter(["Example Folder/Résumé/report.md".cyan()]));
|
||||
assert_eq!(text, expected);
|
||||
}
|
||||
|
||||
@@ -860,7 +859,7 @@ fn file_link_appends_line_number_when_label_lacks_it() {
|
||||
Path::new("/Users/example/code/codex"),
|
||||
);
|
||||
let expected = Text::from(Line::from_iter([
|
||||
"codex-rs/tui/src/markdown_render.rs:74".cyan(),
|
||||
"codex-rs/tui/src/markdown_render.rs:74".cyan()
|
||||
]));
|
||||
assert_eq!(text, expected);
|
||||
}
|
||||
@@ -871,7 +870,9 @@ fn file_link_keeps_absolute_paths_outside_cwd() {
|
||||
"[README.md:74](/Users/example/code/codex/README.md:74)",
|
||||
Path::new("/Users/example/code/codex/codex-rs/tui"),
|
||||
);
|
||||
let expected = Text::from(Line::from_iter(["/Users/example/code/codex/README.md:74".cyan()]));
|
||||
let expected = Text::from(Line::from_iter([
|
||||
"/Users/example/code/codex/README.md:74".cyan()
|
||||
]));
|
||||
assert_eq!(text, expected);
|
||||
}
|
||||
|
||||
@@ -881,10 +882,9 @@ fn file_link_appends_hash_anchor_when_label_lacks_it() {
|
||||
"[markdown_render.rs](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3)",
|
||||
Path::new("/Users/example/code/codex"),
|
||||
);
|
||||
let expected =
|
||||
Text::from(Line::from_iter([
|
||||
"codex-rs/tui/src/markdown_render.rs:74:3".cyan(),
|
||||
]));
|
||||
let expected = Text::from(Line::from_iter([
|
||||
"codex-rs/tui/src/markdown_render.rs:74:3".cyan(),
|
||||
]));
|
||||
assert_eq!(text, expected);
|
||||
}
|
||||
|
||||
@@ -894,10 +894,9 @@ fn file_link_uses_target_path_for_hash_anchor() {
|
||||
"[markdown_render.rs#L74C3](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3)",
|
||||
Path::new("/Users/example/code/codex"),
|
||||
);
|
||||
let expected =
|
||||
Text::from(Line::from_iter([
|
||||
"codex-rs/tui/src/markdown_render.rs:74:3".cyan(),
|
||||
]));
|
||||
let expected = Text::from(Line::from_iter([
|
||||
"codex-rs/tui/src/markdown_render.rs:74:3".cyan(),
|
||||
]));
|
||||
assert_eq!(text, expected);
|
||||
}
|
||||
|
||||
@@ -907,10 +906,9 @@ fn file_link_appends_range_when_label_lacks_it() {
|
||||
"[markdown_render.rs](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74:3-76:9)",
|
||||
Path::new("/Users/example/code/codex"),
|
||||
);
|
||||
let expected =
|
||||
Text::from(Line::from_iter([
|
||||
"codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan(),
|
||||
]));
|
||||
let expected = Text::from(Line::from_iter([
|
||||
"codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan(),
|
||||
]));
|
||||
assert_eq!(text, expected);
|
||||
}
|
||||
|
||||
@@ -920,10 +918,9 @@ fn file_link_uses_target_path_for_range() {
|
||||
"[markdown_render.rs:74:3-76:9](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74:3-76:9)",
|
||||
Path::new("/Users/example/code/codex"),
|
||||
);
|
||||
let expected =
|
||||
Text::from(Line::from_iter([
|
||||
"codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan(),
|
||||
]));
|
||||
let expected = Text::from(Line::from_iter([
|
||||
"codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan(),
|
||||
]));
|
||||
assert_eq!(text, expected);
|
||||
}
|
||||
|
||||
@@ -933,10 +930,9 @@ fn file_link_appends_hash_range_when_label_lacks_it() {
|
||||
"[markdown_render.rs](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3-L76C9)",
|
||||
Path::new("/Users/example/code/codex"),
|
||||
);
|
||||
let expected =
|
||||
Text::from(Line::from_iter([
|
||||
"codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan(),
|
||||
]));
|
||||
let expected = Text::from(Line::from_iter([
|
||||
"codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan(),
|
||||
]));
|
||||
assert_eq!(text, expected);
|
||||
}
|
||||
|
||||
@@ -960,10 +956,9 @@ fn file_link_uses_target_path_for_hash_range() {
|
||||
"[markdown_render.rs#L74C3-L76C9](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3-L76C9)",
|
||||
Path::new("/Users/example/code/codex"),
|
||||
);
|
||||
let expected =
|
||||
Text::from(Line::from_iter([
|
||||
"codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan(),
|
||||
]));
|
||||
let expected = Text::from(Line::from_iter([
|
||||
"codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan(),
|
||||
]));
|
||||
assert_eq!(text, expected);
|
||||
}
|
||||
|
||||
@@ -1103,7 +1098,10 @@ fn code_block_known_lang_has_syntax_colors() {
|
||||
.iter()
|
||||
.flat_map(|l| l.spans.iter())
|
||||
.any(|sp| sp.style.fg.is_some());
|
||||
assert!(has_colored_span, "expected syntax-highlighted spans with color");
|
||||
assert!(
|
||||
has_colored_span,
|
||||
"expected syntax-highlighted spans with color"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1132,7 +1130,10 @@ fn code_block_unknown_lang_plain() {
|
||||
.iter()
|
||||
.flat_map(|l| l.spans.iter())
|
||||
.any(|sp| sp.style.fg.is_some());
|
||||
assert!(!has_colored_span, "expected no syntax coloring for unknown lang");
|
||||
assert!(
|
||||
!has_colored_span,
|
||||
"expected no syntax coloring for unknown lang"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1310,7 +1311,8 @@ fn list_item_after_code_block_keeps_blank_separator() {
|
||||
|
||||
#[test]
|
||||
fn outer_list_item_after_nested_code_block_keeps_blank_separator() {
|
||||
let md = "1. First:\n - Nested:\n\n ```rust\n fn first() {}\n ```\n\n2. Second:\n";
|
||||
let md =
|
||||
"1. First:\n - Nested:\n\n ```rust\n fn first() {}\n ```\n\n2. Second:\n";
|
||||
let text = render_markdown_text(md);
|
||||
let lines = plain_lines(&text);
|
||||
assert_eq!(
|
||||
@@ -1613,7 +1615,8 @@ fn code_block_preserves_trailing_blank_lines() {
|
||||
"expected a line after 'fn main() {{}}' but content ends: {content:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
content[code_start + 1], "",
|
||||
content[code_start + 1],
|
||||
"",
|
||||
"trailing blank line inside code fence was lost: {content:?}"
|
||||
);
|
||||
}
|
||||
@@ -1636,12 +1639,7 @@ fn table_renders_app_style_rows_with_themed_bold_header() {
|
||||
" 1 2".to_string(),
|
||||
]
|
||||
);
|
||||
assert!(
|
||||
text.lines[0]
|
||||
.style
|
||||
.add_modifier
|
||||
.contains(Modifier::BOLD)
|
||||
);
|
||||
assert!(text.lines[0].style.add_modifier.contains(Modifier::BOLD));
|
||||
assert!(
|
||||
text.lines[0].style.fg.is_some(),
|
||||
"expected the syntax theme to provide a table header accent"
|
||||
@@ -1652,12 +1650,7 @@ fn table_renders_app_style_rows_with_themed_bold_header() {
|
||||
.add_modifier
|
||||
.contains(Modifier::DIM)
|
||||
);
|
||||
assert!(
|
||||
!text.lines[2]
|
||||
.style
|
||||
.add_modifier
|
||||
.contains(Modifier::BOLD)
|
||||
);
|
||||
assert!(!text.lines[2].style.add_modifier.contains(Modifier::BOLD));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1839,7 +1832,11 @@ fn table_falls_back_to_key_value_records_if_grid_cannot_fit() {
|
||||
.collect();
|
||||
|
||||
assert!(lines.first().is_some_and(|line| line.contains("c1")));
|
||||
assert!(lines.iter().any(|line| line.contains("c10") && line.contains("10")));
|
||||
assert!(
|
||||
lines
|
||||
.iter()
|
||||
.any(|line| line.contains("c10") && line.contains("10"))
|
||||
);
|
||||
assert!(
|
||||
!lines
|
||||
.iter()
|
||||
|
||||
@@ -141,9 +141,18 @@ def test_root_fmt_recipes_use_shared_formatter_driver() -> None:
|
||||
|
||||
def test_root_format_driver_covers_all_formatter_groups(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""The shared driver should retain every formatter in both modes."""
|
||||
script = _load_root_format_script_module()
|
||||
for name in (
|
||||
"bazel/rules/example.rs",
|
||||
"codex-rs/src/lib.rs",
|
||||
"codex-rs/new file.rs",
|
||||
):
|
||||
path = tmp_path / name
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_text("")
|
||||
git_ls_files_args = [
|
||||
"git",
|
||||
"ls-files",
|
||||
@@ -153,11 +162,18 @@ def test_root_format_driver_covers_all_formatter_groups(
|
||||
"--exclude-standard",
|
||||
]
|
||||
|
||||
# The Python SDK CI image has no Git; keep discovery mocked at the process boundary.
|
||||
def fake_check_output(args, *, cwd):
|
||||
assert cwd == tmp_path
|
||||
if args == git_ls_files_args + ["--", "*.rs"]:
|
||||
return (
|
||||
b"codex-rs/src/lib.rs\0bazel/rules/example.rs\0"
|
||||
b"codex-rs/new file.rs\0codex-rs/deleted.rs\0"
|
||||
)
|
||||
assert args == git_ls_files_args
|
||||
assert cwd == script.REPO_ROOT
|
||||
return b"MODULE.bazel\0README.md\0third_party/v8/libcxx.BUILD.bazel\0"
|
||||
|
||||
monkeypatch.setattr(script, "REPO_ROOT", tmp_path)
|
||||
monkeypatch.setattr(script.subprocess, "check_output", fake_check_output)
|
||||
formatters = script.formatter_groups(check=False)
|
||||
checks = script.formatter_groups(check=True)
|
||||
@@ -215,20 +231,25 @@ def test_root_format_driver_covers_all_formatter_groups(
|
||||
)
|
||||
assert formatters[0].commands[-1].args == ("just", "--unstable", "--fmt")
|
||||
assert checks[0].commands[-1].args == ("just", "--unstable", "--fmt", "--check")
|
||||
assert formatters[1].commands[-1].args == (
|
||||
"cargo",
|
||||
"fmt",
|
||||
"--",
|
||||
rustfmt_args = (
|
||||
"rustfmt",
|
||||
"--edition",
|
||||
"2024",
|
||||
"--config-path",
|
||||
str(tmp_path / "codex-rs/rustfmt.toml"),
|
||||
"--config",
|
||||
"imports_granularity=Item",
|
||||
"imports_granularity=Item,skip_children=true",
|
||||
)
|
||||
assert checks[1].commands[-1].args == (
|
||||
"cargo",
|
||||
"fmt",
|
||||
"--",
|
||||
"--config",
|
||||
"imports_granularity=Item",
|
||||
"--check",
|
||||
rust_files = (
|
||||
os.path.join("..", "bazel", "rules", "example.rs"),
|
||||
"new file.rs",
|
||||
os.path.join("src", "lib.rs"),
|
||||
)
|
||||
assert formatters[1].commands == (
|
||||
script.Command(rustfmt_args + rust_files, tmp_path / "codex-rs"),
|
||||
)
|
||||
assert checks[1].commands == (
|
||||
script.Command(rustfmt_args + ("--check",) + rust_files, tmp_path / "codex-rs"),
|
||||
)
|
||||
format_buildifier_args = formatters[2].commands[-1].args
|
||||
check_buildifier_args = checks[2].commands[-1].args
|
||||
|
||||
@@ -6,6 +6,8 @@ fn run_git_for_stdout(repo_root: &str, args: Vec<&str>, env: Option<&str>) -> St
|
||||
String::new()
|
||||
}
|
||||
|
||||
// Keep the call split across lines so this fixture tests multiline arguments.
|
||||
#[rustfmt::skip]
|
||||
fn main() {
|
||||
let _ = run_git_for_stdout(
|
||||
"/tmp/repo",
|
||||
|
||||
Reference in New Issue
Block a user