-
Notifications
You must be signed in to change notification settings - Fork 4
test: move inline tests into *_tests.rs files #42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
688e70c
715b854
c205010
67e4bfa
b7f6e50
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| use super::*; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Both newly created test files begin directly with an import and omit the required concise AGENTS.md reference: AGENTS.md:L202-L203 Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Add module descriptions to both new test files. Both files start with imports instead of the required module-level
As per coding guidelines, “Start every 📍 Affects 2 files
🤖 Prompt for AI AgentsSource: Coding guidelines There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| fn call(id: &str) -> NativeToolCall { | ||
| NativeToolCall { | ||
| id: id.into(), | ||
| name: "shell".into(), | ||
| arguments: r#"{"command":"ls"}"#.into(), | ||
| extra_content: None, | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn assistant_envelope_round_trips_byte_exact() { | ||
| let text = encode_assistant_envelope(Some("on it"), &[call("c1"), call("c2")], None); | ||
| let parsed = parse_canonical_assistant_envelope(&text).unwrap(); | ||
| assert_eq!(parsed.content, "on it"); | ||
| assert_eq!(parsed.tool_calls.len(), 2); | ||
| assert_eq!( | ||
| encode_assistant_envelope(Some(&parsed.content), &parsed.tool_calls, None), | ||
| text | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn literal_envelope_shape_is_stable() { | ||
| // Compared as values: key order depends on whether a downstream build | ||
| // unifies serde_json's `preserve_order`, the shape does not. | ||
| let value = |text: String| serde_json::from_str::<Value>(&text).unwrap(); | ||
| assert_eq!( | ||
| value(encode_assistant_envelope(Some("hi"), &[call("c1")], None)), | ||
| serde_json::json!({ | ||
| "content": "hi", | ||
| "tool_calls": [{"id": "c1", "name": "shell", "arguments": "{\"command\":\"ls\"}"}] | ||
| }) | ||
| ); | ||
| assert_eq!( | ||
| value(encode_tool_envelope("c1", "ok")), | ||
| serde_json::json!({"tool_call_id": "c1", "content": "ok"}) | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn non_canonical_assistant_envelopes_stay_opaque() { | ||
| for text in [ | ||
| encode_assistant_envelope(None, &[call("c1")], None), | ||
| encode_assistant_envelope(Some("x"), &[call("c1")], Some("because")), | ||
| r#"{"content":"x","tool_calls":[{"id":"c1","name":"n","arguments":"{}"}],"extra":1}"# | ||
| .to_string(), | ||
| "plain prose".to_string(), | ||
| r#"{"content":"x","tool_calls":[]}"#.to_string(), | ||
| ] { | ||
| assert!( | ||
| parse_canonical_assistant_envelope(&text).is_none(), | ||
| "{text}" | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn tool_envelope_round_trips_and_rejects_extras() { | ||
| let text = encode_tool_envelope("c1", "result \"quoted\""); | ||
| assert_eq!( | ||
| parse_canonical_tool_envelope(&text), | ||
| Some(("c1".into(), "result \"quoted\"".into())) | ||
| ); | ||
| assert!( | ||
| parse_canonical_tool_envelope(r#"{"tool_call_id":"c1","content":"a","name":"n"}"#) | ||
| .is_none() | ||
| ); | ||
| assert!(parse_canonical_tool_envelope("bare").is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn image_parts_split_and_join_exactly() { | ||
| for text in [ | ||
| "just text", | ||
| "[OH_IMAGE:data:image/png;base64,AAAA]", | ||
| "look [OH_IMAGE:data:image/png;base64,AAAA] and [OH_IMAGE:https://x/y.png]\ndone", | ||
| "dangling [OH_IMAGE:data:no-close", | ||
| "", | ||
| ] { | ||
| assert_eq!(join_image_parts(&split_image_parts(text)), text); | ||
| } | ||
| assert_eq!( | ||
| split_image_parts("a[OH_IMAGE:u]b"), | ||
| vec![ | ||
| ContentPart::Text("a".into()), | ||
| ContentPart::Image("u".into()), | ||
| ContentPart::Text("b".into()) | ||
| ] | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -107,7 +107,7 @@ fn native_dialect_covers_non_object_values_fallback_and_protocol_metadata() { | |
|
|
||
| let (text, calls) = dialect.parse_response(&response("narrative only")); | ||
| assert_eq!(text, "narrative only"); | ||
| assert!(calls.is_empty()); | ||
| assert_eq!(calls.len(), 0); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This assertion-only edit makes AGENTS.md reference: AGENTS.md:L314-L316 Useful? React with 👍 / 👎. |
||
| let (text, calls) = dialect.parse_response(&response( | ||
| "narrative <tool_call>{\"name\":\"lookup\",\"arguments\":{}}</tool_call>", | ||
| )); | ||
|
|
@@ -682,7 +682,7 @@ fn native_replay_drops_a_cycle_whose_results_do_not_cover_every_call() { | |
|
|
||
| // Adjacency is not enough: the provider rejects partial coverage the same | ||
| // way it rejects no coverage, so both halves go. | ||
| assert!(NativeDialect.to_provider_messages(&history).is_empty()); | ||
| assert_eq!(NativeDialect.to_provider_messages(&history).len(), 0); | ||
| } | ||
|
|
||
| #[test] | ||
|
|
@@ -692,7 +692,7 @@ fn native_replay_drops_orphan_results() { | |
| "done".to_string(), | ||
| )])]; | ||
|
|
||
| assert!(NativeDialect.to_provider_messages(&history).is_empty()); | ||
| assert_eq!(NativeDialect.to_provider_messages(&history).len(), 0); | ||
| } | ||
|
|
||
| #[test] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Put the module description before the import.
Line 306 says the test file starts with
use super::*;. The coding guideline requires every*_tests.rsfile to start with a concise module-level//!description. Update this instruction to place the module description first, thenuse super::*;.As per coding guidelines, “Start every
mod.rsand*_tests.rswith a concise module-level//!description.”🤖 Prompt for AI Agents
Source: Coding guidelines