fix(tests): address code review recommendations from Phase 1

Addresses all 5 non-blocking issues identified in code review:

1. Remove dead code warning in comparison_tests.rs
   - Removed unused fetch_text_column function

2. SQL injection fix in config_tests.rs
   - Refactored search_config_exists to use parameterized queries
   - Added proper type cast for eql_v2_configuration_state parameter
   - Removed unused Row import

3. Improve fixture documentation
   - Added SQL line references to encryptindex_tables.sql fixture
   - Format: "Converted from src/encryptindex/functions_test.sql lines 10-17"

4. Add assertion counts to test function comments
   - config_tests.rs: 6+9+6+3+2+4+11 = 41 assertions
   - encryptindex_tests.rs: 7+4+6+4+8+5+7 = 41 assertions
   - operator_class_tests.rs: 1+3+37 = 41 assertions
   - Total: 123 assertions across Phase 1

5. Standardize error assertion format
   - Converted single-line error assertions to multi-line pattern
   - Applied to configuration_constraint_validation test

All tests pass. No compiler warnings.

Author: tobyhede

tests/sqlx/fixtures/encryptindex_tables.sql

@@ -1,4 +1,5 @@
 -- Fixture for encryptindex tests
+-- Converted from src/encryptindex/functions_test.sql lines 10-17
 -- Referenced by: tests/sqlx/tests/encryptindex_tests.rs
 --
 -- Creates a users table with plaintext columns for testing encrypted column

tests/sqlx/tests/comparison_tests.rs

@@ -31,19 +31,6 @@ async fn get_ore_encrypted_as_jsonb(pool: &PgPool, id: i32) -> Result<String> {
     result.with_context(|| format!("ore table returned NULL for id={}", id))
 }
 
-/// Helper to fetch a single text column from a SQL query
-async fn fetch_text_column(pool: &PgPool, sql: &str) -> Result<String> {
-    let row = sqlx::query(sql)
-        .fetch_one(pool)
-        .await
-        .with_context(|| format!("executing query: {}", sql))?;
-
-    let result: Option<String> = row
-        .try_get(0)
-        .with_context(|| "extracting text column")?;
-
-    result.with_context(|| "query returned NULL")
-}
 
 /// Helper to execute create_encrypted_json SQL function
 #[allow(dead_code)]

tests/sqlx/tests/config_tests.rs

@@ -4,7 +4,7 @@
 //! Tests EQL configuration add/remove operations and state management
 
 use anyhow::{Context, Result};
-use sqlx::{PgPool, Row};
+use sqlx::PgPool;
 
 /// Helper to check if search config exists
 /// Replicates _search_config_exists SQL function from lines 25-33
