test: enhance duplicate validation with modular testcase logic (#4)

- Refactor verify_duplicate_testcase into organized duplicate_test module
- Add comprehensive timestamp-based duplicate correctness validation
- Improve test output formatting with clear PASSED/FILTERED status
- Enhance error messages for better debugging of test failures
- Add detailed debug output for duplicate detection timing validation

The test infrastructure now provides precise validation of which duplicate
was kept during deduplication, making it easier to catch Lambda bugs
where wrong duplicates are retained instead of "last occurrence wins".

Author: JohnBasrai

CHANGELOG.md

@@ -5,7 +5,6 @@ All notable changes to this project will be documented in this file.
 The format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project uses [semantic versioning](https://semver.org/).
 
 ---
-
 ## [Unreleased]
 
 ### Changed
@@ -16,9 +15,23 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and
   - Added comprehensive boundary testing for time-based filtering rules
   - Self-documenting test cases with clear business rule descriptions
 - **Enhanced Test Reporting**: Added aligned, formatted test output for better readability
+- **Improved Test Validation**: Enhanced duplicate detection logic with precise timestamp validation
+  - Added `verify_duplicate_testcase` function with comprehensive error messaging
+  - Implemented detailed test case validation showing PASSED vs FILTERED status
+  - Refactored test logic into organized `duplicate_test` module for better maintainability
+
+### Added
+- **Comprehensive Edge Case Testing**: Dynamic test data generation for boundary conditions
+  - Automated testing of 7-day and 90-day filtering boundaries
+  - Real-time duplicate detection validation with timing precision
+- **Enhanced Test Output**: Clear, professional test result formatting
+  - Color-coded test status (✅/❌) with descriptive failure messages
+  - Detailed debug output for duplicate validation logic
 
 ### Technical
 - Improved test maintainability with descriptive entity names and test descriptions
+- Modularized duplicate testing logic for better code organization
+- Enhanced error messages for easier debugging of test failures
 
 ---
 

Dockerfile.lambda-runtime

@@ -18,6 +18,9 @@ RUN rustup component add rustfmt
 # Install cargo-lambda
 RUN cargo install cargo-lambda --version 1.8.5 --locked --quiet
 
+# Install clippy component
+RUN rustup component add clippy
+
 # Default user
 RUN useradd -m dev
 WORKDIR /app

scripts/build.sh

@@ -22,11 +22,13 @@ if ! nc -z localhost 9000; then
   exit 1
 fi
 
+RUN="docker exec aws-lambda-action-filter-lambda-1"
 
 # Run all tests inside the container
-docker exec aws-lambda-action-filter-lambda-1 cargo fmt --version
-docker exec aws-lambda-action-filter-lambda-1 cargo fmt --check
-docker exec aws-lambda-action-filter-lambda-1 cargo build --release --quiet
-docker exec aws-lambda-action-filter-lambda-1 cargo test --release --quiet
+${RUN} cargo fmt --version
+${RUN} cargo fmt --check
+${RUN} cargo clippy --quiet --all-features -- -D warnings
+${RUN} cargo build --release --quiet
+${RUN} cargo test --release --quiet -- --nocapture
 
 echo "All tests passed!"

tests/edge_case_tests.rs

@@ -37,16 +37,18 @@ struct TestCase {
     description: &'static str,
 }
 
+const DUP_TEST_ID: &str = "dedup_test_id";
+
 #[rustfmt::skip]
 const EDGE_CASES: &[TestCase] = &[
-    TestCase { entity_id: "dedup_first_occurrence",        next_offset: 30, last_offset: -10, priority: Priority::Urgent, should_pass: true,  description: "Tests deduplication (first occurrence)" },
-    TestCase { entity_id: "dedup_first_occurrence",        next_offset: 35, last_offset: -15, priority: Priority::Normal, should_pass: true,  description: "Tests deduplication (last occurrence wins)" },
-    TestCase { entity_id: "more_than_7_days_ago_fail",     next_offset: 20, last_offset: -7,  priority: Priority::Urgent, should_pass: false, description: "Tests 'more than 7 days ago' rule (should fail)" },
-    TestCase { entity_id: "more_than_7_days_ago_pass",     next_offset: 20, last_offset: -8,  priority: Priority::Urgent, should_pass: true,  description: "Tests 'more than 7 days ago' rule (should pass)" },
-    TestCase { entity_id: "more_than_7_days_ago_pass_2",   next_offset: 25, last_offset: -10, priority: Priority::Urgent, should_pass: true,  description: "Tests 'more than 7 days ago' rule (should pass)" },
-    TestCase { entity_id: "within_90_days_fail",           next_offset: 91, last_offset: -30, priority: Priority::Normal, should_pass: false, description: "Tests 'within 90 days' rule (should fail at 91 days)" },
-    TestCase { entity_id: "within_90_days_pass",           next_offset: 90, last_offset: -30, priority: Priority::Normal, should_pass: true,  description: "Tests 'within 90 days' rule boundary (should pass)" },
-    TestCase { entity_id: "within_90_days_pass_2",         next_offset: 89, last_offset: -20, priority: Priority::Normal, should_pass: true,  description: "Tests 'within 90 days' rule (should pass)" },
+    TestCase { entity_id: DUP_TEST_ID,                   next_offset: 30, last_offset: -10, priority: Priority::Urgent, should_pass: false, description: "Rule: deduplication (first occurrence, expected to be FILTERED)" },
+    TestCase { entity_id: DUP_TEST_ID,                   next_offset: 35, last_offset: -15, priority: Priority::Normal, should_pass: true,  description: "Rule: deduplication (last occurrence wins, expected to be PASSED)" },
+    TestCase { entity_id: "more_than_7_days_ago_fail",   next_offset: 20, last_offset: -7,  priority: Priority::Urgent, should_pass: false, description: "Rule: more than 7 days ago (fail <7)" },
+    TestCase { entity_id: "more_than_7_days_ago_pass",   next_offset: 20, last_offset: -8,  priority: Priority::Urgent, should_pass: true,  description: "Rule: more than 7 days ago (pass =7)" },
+    TestCase { entity_id: "more_than_7_days_ago_pass_2", next_offset: 25, last_offset: -10, priority: Priority::Urgent, should_pass: true,  description: "Rule: more than 7 days ago (pass >7)" },
+    TestCase { entity_id: "within_90_days_fail",         next_offset: 91, last_offset: -30, priority: Priority::Normal, should_pass: false, description: "Rule: within 90 days (fail >90)" },
+    TestCase { entity_id: "within_90_days_pass",         next_offset: 90, last_offset: -30, priority: Priority::Normal, should_pass: true,  description: "Rule: within 90 days (pass =90)" },
+    TestCase { entity_id: "within_90_days_pass_2",       next_offset: 89, last_offset: -20, priority: Priority::Normal, should_pass: true,  description: "Rule: within 90 days (pass <90)" },
 ];
 
 fn create_action(
@@ -84,51 +86,68 @@ fn generate_test_data() -> Result<String> {
     Ok(json)
 }
 
-fn verify_test_expectations(results: &[Action]) -> Result<()> {
+fn verify_test_expectations(results: &[Action]) -> (bool, Vec<String>) {
     // ---
-    let prefix = "verify_test_expectations";
-
     // Convert results to a map for O(1) lookup
     let result_map: HashMap<&str, &Action> =
         results.iter().map(|action| (action.entity_id.as_str(), action)).collect();
 
+    let mut lines = Vec::new();
+    let mut passed = true; // To be set only if setting to false
+
     // Iterate over test expectations and verify against results
     for test_case in EDGE_CASES {
         // ---
-        let found_in_results = result_map.contains_key(test_case.entity_id);
+        let found = result_map.contains_key(test_case.entity_id);
+
+        if test_case.entity_id == DUP_TEST_ID {
+            // ---
+            let (pass, line) =
+                duplicate_test::verify_duplicate_testcase(test_case, &result_map, found);
+
+            lines.push(line);
+
+            if !pass {
+                passed = false;
+            }
+            continue; // Skip to next test case
+        }
 
-        match (test_case.should_pass, found_in_results) {
+        match (test_case.should_pass, found) {
             // ---
             (true, false) => {
                 // ---
-                ensure!(
-                    false,
-                    "{prefix}: {} - Expected to pass but was filtered out. {}",
-                    test_case.entity_id,
-                    test_case.description
-                );
+                lines.push(format!(
+                    "❌ {:<28}: FILTERED   - Expected to PASS but was not found in results. {}-2",
+                    test_case.entity_id, test_case.description
+                ));
+                passed = false;
             }
             (false, true) => {
                 // ---
-                ensure!(
-                    false,
-                    "{prefix}: {} - Expected to be filtered out but found in results. {}",
-                    test_case.entity_id,
-                    test_case.description
-                );
+                lines.push(format!(
+                    "❌ {:<28}: FILTERED   - Expected to be FILTERED but was found in results. {}-3",
+                    test_case.entity_id, test_case.description
+                ));
+                passed = false;
             }
             (true, true) => {
                 // ---
-                println!("✓ {:<28}: PASS     - {}", test_case.entity_id, test_case.description);
+                lines.push(format!(
+                    "✅ {:<28}: PASSED   - {}",
+                    test_case.entity_id, test_case.description
+                ));
             }
             (false, false) => {
                 // ---
-                println!("✓ {:<28}: FILTERED - {}", test_case.entity_id, test_case.description);
+                lines.push(format!(
+                    "✅ {:<28}: FILTERED - {}",
+                    test_case.entity_id, test_case.description
+                ));
             }
         }
     }
-
-    Ok(())
+    (passed, lines)
 }
 
 #[test]
@@ -152,13 +171,22 @@ fn test_dynamic_edge_cases() -> Result<()> {
     let results = run_lambda_invoke(temp_file)?;
 
     println!("Lambda returned {} actions", results.len());
+    results.iter().for_each(|action| {
+        println!("  :: {}", action.entity_id);
+    });
+    print!("---\n\n");
 
     // Verify all test expectations
-    verify_test_expectations(&results)?;
+    let (passed, lines) = verify_test_expectations(&results);
+    let lines = lines.join("\n");
+
+    println!("\nAll TestCase Results:");
+    println!("{lines}");
+    ensure!(passed, "Test failed");
 
     // Additional verification: check expected count
     // Should have 5 actions: 6 that should pass - 1 duplicate = 5
-    // (dedup_first_occurrence appears twice but deduplicated to 1)
+    // (DUP_TEST_ID appears twice but deduplicated to 1)
     let expected_count = 5;
     ensure!(
         results.len() == expected_count,
@@ -181,8 +209,7 @@ fn test_dynamic_edge_cases() -> Result<()> {
     }
 
     // Verify deduplication worked correctly
-    let duplicate_count =
-        results.iter().filter(|a| a.entity_id == "dedup_first_occurrence").count();
+    let duplicate_count = results.iter().filter(|a| a.entity_id == DUP_TEST_ID).count();
     ensure!(
         duplicate_count == 1,
         "Expected exactly 1 'duplicate' entity after deduplication, found {}",
@@ -210,3 +237,121 @@ fn test_dynamic_edge_cases() -> Result<()> {
 
     Ok(())
 }
+
+mod duplicate_test {
+    // ---
+    use super::{Action, TestCase};
+    use chrono::{Duration, Utc};
+    use std::collections::HashMap;
+
+    pub fn verify_duplicate_testcase(
+        test_case: &TestCase,
+        result_map: &HashMap<&str, &Action>,
+        is_found: bool,
+    ) -> (bool, String) {
+        // ---
+        // Early return: Handle case where entity was not found in results
+        if !is_found {
+            // ---
+            return handle_entity_not_found(test_case);
+        }
+
+        // Entity was found - check if it's the correct duplicate
+        let duplicate_check_result = check_duplicate_correctness(test_case, result_map);
+
+        match (test_case.should_pass, duplicate_check_result.is_correct) {
+            // ---
+            (true, true) => {
+                // Expected to pass, and correct duplicate was kept
+                (true, format_success_message(test_case, "PASSED"))
+            }
+            (true, false) => {
+                // Expected to pass, but wrong duplicate was kept
+                (false, format_wrong_duplicate_error(test_case, &duplicate_check_result))
+            }
+            (false, true) => {
+                // Expected to be filtered, but correct duplicate was kept (this shouldn't happen)
+                (false, format_unexpected_correct_duplicate_error(test_case))
+            }
+            (false, false) => {
+                // Expected to be filtered, and wrong duplicate was kept (which is fine)
+                (true, format_success_message(test_case, "FILTERED"))
+            }
+        }
+    }
+
+    // Helper function to handle when entity is not found in results
+    fn handle_entity_not_found(test_case: &TestCase) -> (bool, String) {
+        // ---
+        if test_case.should_pass {
+            // Expected to pass but was filtered out
+            (
+                false,
+                format!(
+                    "❌ {:<28}: FAILED   - {} Expected to PASS but was FILTERED.",
+                    test_case.entity_id, test_case.description
+                ),
+            )
+        } else {
+            // Expected to be filtered and was filtered
+            (
+                true,
+                format!(
+                    "✅ {:<28}: PASSED   - {} (was not passed, which is what we expected)",
+                    test_case.entity_id, test_case.description
+                ),
+            )
+        }
+    }
+
+    // Struct to hold duplicate correctness check results
+    struct DuplicateCheckResult {
+        is_correct: bool,
+        time_diff: i64,
+    }
+
+    // Helper function to check if the correct duplicate was kept
+    fn check_duplicate_correctness(
+        test_case: &TestCase,
+        result_map: &HashMap<&str, &Action>,
+    ) -> DuplicateCheckResult {
+        // ---
+        let action = result_map.get(test_case.entity_id).unwrap();
+        let now = Utc::now();
+        let expected_next_time = now + Duration::days(test_case.next_offset);
+
+        let time_diff = (action.next_action_time - expected_next_time).num_seconds().abs();
+        let is_correct = time_diff < 60; // Allow small time differences
+
+        DuplicateCheckResult { is_correct, time_diff }
+    }
+
+    // Helper functions for formatting messages
+    fn format_success_message(test_case: &TestCase, status: &str) -> String {
+        // ---
+        format!("✅ {:<28}: {:<8} - {}", test_case.entity_id, status, test_case.description)
+    }
+
+    fn format_wrong_duplicate_error(
+        test_case: &TestCase,
+        check_result: &DuplicateCheckResult,
+    ) -> String {
+        // ---
+        format!(
+            "❌ {:<28}: FILTERED - {}. Wrong DUPLICATE kept, \
+             Expected next_offset: {}, but found action with different timing. {}",
+            test_case.entity_id,
+            test_case.description,
+            test_case.next_offset,
+            check_result.time_diff
+        )
+    }
+
+    fn format_unexpected_correct_duplicate_error(test_case: &TestCase) -> String {
+        // ---
+        format!(
+            "❌ {:<28}: FAILED   - {} Expected to be FILTERED but correct duplicate was found in results.",
+            test_case.entity_id, test_case.description
+        )
+    }
+}