Skip to content

Commit 395ea53

Browse files
committed
tests: Replace Result.unwrap with ? operator in test functions
Mostly done with OpenCode and Claude Sonnet 4.6 with the following prompt, and reviewed manually: * Replace the Result.unwrap with the ? operator in test functions * Add TestResult as return type in the test functions where you replace unwrap by the ? operator, and Ok(()) at the end. * Do the replacement only in method with the `#[test]` decorator. You can't exclude files that are not in tests dirs, because some files have inline tests. * Don't replace unwraps in closures. * Don't replace Option.unwrap. * Be careful to not change the insta snapshot references. * You can identify all the Result.unwrap locations with this command: cargo clippy --tests --message-format=short -- -W clippy::unwrap_used 2>&1 | grep / | grep Result | grep <filename> * Run the tests each time you're done with a file. * Begin with the file with the most Result.unwrap in them as shown by this command: cargo clippy --tests --message-format=short -- -W clippy::unwrap_used 2>&1 | grep Result | cut -d: -f1 | sort | uniq -c | sort -nr Before this PR: ❯ cargo clippy --tests --message-format=short -- -W clippy::unwrap_used 2>&1 | grep Result | wc -l 3973 After this PR: ❯ cargo clippy --tests --message-format=short -- -W clippy::unwrap_used 2>&1 | grep Result | wc -l 1174
1 parent 4ea608c commit 395ea53

44 files changed

Lines changed: 2270 additions & 2135 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

cli/src/config.rs