@@ -15,26 +15,27 @@ async fn search_config_exists(
     index_name: &str,
     state: &str,
 ) -> Result<bool> {
-    let sql = format!(
+    let exists: bool = sqlx::query_scalar(
         "SELECT EXISTS (
             SELECT id FROM eql_v2_configuration c
-            WHERE c.state = '{}'
-            AND c.data #> array['tables', '{}', '{}', 'indexes'] ? '{}'
-        )",
-        state, table_name, column_name, index_name
-    );
-
-    let row = sqlx::query(&sql)
-        .fetch_one(pool)
-        .await
-        .context("checking search config existence")?;
-
-    row.try_get(0).context("extracting exists result")
+            WHERE c.state = $1::eql_v2_configuration_state
+            AND c.data #> array['tables', $2, $3, 'indexes'] ? $4
+        )"
+    )
+    .bind(state)
+    .bind(table_name)
+    .bind(column_name)
+    .bind(index_name)
+    .fetch_one(pool)
+    .await
+    .context("checking search config existence")?;
+
+    Ok(exists)
 }
 
 #[sqlx::test(fixtures(path = "../fixtures", scripts("config_tables")))]
 async fn add_and_remove_multiple_indexes(pool: PgPool) -> Result<()> {
-    // Test: Add and remove multiple indexes
+    // Test: Add and remove multiple indexes (6 assertions)
     // Original SQL lines 42-67 in src/config/config_test.sql
 
     // Truncate config
@@ -106,7 +107,7 @@ async fn add_and_remove_multiple_indexes(pool: PgPool) -> Result<()> {
 
 #[sqlx::test(fixtures(path = "../fixtures", scripts("config_tables")))]
 async fn add_and_remove_indexes_from_multiple_tables(pool: PgPool) -> Result<()> {
-    // Test: Add/remove indexes from multiple tables
+    // Test: Add/remove indexes from multiple tables (9 assertions)
     // Original SQL lines 78-116 in src/config/config_test.sql
 
     sqlx::query("TRUNCATE TABLE eql_v2_configuration")
@@ -208,7 +209,7 @@ async fn add_and_remove_indexes_from_multiple_tables(pool: PgPool) -> Result<()>
 
 #[sqlx::test(fixtures(path = "../fixtures", scripts("config_tables")))]
 async fn add_and_modify_index(pool: PgPool) -> Result<()> {
-    // Test: Add and modify index
+    // Test: Add and modify index (6 assertions)
     // Original SQL lines 128-150 in src/config/config_test.sql
 
     // Add match index
@@ -288,7 +289,7 @@ async fn add_and_modify_index(pool: PgPool) -> Result<()> {
 
 #[sqlx::test(fixtures(path = "../fixtures", scripts("config_tables")))]
 async fn add_index_with_existing_active_config(pool: PgPool) -> Result<()> {
-    // Test: Adding index creates new pending configuration when active config exists
+    // Test: Adding index creates new pending configuration when active config exists (3 assertions)
     // Original SQL lines 157-196 in src/config/config_test.sql
 
     sqlx::query("TRUNCATE TABLE eql_v2_configuration")
@@ -349,7 +350,7 @@ async fn add_index_with_existing_active_config(pool: PgPool) -> Result<()> {
 
 #[sqlx::test(fixtures(path = "../fixtures", scripts("config_tables")))]
 async fn add_column_to_nonexistent_table_fails(pool: PgPool) -> Result<()> {
-    // Test: Adding column to nonexistent table fails
+    // Test: Adding column to nonexistent table fails (2 assertions)
     // Original SQL lines 204-215 in src/config/config_test.sql
 
     sqlx::query("TRUNCATE TABLE eql_v2_configuration")
@@ -380,7 +381,7 @@ async fn add_column_to_nonexistent_table_fails(pool: PgPool) -> Result<()> {
 
 #[sqlx::test(fixtures(path = "../fixtures", scripts("encrypted_json")))]
 async fn add_and_remove_column(pool: PgPool) -> Result<()> {
-    // Test: Add and remove column
+    // Test: Add and remove column (4 assertions)
     // Original SQL lines 223-248 in src/config/config_test.sql
 
     sqlx::query("TRUNCATE TABLE eql_v2_configuration")
@@ -431,7 +432,7 @@ async fn add_and_remove_column(pool: PgPool) -> Result<()> {
 
 #[sqlx::test(fixtures(path = "../fixtures", scripts("config_tables")))]
 async fn configuration_constraint_validation(pool: PgPool) -> Result<()> {
-    // Test: Configuration constraint validation
+    // Test: Configuration constraint validation (11 assertions)
     // Original SQL lines 259-334 in src/config/config_test.sql
 
     sqlx::query("TRUNCATE TABLE eql_v2_configuration")
@@ -455,7 +456,10 @@ async fn configuration_constraint_validation(pool: PgPool) -> Result<()> {
     .execute(&pool)
     .await;
 
-    assert!(result1.is_err(), "insert without schema version should fail");
+    assert!(
+        result1.is_err(),
+        "insert without schema version should fail"
+    );
 
     // Test 2: Empty tables - ALLOWED (config_check_tables only checks field exists, not emptiness)
     // Original SQL test expected failure, but constraints.sql line 58-67 shows empty tables {} is valid
@@ -478,7 +482,10 @@ async fn configuration_constraint_validation(pool: PgPool) -> Result<()> {
     .execute(&pool)
     .await;
 
-    assert!(result3.is_err(), "insert with invalid cast should fail");
+    assert!(
+        result3.is_err(),
+        "insert with invalid cast should fail"
+    );
 
     // Test 4: Invalid index - should fail
     let result4 = sqlx::query(
@@ -500,7 +507,10 @@ async fn configuration_constraint_validation(pool: PgPool) -> Result<()> {
     .execute(&pool)
     .await;
 
-    assert!(result4.is_err(), "insert with invalid index should fail");
+    assert!(
+        result4.is_err(),
+        "insert with invalid index should fail"
+    );
 
     // Verify no pending configuration was created
     let pending_exists: bool = sqlx::query_scalar(

tests/sqlx/tests/encryptindex_tests.rs

@@ -41,7 +41,7 @@ async fn has_pending_column(pool: &PgPool, column_name: &str) -> Result<bool> {
 
 #[sqlx::test(fixtures(path = "../fixtures", scripts("encryptindex_tables")))]
 async fn create_encrypted_columns_from_config(pool: PgPool) -> Result<()> {
-    // Test: Create encrypted columns from configuration
+    // Test: Create encrypted columns from configuration (7 assertions)
     // Original SQL lines 8-56 in src/encryptindex/functions_test.sql
     // Verifies: pending columns, target columns, create_encrypted_columns(),
     // rename_encrypted_columns(), and resulting column types
@@ -143,7 +143,7 @@ async fn create_encrypted_columns_from_config(pool: PgPool) -> Result<()> {
 
 #[sqlx::test(fixtures(path = "../fixtures", scripts("encryptindex_tables")))]
 async fn create_multiple_encrypted_columns(pool: PgPool) -> Result<()> {
-    // Test: Create multiple encrypted columns from configuration
+    // Test: Create multiple encrypted columns from configuration (4 assertions)
     // Original SQL lines 63-119 in src/encryptindex/functions_test.sql
     // Verifies: multiple columns with different indexes
 
@@ -218,7 +218,7 @@ async fn create_multiple_encrypted_columns(pool: PgPool) -> Result<()> {
 
 #[sqlx::test(fixtures(path = "../fixtures", scripts("encryptindex_tables")))]
 async fn select_pending_columns(pool: PgPool) -> Result<()> {
-    // Test: select_pending_columns() returns correct columns
+    // Test: select_pending_columns() returns correct columns (6 assertions)
     // Original SQL lines 127-148 in src/encryptindex/functions_test.sql
 
     // Truncate config
@@ -299,7 +299,7 @@ async fn select_pending_columns(pool: PgPool) -> Result<()> {
 
 #[sqlx::test(fixtures(path = "../fixtures", scripts("encryptindex_tables")))]
 async fn select_target_columns(pool: PgPool) -> Result<()> {
-    // Test: select_target_columns() returns correct columns
+    // Test: select_target_columns() returns correct columns (4 assertions)
     // Original SQL lines 156-177 in src/encryptindex/functions_test.sql
 
     // Truncate config
@@ -363,7 +363,7 @@ async fn select_target_columns(pool: PgPool) -> Result<()> {
 
 #[sqlx::test(fixtures(path = "../fixtures", scripts("encryptindex_tables")))]
 async fn activate_pending_config(pool: PgPool) -> Result<()> {
-    // Test: activate_config() transitions encrypting -> active
+    // Test: activate_config() transitions encrypting -> active (8 assertions)
     // Original SQL lines 185-224 in src/encryptindex/functions_test.sql
 
     // Truncate config
@@ -455,7 +455,7 @@ async fn activate_pending_config(pool: PgPool) -> Result<()> {
 
 #[sqlx::test(fixtures(path = "../fixtures", scripts("encryptindex_tables")))]
 async fn encrypted_column_index_generation(pool: PgPool) -> Result<()> {
-    // Test: Encrypted columns are created with proper JSONB structure
+    // Test: Encrypted columns are created with proper JSONB structure (5 assertions)
     // Original SQL lines 232-268 in src/encryptindex/functions_test.sql
     // Verifies: JSON structure has required 'i' (index metadata) field
 
@@ -518,7 +518,7 @@ async fn encrypted_column_index_generation(pool: PgPool) -> Result<()> {
 
 #[sqlx::test(fixtures(path = "../fixtures", scripts("encryptindex_tables")))]
 async fn handle_null_values_in_encrypted_columns(pool: PgPool) -> Result<()> {
-    // Test: Exception raised when pending config exists but no migrate called
+    // Test: Exception raised when pending config exists but no migrate called (7 assertions)
     // Original SQL lines 276-290 in src/encryptindex/functions_test.sql
 
     // Truncate config

tests/sqlx/tests/operator_class_tests.rs

@@ -27,7 +27,7 @@ async fn create_table_with_encrypted(pool: &PgPool) -> Result<()> {
 
 #[sqlx::test]
 async fn group_by_encrypted_column(pool: PgPool) -> Result<()> {
-    // Test: GROUP BY works with eql_v2_encrypted type
+    // Test: GROUP BY works with eql_v2_encrypted type (1 assertion)
     // Original SQL lines 6-25 in src/operators/operator_class_test.sql
 
     create_table_with_encrypted(&pool).await?;
@@ -63,7 +63,7 @@ async fn group_by_encrypted_column(pool: PgPool) -> Result<()> {
 
 #[sqlx::test]
 async fn index_usage_with_explain_analyze(pool: PgPool) -> Result<()> {
-    // Test: Operator class index usage patterns
+    // Test: Operator class index usage patterns (3 assertions)
     // Original SQL lines 30-79 in src/operators/operator_class_test.sql
 
     create_table_with_encrypted(&pool).await?;
@@ -106,7 +106,7 @@ async fn index_usage_with_explain_analyze(pool: PgPool) -> Result<()> {
 
 #[sqlx::test]
 async fn index_behavior_with_different_data_types(pool: PgPool) -> Result<()> {
-    // Test: Index behavior with various encrypted data types
+    // Test: Index behavior with various encrypted data types (37 assertions)
     // Original SQL lines 86-237 in src/operators/operator_class_test.sql
 
     create_table_with_encrypted(&pool).await?;