Add unit tests for every Discord command

crates/sb-discord-bot/src/handler/message.rs

@@ -4,14 +4,16 @@ use serenity::all::{
 
 use crate::{
     store::{
-        bibleapi::{get_chapter, ChapterItem},
+        bibleapi::{ChapterItem, get_chapter},
         store::{Store, StoreKey},
         valid_cache::get_valid_translations,
     },
     util::{
-        format::{format_chapter_content, format_verse_content, get_passage_url, split_into_embed_chunks},
+        format::{
+            format_chapter_content, format_verse_content, get_passage_url, split_into_embed_chunks,
+        },
         prefs::calc_translation,
-        reference_parser::{find_references, format_verse_numbers, ParsedReference},
+        reference_parser::{ParsedReference, find_references, format_verse_numbers},
     },
 };
 
@@ -112,10 +114,11 @@ async fn respond_to_reference(
                         .color(Colour::from_rgb(178, 255, 237)),
                 );
                 if show_button && i + 1 == total {
+                    let chapter_string = reference.chapter.to_string();
                     builder = builder.components(vec![CreateActionRow::Buttons(vec![
                         CreateButton::new_link(get_passage_url(
-                            &reference.book.to_string(),
-                            &reference.chapter.to_string(),
+                            Some(reference.book.get_3c_id()),
+                            Some(&chapter_string),
                             Some(translation),
                             None,
                         ))

crates/sb-discord-bot/src/poise_impl/helpers.rs

@@ -7,27 +7,62 @@ use crate::{
     util::prefs::{calc_lang, calc_translation},
 };
 
+/// Reply shown when a user-supplied book name doesn't parse into a `BibleBooks` variant.
+pub fn unknown_book_reply(book: &str) -> CreateReply {
+    CreateReply {
+        embeds: vec![
+            CreateEmbed::default()
+                .title("Unknown book")
+                .description(format!("\"{}\" is not a recognised Bible book.", book))
+                .color(Colour::new(16730184)),
+        ],
+        ..Default::default()
+    }
+}
+
+/// Parses a book name string into a `BibleBooks` variant. Pure, no I/O.
+pub fn parse_book_str(book: &str) -> Option<BibleBooks> {
+    book.parse::<BibleBooks>().ok()
+}
+
 /// Parses a book name string into a `BibleBooks` variant.
 /// Sends an error embed and returns `None` if unrecognised.
 pub async fn parse_book(ctx: &Context<'_>, book: &str) -> Result<Option<BibleBooks>, Error> {
-    match book.parse::<BibleBooks>() {
-        Ok(b) => Ok(Some(b)),
-        Err(_) => {
-            ctx.send(CreateReply {
-                embeds: vec![
-                    CreateEmbed::default()
-                        .title("Unknown book")
-                        .description(format!("\"{}\" is not a recognised Bible book.", book))
-                        .color(Colour::new(16730184)),
-                ],
-                ..Default::default()
-            })
-            .await?;
+    match parse_book_str(book) {
+        Some(b) => Ok(Some(b)),
+        None => {
+            ctx.send(unknown_book_reply(book)).await?;
             Ok(None)
         }
     }
 }
 
+/// Reply shown when `chapter` exceeds `max` chapters available for `book` in `translation`.
+pub fn invalid_chapter_reply(book: &BibleBooks, max: i64, translation: &str) -> CreateReply {
+    CreateReply {
+        embeds: vec![
+            CreateEmbed::default()
+                .title("Invalid chapter")
+                .description(format!(
+                    "{} only has {} chapter(s) in {}.",
+                    book, max, translation
+                ))
+                .color(Colour::new(16730184)),
+        ],
+        ..Default::default()
+    }
+}
+
+/// Whether `chapter` is within range given an optional known chapter count.
+/// `None` means the count couldn't be determined, in which case validation
+/// is skipped and the API call itself is left to respond. Pure, no I/O.
+pub fn chapter_in_range(chapter: i64, max_chapters: Option<i64>) -> bool {
+    match max_chapters {
+        Some(max) => chapter <= max,
+        None => true,
+    }
+}
+
 /// Validates that `chapter` is within range for `book` in `translation`.
 /// Sends an error embed and returns `false` if out of range.
 pub async fn validate_chapter(
@@ -36,23 +71,11 @@ pub async fn validate_chapter(
     translation: &str,
     chapter: i64,
 ) -> Result<bool, Error> {
-    let Some(max) = get_chapter_count(translation, book.get_3c_id()).await else {
-        return Ok(true); // can't validate, let the API respond
-    };
-    if chapter > max {
-        ctx.send(CreateReply {
-            embeds: vec![
-                CreateEmbed::default()
-                    .title("Invalid chapter")
-                    .description(format!(
-                        "{} only has {} chapter(s) in {}.",
-                        book, max, translation
-                    ))
-                    .color(Colour::new(16730184)),
-            ],
-            ..Default::default()
-        })
-        .await?;
+    let max = get_chapter_count(translation, book.get_3c_id()).await;
+    if !chapter_in_range(chapter, max) {
+        let max = max.expect("chapter_in_range only rejects a known max");
+        ctx.send(invalid_chapter_reply(book, max, translation))
+            .await?;
         return Ok(false);
     }
     Ok(true)
@@ -92,3 +115,89 @@ pub fn split_reference(reference: &str) -> (String, Option<String>) {
     }
     (tokens.join(" "), None)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::poise_impl::test_support::{embed_description, embed_title};
+
+    #[test]
+    fn split_reference_separates_trailing_numeric_chapter() {
+        assert_eq!(
+            split_reference("John 3"),
+            ("John".to_string(), Some("3".to_string()))
+        );
+    }
+
+    #[test]
+    fn split_reference_handles_multi_word_book_with_chapter() {
+        assert_eq!(
+            split_reference("1 John 3"),
+            ("1 John".to_string(), Some("3".to_string()))
+        );
+    }
+
+    #[test]
+    fn split_reference_treats_whole_reference_as_book_without_chapter() {
+        assert_eq!(split_reference("Genesis"), ("Genesis".to_string(), None));
+    }
+
+    #[test]
+    fn split_reference_treats_multi_word_book_without_trailing_number_as_book_only() {
+        assert_eq!(
+            split_reference("Song of Solomon"),
+            ("Song of Solomon".to_string(), None)
+        );
+    }
+
+    #[test]
+    fn split_reference_handles_empty_input() {
+        assert_eq!(split_reference(""), ("".to_string(), None));
+    }
+
+    #[test]
+    fn parse_book_str_accepts_known_book_case_insensitively() {
+        assert!(matches!(parse_book_str("john"), Some(BibleBooks::John)));
+        assert!(matches!(
+            parse_book_str("1 Samuel"),
+            Some(BibleBooks::Samuel1)
+        ));
+    }
+
+    #[test]
+    fn parse_book_str_rejects_unknown_book() {
+        assert!(parse_book_str("Not A Book").is_none());
+    }
+
+    #[test]
+    fn unknown_book_reply_mentions_the_input() {
+        let reply = unknown_book_reply("Frodo");
+        assert_eq!(embed_title(&reply, 0), "Unknown book");
+        assert!(embed_description(&reply, 0).contains("Frodo"));
+    }
+
+    #[test]
+    fn chapter_in_range_allows_unknown_max() {
+        assert!(chapter_in_range(999, None));
+    }
+
+    #[test]
+    fn chapter_in_range_allows_chapter_at_max() {
+        assert!(chapter_in_range(5, Some(5)));
+    }
+
+    #[test]
+    fn chapter_in_range_rejects_chapter_over_max() {
+        assert!(!chapter_in_range(6, Some(5)));
+    }
+
+    #[test]
+    fn invalid_chapter_reply_mentions_book_and_max() {
+        let reply = invalid_chapter_reply(&BibleBooks::John, 21, "BSB");
+        assert_eq!(embed_title(&reply, 0), "Invalid chapter");
+        let description = embed_description(&reply, 0);
+        assert!(description.contains("John"));
+        assert!(description.contains("21"));
+        assert!(description.contains("BSB"));
+    }
+}

crates/sb-discord-bot/src/poise_impl/mod.rs

@@ -1,5 +1,8 @@
 pub mod data;
 pub mod framework;
 pub mod helpers;
+pub mod replies;
 pub mod slash_commands;
+#[cfg(test)]
+pub mod test_support;
 pub mod types;