Skip to content

Commit 5d5f81e

Browse files
committed
♻️ Replace unwrap with expect for clearer test assertions
Replace unwrap() calls with expect() across test files to provide clear error context when assertions fail. Also refactor raw string literals to use single-character escape sequences where appropriate. Changes: - Use expect("should be err") instead of unwrap_err() for failed validation tests in context.rs, status_messages.rs, and providers.rs - Use expect("should have current") for status message batch navigation tests - Replace Vec literal with array literal in status_messages.rs test scenarios - Switch #[ignore] to #[ignore = "reason"] with descriptive message - Replace r#"..."# raw strings with r"..." for single-quote content in changelog_tests.rs, pr_tests.rs, and test_utils.rs Improves test failure diagnostics without changing test behavior.
1 parent d66dd2f commit 5d5f81e

8 files changed

Lines changed: 58 additions & 31 deletions

File tree

src/agents/context.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,12 @@ mod tests {
300300
fn test_review_from_without_to_fails() {
301301
let result = TaskContext::for_review(None, Some("main".to_string()), None, false);
302302
assert!(result.is_err());
303-
assert!(result.unwrap_err().to_string().contains("--to"));
303+
assert!(
304+
result
305+
.expect_err("should be err")
306+
.to_string()
307+
.contains("--to")
308+
);
304309
}
305310

306311
#[test]
@@ -315,7 +320,7 @@ mod tests {
315320
assert!(result.is_err());
316321
assert!(
317322
result
318-
.unwrap_err()
323+
.expect_err("should be err")
319324
.to_string()
320325
.contains("mutually exclusive")
321326
);
@@ -330,7 +335,12 @@ mod tests {
330335
true,
331336
);
332337
assert!(result.is_err());
333-
assert!(result.unwrap_err().to_string().contains("include-unstaged"));
338+
assert!(
339+
result
340+
.expect_err("should be err")
341+
.to_string()
342+
.contains("include-unstaged")
343+
);
334344
}
335345

336346
#[test]

src/agents/status_messages.rs

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -496,13 +496,22 @@ mod tests {
496496
});
497497

498498
assert_eq!(batch.len(), 2);
499-
assert_eq!(batch.current().unwrap().message, "First");
499+
assert_eq!(
500+
batch.current().expect("should have current").message,
501+
"First"
502+
);
500503

501504
batch.next();
502-
assert_eq!(batch.current().unwrap().message, "Second");
505+
assert_eq!(
506+
batch.current().expect("should have current").message,
507+
"Second"
508+
);
503509

504510
batch.next();
505-
assert_eq!(batch.current().unwrap().message, "First"); // Cycles back
511+
assert_eq!(
512+
batch.current().expect("should have current").message,
513+
"First"
514+
); // Cycles back
506515
}
507516

508517
#[test]
@@ -515,17 +524,17 @@ mod tests {
515524
assert!(prompt.contains("commit"));
516525
assert!(prompt.contains("analyzing staged changes"));
517526
assert!(prompt.contains("feature/awesome"));
518-
assert!(prompt.contains("3"));
527+
assert!(prompt.contains('3'));
519528
}
520529

