From 8767d9425aef54fa0350f49d8275fae778647b47 Mon Sep 17 00:00:00 2001 From: Mikkel Paulson Date: Wed, 4 Sep 2024 11:02:43 -0400 Subject: [PATCH] apply clippy lints to tests Turns out Clippy has Opinions about clean code in tests too! --- .github/workflows/rust.yml | 2 +- cli/src/rich/mod.rs | 20 ++++++----- core/src/app/command/alias.rs | 2 +- core/src/app/command/app.rs | 2 +- core/src/app/command/mod.rs | 2 +- core/src/app/command/runnable.rs | 2 +- core/src/app/meta.rs | 4 +-- core/src/reference/command.rs | 2 +- core/src/storage/data_store.rs | 3 +- core/src/storage/repository.rs | 8 ++--- core/src/time/command.rs | 20 +++++------ core/src/world/command/autocomplete.rs | 2 +- core/src/world/command/mod.rs | 3 +- core/src/world/npc/ethnicity/mod.rs | 8 ++--- core/src/world/npc/species/mod.rs | 36 +++++++++++-------- core/src/world/place/mod.rs | 2 +- core/tests/common/mod.rs | 2 +- core/tests/integration/app/{app => }/about.rs | 0 core/tests/integration/app/app/mod.rs | 5 --- .../integration/app/{app => }/changelog.rs | 0 core/tests/integration/app/{app => }/debug.rs | 0 core/tests/integration/app/{app => }/help.rs | 2 +- core/tests/integration/app/mod.rs | 6 +++- core/tests/integration/app/{app => }/roll.rs | 0 .../integration/storage/change/delete.rs | 2 +- core/tests/integration/storage/change/save.rs | 4 +-- core/tests/integration/storage/journal.rs | 16 ++++----- core/tests/integration/storage/mod.rs | 2 +- core/tests/integration/world/create.rs | 2 +- .../integration/world/create_multiple.rs | 4 +-- core/tests/integration/world/edit.rs | 2 +- 31 files changed, 86 insertions(+), 79 deletions(-) rename core/tests/integration/app/{app => }/about.rs (100%) delete mode 100644 core/tests/integration/app/app/mod.rs rename core/tests/integration/app/{app => }/changelog.rs (100%) rename core/tests/integration/app/{app => }/debug.rs (100%) rename core/tests/integration/app/{app => }/help.rs (89%) rename core/tests/integration/app/{app => }/roll.rs (100%) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 2ef2a5e6..6d7a01ec 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -24,7 +24,7 @@ jobs: - name: Verify no mangled parens run: if git grep ',)' '*.rs'; then exit 1; fi - name: Run Clippy - run: cargo clippy --workspace -- --deny warnings + run: cargo clippy --workspace --tests -- --deny warnings unit-tests: name: Unit tests diff --git a/cli/src/rich/mod.rs b/cli/src/rich/mod.rs index 2ac28938..b284926a 100644 --- a/cli/src/rich/mod.rs +++ b/cli/src/rich/mod.rs @@ -742,7 +742,7 @@ mod test { selected: None, query: "sh".into(), }; - assert_eq!(single.get_only_suggestion(), single.suggestions.get(0)); + assert_eq!(single.get_only_suggestion(), single.suggestions.first()); let multiple = Autocomplete { show_single_suggestion: true, @@ -777,7 +777,7 @@ mod test { autocomplete = autocomplete.up(); assert_eq!( autocomplete.get_selected_suggestion(), - autocomplete.suggestions.get(0) + autocomplete.suggestions.first(), ); autocomplete = autocomplete.up(); @@ -805,19 +805,19 @@ mod test { autocomplete = autocomplete.down(); assert_eq!( autocomplete.get_selected_suggestion(), - autocomplete.suggestions.get(0) + autocomplete.suggestions.first(), ); autocomplete = autocomplete.down(); assert_eq!( autocomplete.get_selected_suggestion(), - autocomplete.suggestions.get(1) + autocomplete.suggestions.get(1), ); autocomplete = autocomplete.down(); assert_eq!( autocomplete.get_selected_suggestion(), - autocomplete.suggestions.get(0) + autocomplete.suggestions.first(), ); } @@ -971,7 +971,9 @@ mod test { assert_eq!(3, input.index); assert_eq!(0, input.cursor); - input.history.last_mut().map(|s| s.push_str("foo")); + if let Some(s) = input.history.last_mut() { + s.push_str("foo"); + } input.cursor = 3; assert_eq!(vec!["foo", "bar", "foo", "foo"], input.history); @@ -1015,8 +1017,10 @@ mod test { #[test] fn key_esc_test() { - let mut input = Input::default(); - input.search_query = Some(String::new()); + let mut input = Input { + search_query: Some(String::new()), + ..Default::default() + }; input.key(Key::Esc, false); assert_eq!(None, input.search_query); diff --git a/core/src/app/command/alias.rs b/core/src/app/command/alias.rs index 6d38b449..3a37f815 100644 --- a/core/src/app/command/alias.rs +++ b/core/src/app/command/alias.rs @@ -325,7 +325,7 @@ mod tests { fn event_dispatcher(_event: Event) {} fn app_meta() -> AppMeta { - AppMeta::new(NullDataStore::default(), &event_dispatcher) + AppMeta::new(NullDataStore, &event_dispatcher) } fn literal( diff --git a/core/src/app/command/app.rs b/core/src/app/command/app.rs index 4644f97b..fbb9594f 100644 --- a/core/src/app/command/app.rs +++ b/core/src/app/command/app.rs @@ -237,6 +237,6 @@ mod test { fn event_dispatcher(_event: Event) {} fn app_meta() -> AppMeta { - AppMeta::new(NullDataStore::default(), &event_dispatcher) + AppMeta::new(NullDataStore, &event_dispatcher) } } diff --git a/core/src/app/command/mod.rs b/core/src/app/command/mod.rs index 4ca127d3..086b4149 100644 --- a/core/src/app/command/mod.rs +++ b/core/src/app/command/mod.rs @@ -396,6 +396,6 @@ mod test { fn event_dispatcher(_event: Event) {} fn app_meta() -> AppMeta { - AppMeta::new(NullDataStore::default(), &event_dispatcher) + AppMeta::new(NullDataStore, &event_dispatcher) } } diff --git a/core/src/app/command/runnable.rs b/core/src/app/command/runnable.rs index 8945d441..d8af7a75 100644 --- a/core/src/app/command/runnable.rs +++ b/core/src/app/command/runnable.rs @@ -25,7 +25,7 @@ pub fn assert_autocomplete( actual_suggestions: Vec, ) { let mut expected: Vec<_> = expected_suggestions - .into_iter() + .iter() .map(|(a, b)| ((*a).into(), (*b).into())) .collect(); expected.sort(); diff --git a/core/src/app/meta.rs b/core/src/app/meta.rs index b8c72d33..6ad9d774 100644 --- a/core/src/app/meta.rs +++ b/core/src/app/meta.rs @@ -48,7 +48,7 @@ mod test { #[test] fn debug_test() { let mut app_meta = app_meta(); - app_meta.demographics = Demographics::new(HashMap::new().into()); + app_meta.demographics = Demographics::new(HashMap::new()); assert_eq!( "AppMeta { command_aliases: {}, demographics: Demographics { groups: GroupMapWrapper({}) }, repository: Repository { data_store_enabled: false, recent: [] } }", @@ -59,6 +59,6 @@ mod test { fn event_dispatcher(_event: Event) {} fn app_meta() -> AppMeta { - AppMeta::new(NullDataStore::default(), &event_dispatcher) + AppMeta::new(NullDataStore, &event_dispatcher) } } diff --git a/core/src/reference/command.rs b/core/src/reference/command.rs index 76421ef3..79d6cbb7 100644 --- a/core/src/reference/command.rs +++ b/core/src/reference/command.rs @@ -254,6 +254,6 @@ mod test { fn event_dispatcher(_event: Event) {} fn app_meta() -> AppMeta { - AppMeta::new(NullDataStore::default(), &event_dispatcher) + AppMeta::new(NullDataStore, &event_dispatcher) } } diff --git a/core/src/storage/data_store.rs b/core/src/storage/data_store.rs index b4363a9d..1d6cf5aa 100644 --- a/core/src/storage/data_store.rs +++ b/core/src/storage/data_store.rs @@ -311,8 +311,7 @@ mod test { Some("Gandalf the White"), block_on(ds.get_all_the_things()) .unwrap() - .iter() - .next() + .first() .unwrap() .name() .value() diff --git a/core/src/storage/repository.rs b/core/src/storage/repository.rs index 81cef21a..98cccb89 100644 --- a/core/src/storage/repository.rs +++ b/core/src/storage/repository.rs @@ -1339,7 +1339,7 @@ data_store.snapshot() = {:?}", #[test] fn change_test_edit_by_name_from_recent_data_store_failed() { let mut repo = repo(); - repo.data_store = Box::new(NullDataStore::default()); + repo.data_store = Box::new(NullDataStore); let change = Change::Edit { name: "Odysseus".into(), uuid: None, @@ -1956,14 +1956,14 @@ data_store.snapshot() = {:?}", fn data_store_enabled_test_success() { let mut repo = repo(); block_on(repo.init()); - assert_eq!(true, repo.data_store_enabled()); + assert!(repo.data_store_enabled()); } #[test] fn data_store_enabled_test_failure() { let mut repo = null_repo(); block_on(repo.init()); - assert_eq!(false, repo.data_store_enabled()); + assert!(!repo.data_store_enabled()); } fn thing(uuid: Uuid, data: impl Into) -> Thing { @@ -1989,7 +1989,7 @@ data_store.snapshot() = {:?}", } fn null_repo() -> Repository { - Repository::new(NullDataStore::default()) + Repository::new(NullDataStore) } fn populate_repo(repo: &mut Repository) { diff --git a/core/src/time/command.rs b/core/src/time/command.rs index 45421390..9ee51d5f 100644 --- a/core/src/time/command.rs +++ b/core/src/time/command.rs @@ -262,39 +262,39 @@ mod test { ); assert_autocomplete( - &[("+1d".into(), "advance time by 1 day".into())][..], + &[("+1d", "advance time by 1 day")][..], block_on(TimeCommand::autocomplete("+1d", &app_meta)), ); assert_autocomplete( - &[("+1D".into(), "advance time by 1 day".into())][..], + &[("+1D", "advance time by 1 day")][..], block_on(TimeCommand::autocomplete("+1D", &app_meta)), ); assert_autocomplete( - &[("+1h".into(), "advance time by 1 hour".into())][..], + &[("+1h", "advance time by 1 hour")][..], block_on(TimeCommand::autocomplete("+1h", &app_meta)), ); assert_autocomplete( - &[("+1H".into(), "advance time by 1 hour".into())][..], + &[("+1H", "advance time by 1 hour")][..], block_on(TimeCommand::autocomplete("+1H", &app_meta)), ); assert_autocomplete( - &[("+1m".into(), "advance time by 1 minute".into())][..], + &[("+1m", "advance time by 1 minute")][..], block_on(TimeCommand::autocomplete("+1m", &app_meta)), ); assert_autocomplete( - &[("+1M".into(), "advance time by 1 minute".into())][..], + &[("+1M", "advance time by 1 minute")][..], block_on(TimeCommand::autocomplete("+1M", &app_meta)), ); assert_autocomplete( - &[("+1s".into(), "advance time by 1 second".into())][..], + &[("+1s", "advance time by 1 second")][..], block_on(TimeCommand::autocomplete("+1s", &app_meta)), ); assert_autocomplete( - &[("+1S".into(), "advance time by 1 second".into())][..], + &[("+1S", "advance time by 1 second")][..], block_on(TimeCommand::autocomplete("+1S", &app_meta)), ); assert_autocomplete( - &[("+1r".into(), "advance time by 1 round".into())][..], + &[("+1r", "advance time by 1 round")][..], block_on(TimeCommand::autocomplete("+1r", &app_meta)), ); assert_autocomplete( @@ -343,6 +343,6 @@ mod test { fn event_dispatcher(_event: Event) {} fn app_meta() -> AppMeta { - AppMeta::new(NullDataStore::default(), &event_dispatcher) + AppMeta::new(NullDataStore, &event_dispatcher) } } diff --git a/core/src/world/command/autocomplete.rs b/core/src/world/command/autocomplete.rs index 611601ac..a423d657 100644 --- a/core/src/world/command/autocomplete.rs +++ b/core/src/world/command/autocomplete.rs @@ -519,6 +519,6 @@ mod test { fn event_dispatcher(_event: Event) {} fn app_meta() -> AppMeta { - AppMeta::new(NullDataStore::default(), &event_dispatcher) + AppMeta::new(NullDataStore, &event_dispatcher) } } diff --git a/core/src/world/command/mod.rs b/core/src/world/command/mod.rs index 4a50b8ca..1ce861bc 100644 --- a/core/src/world/command/mod.rs +++ b/core/src/world/command/mod.rs @@ -575,6 +575,7 @@ mod test { ..Default::default() } .into(), + #[allow(clippy::single_range_in_vec_init)] unknown_words: vec![10..14], word_count: 2, }, @@ -742,6 +743,6 @@ mod test { fn event_dispatcher(_event: Event) {} fn app_meta() -> AppMeta { - AppMeta::new(NullDataStore::default(), &event_dispatcher) + AppMeta::new(NullDataStore, &event_dispatcher) } } diff --git a/core/src/world/npc/ethnicity/mod.rs b/core/src/world/npc/ethnicity/mod.rs index d74d5a36..7bde56c3 100644 --- a/core/src/world/npc/ethnicity/mod.rs +++ b/core/src/world/npc/ethnicity/mod.rs @@ -213,10 +213,10 @@ mod test { (0..20) .map(|_| gen_name( &mut rng, - &syllable_count_dist, - &start_dist, - &mid_dist, - &end_dist + syllable_count_dist, + start_dist, + mid_dist, + end_dist )) .collect::>(), ); diff --git a/core/src/world/npc/species/mod.rs b/core/src/world/npc/species/mod.rs index c4bc999f..c3489f47 100644 --- a/core/src/world/npc/species/mod.rs +++ b/core/src/world/npc/species/mod.rs @@ -155,8 +155,10 @@ mod test { #[test] fn regenerate_test_default() { - let mut npc = NpcData::default(); - npc.species = Field::new_generated(Species::Human); + let mut npc = NpcData { + species: Field::new_generated(Species::Human), + ..Default::default() + }; let mut rng = SmallRng::seed_from_u64(0); regenerate(&mut rng, &mut npc); @@ -169,16 +171,18 @@ mod test { #[test] fn regenerate_test_locked() { - let mut npc = NpcData::default(); - npc.species = Species::Human.into(); - npc.age = Age::Adult.into(); - npc.age_years = u16::MAX.into(); - npc.gender = Gender::Neuter.into(); - npc.size = Size::Tiny { - height: u16::MAX, - weight: u16::MAX, - } - .into(); + let mut npc = NpcData { + species: Species::Human.into(), + age: Age::Adult.into(), + age_years: u16::MAX.into(), + gender: Gender::Neuter.into(), + size: Size::Tiny { + height: u16::MAX, + weight: u16::MAX, + } + .into(), + ..Default::default() + }; let mut rng = SmallRng::seed_from_u64(0); @@ -198,9 +202,11 @@ mod test { #[test] fn regenerate_test_age_years_provided() { - let mut npc = NpcData::default(); - npc.species = Species::Human.into(); - npc.age_years = u16::MAX.into(); + let mut npc = NpcData { + species: Species::Human.into(), + age_years: u16::MAX.into(), + ..Default::default() + }; let mut rng = SmallRng::seed_from_u64(0); diff --git a/core/src/world/place/mod.rs b/core/src/world/place/mod.rs index 587acfa1..72ae6f59 100644 --- a/core/src/world/place/mod.rs +++ b/core/src/world/place/mod.rs @@ -525,7 +525,7 @@ mod test { Place { uuid: uuid::Uuid::nil(), data: PlaceData { - location_uuid: Uuid::from(uuid::Uuid::nil()).into(), + location_uuid: uuid::Uuid::nil().into(), subtype: "inn".parse::().ok().into(), name: "Oaken Mermaid Inn".into(), diff --git a/core/tests/common/mod.rs b/core/tests/common/mod.rs index 92e3b5f1..9badcc16 100644 --- a/core/tests/common/mod.rs +++ b/core/tests/common/mod.rs @@ -18,7 +18,7 @@ pub fn sync_app() -> SyncApp { #[allow(dead_code)] pub fn sync_app_with_invalid_data_store() -> SyncApp { - sync_app_with_data_store(NullDataStore::default()) + sync_app_with_data_store(NullDataStore) } #[allow(dead_code)] diff --git a/core/tests/integration/app/app/about.rs b/core/tests/integration/app/about.rs similarity index 100% rename from core/tests/integration/app/app/about.rs rename to core/tests/integration/app/about.rs diff --git a/core/tests/integration/app/app/mod.rs b/core/tests/integration/app/app/mod.rs deleted file mode 100644 index bd1b3034..00000000 --- a/core/tests/integration/app/app/mod.rs +++ /dev/null @@ -1,5 +0,0 @@ -mod about; -mod changelog; -mod debug; -mod help; -mod roll; diff --git a/core/tests/integration/app/app/changelog.rs b/core/tests/integration/app/changelog.rs similarity index 100% rename from core/tests/integration/app/app/changelog.rs rename to core/tests/integration/app/changelog.rs diff --git a/core/tests/integration/app/app/debug.rs b/core/tests/integration/app/debug.rs similarity index 100% rename from core/tests/integration/app/app/debug.rs rename to core/tests/integration/app/debug.rs diff --git a/core/tests/integration/app/app/help.rs b/core/tests/integration/app/help.rs similarity index 89% rename from core/tests/integration/app/app/help.rs rename to core/tests/integration/app/help.rs index 1a49616e..765db8ba 100644 --- a/core/tests/integration/app/app/help.rs +++ b/core/tests/integration/app/help.rs @@ -19,7 +19,7 @@ fn all_commands_are_valid() { .filter(|s| !s.contains('[')) { // Basically, we just want to make sure all commands run successfully. - app.command(command).expect(&format!("{}", command)); + app.command(command).expect(command); } } } diff --git a/core/tests/integration/app/mod.rs b/core/tests/integration/app/mod.rs index 5c8c8a84..9f46c653 100644 --- a/core/tests/integration/app/mod.rs +++ b/core/tests/integration/app/mod.rs @@ -1,4 +1,8 @@ -mod app; +mod about; +mod changelog; +mod debug; +mod help; +mod roll; mod tutorial; use crate::common::{get_name, sync_app}; diff --git a/core/tests/integration/app/app/roll.rs b/core/tests/integration/app/roll.rs similarity index 100% rename from core/tests/integration/app/app/roll.rs rename to core/tests/integration/app/roll.rs diff --git a/core/tests/integration/storage/change/delete.rs b/core/tests/integration/storage/change/delete.rs index 96d7dfd8..0b856002 100644 --- a/core/tests/integration/storage/change/delete.rs +++ b/core/tests/integration/storage/change/delete.rs @@ -90,7 +90,7 @@ fn npc_can_be_deleted_from_data_store() { #[test] fn delete_works_with_unusable_data_store() { - let mut app = sync_app_with_data_store(NullDataStore::default()); + let mut app = sync_app_with_data_store(NullDataStore); app.command("npc named Potato Johnson").unwrap(); diff --git a/core/tests/integration/storage/change/save.rs b/core/tests/integration/storage/change/save.rs index 5e2b4c2b..ec6b4c64 100644 --- a/core/tests/integration/storage/change/save.rs +++ b/core/tests/integration/storage/change/save.rs @@ -59,7 +59,7 @@ fn npc_is_saved_to_storage_by_alias() { #[test] fn npc_can_be_saved_with_invalid_data_store() { - let mut app = sync_app_with_data_store(NullDataStore::default()); + let mut app = sync_app_with_data_store(NullDataStore); let generated_output = app.command("npc").unwrap(); let npc_name = generated_output @@ -88,7 +88,7 @@ fn npc_can_be_saved_with_invalid_data_store() { #[test] fn npc_can_be_saved_by_alias_with_invalid_data_store() { - let mut app = sync_app_with_data_store(NullDataStore::default()); + let mut app = sync_app_with_data_store(NullDataStore); let generated_output = app.command("npc").unwrap(); let npc_name = generated_output diff --git a/core/tests/integration/storage/journal.rs b/core/tests/integration/storage/journal.rs index 0b57ee3a..f0548103 100644 --- a/core/tests/integration/storage/journal.rs +++ b/core/tests/integration/storage/journal.rs @@ -32,16 +32,15 @@ fn it_shows_alphabetized_results() { .lines() .filter(|s| s.starts_with('~')) .map(|s| s[4..].trim_end_matches('\\')) - .map(|line| { + .inspect(|line| { println!( "{}", app.command(&format!( "save {}", - line.find('(').map(|pos| &line[6..(pos - 2)]).unwrap() + line.find('(').map(|pos| &line[6..(pos - 2)]).unwrap(), )) .unwrap(), - ); - line + ) }) .collect(); @@ -56,16 +55,15 @@ fn it_shows_alphabetized_results() { .lines() .filter(|s| s.starts_with('~')) .map(|s| s[4..].trim_end_matches('\\')) - .map(|line| { + .inspect(|line| { println!( "{}", app.command(&format!( "save {}", - line.find('(').map(|pos| &line[6..(pos - 2)]).unwrap() + line.find('(').map(|pos| &line[6..(pos - 2)]).unwrap(), )) .unwrap(), - ); - line + ) }) .collect(); @@ -107,7 +105,7 @@ fn it_shows_alphabetized_results() { assert_eq!( Some("*To export the contents of your journal, use `export`.*"), - output_iter.by_ref().skip(1).next(), + output_iter.by_ref().nth(1), ); assert!(output_iter.next().is_none()); diff --git a/core/tests/integration/storage/mod.rs b/core/tests/integration/storage/mod.rs index fe222573..b186bbf2 100644 --- a/core/tests/integration/storage/mod.rs +++ b/core/tests/integration/storage/mod.rs @@ -12,7 +12,7 @@ fn event_dispatcher(_event: Event) {} #[test] fn startup_error_with_unusable_data_store() { { - let mut app = SyncApp::new(NullDataStore::default(), &event_dispatcher); + let mut app = SyncApp::new(NullDataStore, &event_dispatcher); let output = app.init(); assert!( output.contains("Local storage is not available in your browser."), diff --git a/core/tests/integration/world/create.rs b/core/tests/integration/world/create.rs index 34178b9d..52c50421 100644 --- a/core/tests/integration/world/create.rs +++ b/core/tests/integration/world/create.rs @@ -31,7 +31,7 @@ fn save_alias() { #[test] fn save_alias_exists_with_invalid_data_store() { - let mut app = sync_app_with_data_store(NullDataStore::default()); + let mut app = sync_app_with_data_store(NullDataStore); let name = get_name(&app.command("npc").unwrap()); diff --git a/core/tests/integration/world/create_multiple.rs b/core/tests/integration/world/create_multiple.rs index 4969a443..7b31210a 100644 --- a/core/tests/integration/world/create_multiple.rs +++ b/core/tests/integration/world/create_multiple.rs @@ -57,7 +57,7 @@ fn more_alias() { #[test] fn more_alias_exists_with_invalid_data_store() { - let mut app = sync_app_with_data_store(NullDataStore::default()); + let mut app = sync_app_with_data_store(NullDataStore); let output = app.command("npc").unwrap(); assert!(output.contains("~more~"), "{}", output); @@ -74,7 +74,7 @@ fn more_alias_does_not_exist_with_name() { } { - let mut app = sync_app_with_data_store(NullDataStore::default()); + let mut app = sync_app_with_data_store(NullDataStore); let output = app.command("place called Home").unwrap(); assert!(!output.contains("~more~"), "{}", output); app.command("more").unwrap_err(); diff --git a/core/tests/integration/world/edit.rs b/core/tests/integration/world/edit.rs index dccaa8d8..67518abc 100644 --- a/core/tests/integration/world/edit.rs +++ b/core/tests/integration/world/edit.rs @@ -120,7 +120,7 @@ fn edit_implicitly_saves() { { let output = app.command("journal").unwrap(); - assert!(output.contains(&name), "{}", output); + assert!(output.contains(name), "{}", output); } }