Lines changed: 48 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,6 +1049,7 @@ mod tests {
10491049
use indoc::indoc;
10501050
use maplit::hashmap;
10511051
use test_case::test_case;
1052+
use testutils::TestResult;
10521053

10531054
use super::*;
10541055

@@ -1060,30 +1061,30 @@ mod tests {
10601061
}
10611062

10621063
#[test]
1063-
fn test_parse_value_or_bare_string() {
1064+
fn test_parse_value_or_bare_string() -> TestResult {
10641065
let parse = |s: &str| parse_value_or_bare_string(s);
10651066

10661067
// Value in TOML syntax
1067-
assert_eq!(parse("true").unwrap().as_bool(), Some(true));
1068-
assert_eq!(parse("42").unwrap().as_integer(), Some(42));
1069-
assert_eq!(parse("-1").unwrap().as_integer(), Some(-1));
1070-
assert_eq!(parse("'a'").unwrap().as_str(), Some("a"));
1071-
assert!(parse("[]").unwrap().is_array());
1072-
assert!(parse("{ a = 'b' }").unwrap().is_inline_table());
1068+
assert_eq!(parse("true")?.as_bool(), Some(true));
1069+
assert_eq!(parse("42")?.as_integer(), Some(42));
1070+
assert_eq!(parse("-1")?.as_integer(), Some(-1));
1071+
assert_eq!(parse("'a'")?.as_str(), Some("a"));
1072+
assert!(parse("[]")?.is_array());
1073+
assert!(parse("{ a = 'b' }")?.is_inline_table());
10731074

10741075
// Bare string
1075-
assert_eq!(parse("").unwrap().as_str(), Some(""));
1076-
assert_eq!(parse("John Doe").unwrap().as_str(), Some("John Doe"));
1077-
assert_eq!(parse("Doe, John").unwrap().as_str(), Some("Doe, John"));
1078-
assert_eq!(parse("It's okay").unwrap().as_str(), Some("It's okay"));
1076+
assert_eq!(parse("")?.as_str(), Some(""));
1077+
assert_eq!(parse("John Doe")?.as_str(), Some("John Doe"));
1078+
assert_eq!(parse("Doe, John")?.as_str(), Some("Doe, John"));
1079+
assert_eq!(parse("It's okay")?.as_str(), Some("It's okay"));
10791080
assert_eq!(
1080-
parse("<foo+bar@example.org>").unwrap().as_str(),
1081+
parse("<foo+bar@example.org>")?.as_str(),
10811082
Some("<foo+bar@example.org>")
10821083
);
1083-
assert_eq!(parse("#ff00aa").unwrap().as_str(), Some("#ff00aa"));
1084-
assert_eq!(parse("all()").unwrap().as_str(), Some("all()"));
1085-
assert_eq!(parse("glob:*.*").unwrap().as_str(), Some("glob:*.*"));
1086-
assert_eq!(parse("柔術").unwrap().as_str(), Some("柔術"));
1084+
assert_eq!(parse("#ff00aa")?.as_str(), Some("#ff00aa"));
1085+
assert_eq!(parse("all()")?.as_str(), Some("all()"));
1086+
assert_eq!(parse("glob:*.*")?.as_str(), Some("glob:*.*"));
1087+
assert_eq!(parse("柔術")?.as_str(), Some("柔術"));
10871088

10881089
// Error in TOML value
10891090
assert!(parse("'foo").is_err());
@@ -1093,6 +1094,7 @@ mod tests {
10931094
assert!(parse("\n { x").is_err());
10941095
assert!(parse(" x ] ").is_err());
10951096
assert!(parse("[table]\nkey = 'value'").is_err());
1097+
Ok(())
10961098
}
10971099

10981100
#[test]
@@ -1140,12 +1142,11 @@ mod tests {
11401142
}
11411143

11421144
#[test]
1143-
fn test_command_args() {
1145+
fn test_command_args() -> TestResult {
11441146
let mut config = StackedConfig::empty();
1145-
config.add_layer(
1146-
ConfigLayer::parse(
1147-
ConfigSource::User,
1148-
indoc! {"
1147+
config.add_layer(ConfigLayer::parse(
1148+
ConfigSource::User,
1149+
indoc! {"
11491150
empty_array = []
11501151
empty_string = ''
11511152
array = ['emacs', '-nw']
@@ -1154,19 +1155,17 @@ mod tests {
11541155
structured.env = { KEY1 = 'value1', KEY2 = 'value2' }
11551156
structured.command = ['emacs', '-nw']
11561157
"},
1157-
)
1158-
.unwrap(),
1159-
);
1158+
)?);
11601159

11611160
assert!(config.get::<CommandNameAndArgs>("empty_array").is_err());
11621161

1163-
let command_args: CommandNameAndArgs = config.get("empty_string").unwrap();
1162+
let command_args: CommandNameAndArgs = config.get("empty_string")?;
11641163
assert_eq!(command_args, CommandNameAndArgs::String("".to_owned()));
11651164
let (name, args) = command_args.split_name_and_args();
11661165
assert_eq!(name, "");
11671166
assert!(args.is_empty());
11681167

1169-
let command_args: CommandNameAndArgs = config.get("array").unwrap();
1168+
let command_args: CommandNameAndArgs = config.get("array")?;
11701169
assert_eq!(
11711170
command_args,
11721171
CommandNameAndArgs::Vec(NonEmptyCommandArgsVec(
@@ -1177,7 +1176,7 @@ mod tests {
11771176
assert_eq!(name, "emacs");
11781177
assert_eq!(args, ["-nw"].as_ref());
11791178

1180-
let command_args: CommandNameAndArgs = config.get("string").unwrap();
1179+
let command_args: CommandNameAndArgs = config.get("string")?;
11811180
assert_eq!(
11821181
command_args,
11831182
CommandNameAndArgs::String("emacs -nw".to_owned())
@@ -1186,7 +1185,7 @@ mod tests {
11861185
assert_eq!(name, "emacs");
11871186
assert_eq!(args, ["-nw"].as_ref());
11881187

1189-
let command_args: CommandNameAndArgs = config.get("string_quoted").unwrap();
1188+
let command_args: CommandNameAndArgs = config.get("string_quoted")?;
11901189
assert_eq!(
11911190
command_args,
11921191
CommandNameAndArgs::String("\"spaced path/to/emacs\" -nw".to_owned())
@@ -1195,7 +1194,7 @@ mod tests {
11951194
assert_eq!(name, "spaced path/to/emacs");
11961195
assert_eq!(args, ["-nw"].as_ref());
11971196

1198-
let command_args: CommandNameAndArgs = config.get("structured").unwrap();
1197+
let command_args: CommandNameAndArgs = config.get("structured")?;
11991198
assert_eq!(
12001199
command_args,
12011200
CommandNameAndArgs::Structured {
@@ -1209,6 +1208,7 @@ mod tests {
12091208
let (name, args) = command_args.split_name_and_args();
12101209
assert_eq!(name, "emacs");
12111210
assert_eq!(args, ["-nw"].as_ref());
1211+
Ok(())
12121212
}
12131213

12141214
#[test]
@@ -1218,20 +1218,14 @@ mod tests {
12181218
}
12191219

12201220
#[test]
1221-
fn test_resolved_config_values_single_key() {
1221+
fn test_resolved_config_values_single_key() -> TestResult {
12221222
let settings = insta_settings();
12231223
let _guard = settings.bind_to_scope();
12241224
let mut env_base_layer = ConfigLayer::empty(ConfigSource::EnvBase);
1225-
env_base_layer
1226-
.set_value("user.name", "base-user-name")
1227-
.unwrap();
1228-
env_base_layer
1229-
.set_value("user.email", "base@user.email")
1230-
.unwrap();
1225+
env_base_layer.set_value("user.name", "base-user-name")?;
1226+
env_base_layer.set_value("user.email", "base@user.email")?;
12311227
let mut repo_layer = ConfigLayer::empty(ConfigSource::Repo);
1232-
repo_layer
1233-
.set_value("user.email", "repo@user.email")
1234-
.unwrap();
1228+
repo_layer.set_value("user.email", "repo@user.email")?;
12351229
let mut config = StackedConfig::empty();
12361230
config.add_layer(env_base_layer);
12371231
config.add_layer(repo_layer);
@@ -1327,17 +1321,18 @@ mod tests {
13271321
]
13281322
"#
13291323
);
1324+
Ok(())
13301325
}
13311326

13321327
#[test]
1333-
fn test_resolved_config_values_filter_path() {
1328+
fn test_resolved_config_values_filter_path() -> TestResult {
13341329
let settings = insta_settings();
13351330
let _guard = settings.bind_to_scope();
13361331
let mut user_layer = ConfigLayer::empty(ConfigSource::User);
1337-
user_layer.set_value("test-table1.foo", "user-FOO").unwrap();
1338-
user_layer.set_value("test-table2.bar", "user-BAR").unwrap();
1332+
user_layer.set_value("test-table1.foo", "user-FOO")?;
1333+
user_layer.set_value("test-table2.bar", "user-BAR")?;
13391334
let mut repo_layer = ConfigLayer::empty(ConfigSource::Repo);
1340-
repo_layer.set_value("test-table1.bar", "repo-BAR").unwrap();
1335+
repo_layer.set_value("test-table1.bar", "repo-BAR")?;
13411336
let mut config = StackedConfig::empty();
13421337
config.add_layer(user_layer);
13431338
config.add_layer(repo_layer);
@@ -1404,10 +1399,11 @@ mod tests {
14041399
]
14051400
"#
14061401
);
1402+
Ok(())
14071403
}
14081404

14091405
#[test]
1410-
fn test_resolved_config_values_overridden() {
1406+
fn test_resolved_config_values_overridden() -> TestResult {
14111407
let list = |layers: &[&ConfigLayer], prefix: &str| -> String {
14121408
let mut config = StackedConfig::empty();
14131409
config.extend_layers(layers.iter().copied().cloned());
@@ -1426,15 +1422,15 @@ mod tests {
14261422
};
14271423

14281424
let mut layer0 = ConfigLayer::empty(ConfigSource::User);
1429-
layer0.set_value("a.b.e", "0.0").unwrap();
1430-
layer0.set_value("a.b.c.f", "0.1").unwrap();
1431-
layer0.set_value("a.b.d", "0.2").unwrap();
1425+
layer0.set_value("a.b.e", "0.0")?;
1426+
layer0.set_value("a.b.c.f", "0.1")?;
1427+
layer0.set_value("a.b.d", "0.2")?;
14321428
let mut layer1 = ConfigLayer::empty(ConfigSource::User);
1433-
layer1.set_value("a.b", "1.0").unwrap();
1434-
layer1.set_value("a.c", "1.1").unwrap();
1429+
layer1.set_value("a.b", "1.0")?;
1430+
layer1.set_value("a.c", "1.1")?;
14351431
let mut layer2 = ConfigLayer::empty(ConfigSource::User);
1436-
layer2.set_value("a.b.g", "2.0").unwrap();
1437-
layer2.set_value("a.b.d", "2.1").unwrap();
1432+
layer2.set_value("a.b.g", "2.0")?;
1433+
layer2.set_value("a.b.d", "2.1")?;
14381434

14391435
// a.b.* is shadowed by a.b
14401436
let layers = [&layer0, &layer1];
@@ -1510,6 +1506,7 @@ mod tests {
15101506
a.b.d = "2.1"
15111507
"#);
15121508
insta::assert_snapshot!(list(&layers, "a.b.c"), @r#"!a.b.c.f = "0.1""#);
1509+
Ok(())
15131510
}
15141511

15151512
struct TestCase {

0 commit comments

Comments
 (0)