521530
/// Debug test to evaluate status message quality
522-
/// Run with: cargo test debug_status_messages -- --ignored --nocapture
531+
/// Run with: cargo test `debug_status_messages` -- --ignored --nocapture
523532
#[test]
524-
#[ignore]
533+
#[ignore = "manual debug test for evaluating status message quality"]
525534
fn debug_status_messages() {
526535
use tokio::runtime::Runtime;
527536

528-
let rt = Runtime::new().unwrap();
537+
let rt = Runtime::new().expect("failed to create tokio runtime");
529538
rt.block_on(async {
530539
// Get provider/model from env or use defaults
531540
let provider =
@@ -543,7 +552,7 @@ mod tests {
543552
let generator = StatusMessageGenerator::new(&provider, &model, None);
544553

545554
// Test scenarios
546-
let scenarios = vec![
555+
let scenarios = [
547556
StatusContext::new("commit", "crafting commit message")
548557
.with_branch("main")
549558
.with_files(vec![
@@ -589,7 +598,7 @@ mod tests {
589598

590599
// Generate 5 messages for each scenario
591600
for j in 1..=5 {
592-
let msg = generator.generate(&ctx).await;
601+
let msg = generator.generate(ctx).await;
593602
println!(" {}: {}", j, msg.message);
594603
}
595604
println!();

src/providers.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -339,15 +339,15 @@ mod tests {
339339
fn test_api_key_validation_too_short() {
340340
let result = Provider::OpenAI.validate_api_key_format("sk-short");
341341
assert!(result.is_err());
342-
assert!(result.unwrap_err().contains("too short"));
342+
assert!(result.expect_err("should be err").contains("too short"));
343343
}
344344

345345
#[test]
346346
fn test_api_key_validation_wrong_prefix_openai() {
347347
// Long enough but wrong prefix
348348
let result = Provider::OpenAI.validate_api_key_format("wrong-prefix-1234567890abcdef");
349349
assert!(result.is_err());
350-
let err = result.unwrap_err();
350+
let err = result.expect_err("should be err");
351351
assert!(err.contains("should start with"));
352352
// Error should mention valid prefixes
353353
assert!(err.contains("'sk-'") || err.contains("'sk-proj-'"));
@@ -360,7 +360,7 @@ mod tests {
360360
// Has sk- but not sk-ant- (might be OpenAI key used for Anthropic)
361361
let result = Provider::Anthropic.validate_api_key_format("sk-1234567890abcdefghijklmnop");
362362
assert!(result.is_err());
363-
let err = result.unwrap_err();
363+
let err = result.expect_err("should be err");
364364
assert!(err.contains("sk-ant-"));
365365
// Verify we don't expose the actual key content
366366
assert!(err.contains("unexpected prefix"));

tests/changelog_tests.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ fn setup_test_repo() -> Result<(TempDir, Repository)> {
1919
#[test]
2020
fn test_markdown_changelog_format() {
2121
let changelog = MarkdownChangelog {
22-
content: r#"## [1.0.0] - 2023-06-01
22+
content: r"## [1.0.0] - 2023-06-01
2323
2424
This release adds new features and fixes bugs.
2525
@@ -42,7 +42,7 @@ This release adds new features and fixes bugs.
4242
- Files Changed: 12
4343
- Insertions: +245
4444
- Deletions: -87
45-
"#
45+
"
4646
.to_string(),
4747
};
4848

@@ -114,7 +114,7 @@ fn test_markdown_release_notes_format() {
114114
use git_iris::types::MarkdownReleaseNotes;
115115

116116
let release_notes = MarkdownReleaseNotes {
117-
content: r#"# Release Notes v1.0.0
117+
content: r"# Release Notes v1.0.0
118118
119119
**Released:** 2023-06-01
120120
@@ -133,7 +133,7 @@ A great new feature was added.
133133
## Breaking Changes
134134
135135
- API endpoint changed
136-
"#
136+
"
137137
.to_string(),
138138
};
139139

tests/pr_tests.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ fn test_format_pull_request() {
4545
#[test]
4646
fn test_format_pull_request_minimal() {
4747
let pr = MarkdownPullRequest {
48-
content: r#"# Fix bug in user authentication
48+
content: r"# Fix bug in user authentication
4949
5050
## Summary
5151
@@ -58,7 +58,7 @@ This PR fixes an issue where users couldn't log in properly.
5858
## Commits
5959
6060
- `abc1234`: Fix authentication bug
61-
"#
61+
"
6262
.to_string(),
6363
};
6464

@@ -166,7 +166,7 @@ async fn test_git_repo_pr_methods() -> Result<()> {
166166
#[test]
167167
fn test_format_pull_request_with_unicode() {
168168
let pr = MarkdownPullRequest {
169-
content: r#"# 🚀 Add deployment automation
169+
content: r"# 🚀 Add deployment automation
170170
171171
## Summary
172172
@@ -194,7 +194,7 @@ Test with 🧪 test suite
194194
## Notes
195195
196196
Deployment requires 🔑 secrets
197-
"#
197+
"
198198
.to_string(),
199199
};
200200

tests/remote_repository_integration_tests.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,11 @@ async fn test_cli_with_remote_repository() -> Result<()> {
2727
// 1. Test ReleaseNotes command with repository URL
2828
let common = CommonParams {
2929
provider: Some("mock".to_string()), // Use mock provider to avoid real API calls
30+
model: None,
3031
instructions: None,
3132
preset: None,
33+
gitmoji_flag: false,
34+
no_gitmoji: false,
3235
gitmoji: Some(false),
3336
repository_url: Some(repo_url.to_string()),
3437
};
@@ -37,11 +40,14 @@ async fn test_cli_with_remote_repository() -> Result<()> {
3740
common: common.clone(),
3841
from: "v1.0.0".to_string(), // Use a tag that's likely to exist in the repo
3942
to: Some("HEAD".to_string()),
43+
raw: false,
44+
update: false,
45+
file: None,
4046
version_name: None,
4147
};
4248

4349
// Just testing that it doesn't panic, we're not making actual API calls
44-
let result = git_iris::cli::handle_command(release_notes_command, None, false).await;
50+
let result = git_iris::cli::handle_command(release_notes_command, None).await;
4551
assert!(
4652
result.is_err(),
4753
"Command should fail because we're using a mock provider"
@@ -52,13 +58,14 @@ async fn test_cli_with_remote_repository() -> Result<()> {
5258
common: common.clone(),
5359
from: "v1.0.0".to_string(),
5460
to: Some("HEAD".to_string()),
61+
raw: false,
5562
file: None,
5663
update: false,
5764
version_name: None,
5865
};
5966

6067
// Just testing that it doesn't panic
61-
let result = git_iris::cli::handle_command(changelog_command, None, false).await;
68+
let result = git_iris::cli::handle_command(changelog_command, None).await;
6269
assert!(
6370
result.is_err(),
6471
"Command should fail because we're using a mock provider"
@@ -68,14 +75,15 @@ async fn test_cli_with_remote_repository() -> Result<()> {
6875
let review_command = Commands::Review {
6976
common: common.clone(),
7077
print: true,
78+
raw: false,
7179
commit: None,
7280
include_unstaged: false,
7381
from: None,
7482
to: None,
7583
};
7684

7785
// Just testing that it doesn't panic
78-
let result = git_iris::cli::handle_command(review_command, None, false).await;
86+
let result = git_iris::cli::handle_command(review_command, None).await;
7987
assert!(
8088
result.is_err(),
8189
"Command should fail because we're using a mock provider"
@@ -85,13 +93,13 @@ async fn test_cli_with_remote_repository() -> Result<()> {
8593
let gen_command = Commands::Gen {
8694
common,
8795
auto_commit: false,
88-
no_gitmoji: true,
8996
print: true,
9097
no_verify: true,
98+
amend: false,
9199
};
92100

93101
// Just testing that it doesn't panic
94-
let result = git_iris::cli::handle_command(gen_command, None, false).await;
102+
let result = git_iris::cli::handle_command(gen_command, None).await;
95103
assert!(
96104
result.is_err(),
97105
"Command should fail because we're using a mock provider"

tests/review_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Tests for review functionality
22
//!
3-
//! Note: Legacy GeneratedReview tests removed. MarkdownReview is now the active code path.
3+
//! Note: Legacy `GeneratedReview` tests removed. `MarkdownReview` is now the active code path.
44
55
// Use our centralized test infrastructure
66
#[path = "test_utils.rs"]

tests/test_utils.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ impl MockDataBuilder {
436436
/// Create a mock `MarkdownPullRequest`
437437
pub fn generated_pull_request() -> MarkdownPullRequest {
438438
MarkdownPullRequest {
439-
content: r#"# Add JWT authentication with user registration
439+
content: r"# Add JWT authentication with user registration
440440
441441
## Summary
442442
@@ -473,7 +473,7 @@ Test user registration flow and verify JWT tokens are properly validated on prot
473473
## Notes
474474
475475
Requires JWT_SECRET environment variable to be set before deployment.
476-
"#.to_string(),
476+
".to_string(),
477477
}
478478
}
479479

0 commit comments

Comments
 (0)