test(rules): add unit tests covering all public methods and edge cases

- Add comprehensive test coverage for all Rules public methods
- Test both success and error paths with proper assertions
- Add PartialEq derive to EolAction to enable comparisons in tests
- Include edge cases like duplicate labels and nonexistent IDs
- Mark file system integration tests as ignore to avoid interference

src/eol_action.rs

@@ -3,7 +3,7 @@ use std::fmt;
 /// End of life action
 /// - Trash - move the message to the trash to be automatically deleted by Google
 /// - Delete - delete the message immediately without allowing rescue from trash
-#[derive(Debug, Default, Clone)]
+#[derive(Debug, Default, Clone, PartialEq)]
 pub enum EolAction {
     #[default]
     /// Move the message to the trash

src/rules.rs

@@ -574,3 +574,349 @@ impl Rules {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::test_utils::get_test_logger;
+    use std::fs;
+
+    fn setup_test_environment() {
+        get_test_logger();
+        // Clean up any existing test files
+        let home_dir = env::home_dir().unwrap();
+        let test_config_dir = home_dir.join(".cull-gmail");
+        let test_rules_file = test_config_dir.join("rules.toml");
+        if test_rules_file.exists() {
+            let _ = fs::remove_file(&test_rules_file);
+        }
+    }
+
+    #[test]
+    fn test_rules_new_creates_default_rules() {
+        setup_test_environment();
+        
+        let rules = Rules::new();
+        
+        // Should have some default rules
+        let labels = rules.labels();
+        assert!(!labels.is_empty(), "Default rules should create some labels");
+        
+        // Should contain the expected retention labels
+        assert!(labels.iter().any(|l| l.contains("retention/1-years")));
+        assert!(labels.iter().any(|l| l.contains("retention/1-weeks")));
+        assert!(labels.iter().any(|l| l.contains("retention/1-months")));
+        assert!(labels.iter().any(|l| l.contains("retention/5-years")));
+    }
+
+    #[test]
+    fn test_rules_default_same_as_new() {
+        setup_test_environment();
+        
+        let rules_new = Rules::new();
+        let rules_default = Rules::default();
+        
+        // Both should have the same number of rules
+        assert_eq!(rules_new.labels().len(), rules_default.labels().len());
+    }
+
+    #[test]
+    fn test_add_rule_with_label() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        let initial_label_count = rules.labels().len();
+        
+        let retention = Retention::new(MessageAge::Days(30), false);
+        rules.add_rule(retention, Some(&"test-label".to_string()), false);
+        
+        let labels = rules.labels();
+        assert!(labels.contains(&"test-label".to_string()));
+        assert_eq!(labels.len(), initial_label_count + 1);
+    }
+
+    #[test]
+    fn test_add_rule_without_label() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        let initial_label_count = rules.labels().len();
+        
+        let retention = Retention::new(MessageAge::Days(30), false);
+        rules.add_rule(retention, None, false);
+        
+        // Should not add any new labels since no label specified and generate_label is false
+        let labels = rules.labels();
+        assert_eq!(labels.len(), initial_label_count);
+    }
+
+    #[test]
+    fn test_add_rule_with_delete_action() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        let retention = Retention::new(MessageAge::Days(7), false);
+        rules.add_rule(retention, Some(&"delete-test".to_string()), true);
+        
+        let rules_by_label = rules.get_rules_by_label();
+        let rule = rules_by_label.get("delete-test").unwrap();
+        assert_eq!(rule.action(), Some(EolAction::Delete));
+    }
+
+    #[test]
+    fn test_add_duplicate_label_warns_and_skips() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        let retention1 = Retention::new(MessageAge::Days(30), false);
+        let retention2 = Retention::new(MessageAge::Days(60), false);
+        
+        rules.add_rule(retention1, Some(&"duplicate".to_string()), false);
+        let initial_count = rules.labels().len();
+        
+        // Try to add another rule with the same label
+        rules.add_rule(retention2, Some(&"duplicate".to_string()), false);
+        
+        // Should not increase the count of labels
+        assert_eq!(rules.labels().len(), initial_count);
+    }
+
+    #[test]
+    fn test_get_rule_existing() {
+        setup_test_environment();
+        
+        let rules = Rules::new();
+        
+        // Default rules should have ID 1
+        let rule = rules.get_rule(1);
+        assert!(rule.is_some());
+        assert_eq!(rule.unwrap().id(), 1);
+    }
+
+    #[test]
+    fn test_get_rule_nonexistent() {
+        setup_test_environment();
+        
+        let rules = Rules::new();
+        
+        // ID 999 should not exist
+        let rule = rules.get_rule(999);
+        assert!(rule.is_none());
+    }
+
+    #[test]
+    fn test_labels_returns_all_labels() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        let retention = Retention::new(MessageAge::Days(30), false);
+        rules.add_rule(retention, Some(&"custom-label".to_string()), false);
+        
+        let labels = rules.labels();
+        assert!(labels.contains(&"custom-label".to_string()));
+    }
+
+    #[test]
+    fn test_get_rules_by_label() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        let retention = Retention::new(MessageAge::Days(30), false);
+        rules.add_rule(retention, Some(&"mapped-label".to_string()), false);
+        
+        let label_map = rules.get_rules_by_label();
+        let rule = label_map.get("mapped-label");
+        assert!(rule.is_some());
+        assert!(rule.unwrap().labels().contains(&"mapped-label".to_string()));
+    }
+
+    #[test]
+    fn test_remove_rule_by_id_existing() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        
+        // Remove a default rule (assuming ID 1 exists)
+        let result = rules.remove_rule_by_id(1);
+        assert!(result.is_ok());
+        
+        // Rule should no longer exist
+        assert!(rules.get_rule(1).is_none());
+    }
+
+    #[test]
+    fn test_remove_rule_by_id_nonexistent() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        
+        // Removing non-existent rule should still succeed
+        let result = rules.remove_rule_by_id(999);
+        assert!(result.is_ok());
+    }
+
+    #[test]
+    fn test_remove_rule_by_label_existing() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        let retention = Retention::new(MessageAge::Days(30), false);
+        rules.add_rule(retention, Some(&"remove-me".to_string()), false);
+        
+        let result = rules.remove_rule_by_label("remove-me");
+        assert!(result.is_ok());
+        
+        // Label should no longer exist
+        let labels = rules.labels();
+        assert!(!labels.contains(&"remove-me".to_string()));
+    }
+
+    #[test]
+    fn test_remove_rule_by_label_nonexistent() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        
+        let result = rules.remove_rule_by_label("nonexistent-label");
+        assert!(result.is_err());
+        
+        match result.unwrap_err() {
+            Error::LabelNotFoundInRules(label) => {
+                assert_eq!(label, "nonexistent-label");
+            }
+            _ => panic!("Expected LabelNotFoundInRules error"),
+        }
+    }
+
+    #[test]
+    fn test_add_label_to_rule_existing_rule() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        
+        // Add label to existing rule (ID 1)
+        let result = rules.add_label_to_rule(1, "new-label");
+        assert!(result.is_ok());
+        
+        let rule = rules.get_rule(1).unwrap();
+        assert!(rule.labels().contains(&"new-label".to_string()));
+    }
+
+    #[test]
+    fn test_add_label_to_rule_nonexistent_rule() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        
+        let result = rules.add_label_to_rule(999, "new-label");
+        assert!(result.is_err());
+        
+        match result.unwrap_err() {
+            Error::RuleNotFound(id) => {
+                assert_eq!(id, 999);
+            }
+            _ => panic!("Expected RuleNotFound error"),
+        }
+    }
+
+    #[test]
+    fn test_remove_label_from_rule_existing() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        
+        // First add a label
+        let result = rules.add_label_to_rule(1, "temp-label");
+        assert!(result.is_ok());
+        
+        // Then remove it
+        let result = rules.remove_label_from_rule(1, "temp-label");
+        assert!(result.is_ok());
+        
+        let rule = rules.get_rule(1).unwrap();
+        assert!(!rule.labels().contains(&"temp-label".to_string()));
+    }
+
+    #[test]
+    fn test_remove_label_from_rule_nonexistent_rule() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        
+        let result = rules.remove_label_from_rule(999, "any-label");
+        assert!(result.is_err());
+        
+        match result.unwrap_err() {
+            Error::RuleNotFound(id) => {
+                assert_eq!(id, 999);
+            }
+            _ => panic!("Expected RuleNotFound error"),
+        }
+    }
+
+    #[test]
+    fn test_set_action_on_rule_existing() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        
+        // Set action to Delete
+        let result = rules.set_action_on_rule(1, &EolAction::Delete);
+        assert!(result.is_ok());
+        
+        let rule = rules.get_rule(1).unwrap();
+        assert_eq!(rule.action(), Some(EolAction::Delete));
+    }
+
+    #[test]
+    fn test_set_action_on_rule_nonexistent() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        
+        let result = rules.set_action_on_rule(999, &EolAction::Delete);
+        assert!(result.is_err());
+        
+        match result.unwrap_err() {
+            Error::RuleNotFound(id) => {
+                assert_eq!(id, 999);
+            }
+            _ => panic!("Expected RuleNotFound error"),
+        }
+    }
+
+    #[test]
+    fn test_list_rules_succeeds() {
+        setup_test_environment();
+        
+        let rules = Rules::new();
+        
+        // Should not panic or return error
+        let result = rules.list_rules();
+        assert!(result.is_ok());
+    }
+
+    // Integration tests for save/load would require file system setup
+    // These are marked as ignore to avoid interference with actual config files
+    #[test]
+    #[ignore = "Integration test that modifies file system"]
+    fn test_save_and_load_roundtrip() {
+        setup_test_environment();
+        
+        let mut rules = Rules::new();
+        let retention = Retention::new(MessageAge::Days(30), false);
+        rules.add_rule(retention, Some(&"save-test".to_string()), false);
+        
+        // Save to disk
+        let save_result = rules.save();
+        assert!(save_result.is_ok());
+        
+        // Load from disk
+        let loaded_rules = Rules::load();
+        assert!(loaded_rules.is_ok());
+        
+        let loaded_rules = loaded_rules.unwrap();
+        let labels = loaded_rules.labels();
+        assert!(labels.contains(&"save-test".to_string()));
+    }
+}