Skip to content

Commit 2d9729f

Browse files
committed
refactor(tests): address code review suggestions
- Fix type mismatch: change i32 to i64 for ore table id column - Extract get_ore_encrypted helper to shared module (tests/sqlx/src/helpers.rs) - Add missing jsonb <= e reverse direction test for symmetry - Fix QueryAssertion pattern inconsistency (remove intermediate variable) All non-blocking code review suggestions addressed.
1 parent 289ee11 commit 2d9729f

File tree

4 files changed

+53
-41
lines changed

4 files changed

+53
-41
lines changed

tests/sqlx/src/helpers.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//! Test helper functions for EQL tests
2+
//!
3+
//! Common utilities for working with encrypted data in tests.
4+
5+
use anyhow::{Context, Result};
6+
use sqlx::{PgPool, Row};
7+
8+
/// Fetch ORE encrypted value from pre-seeded ore table
9+
///
10+
/// The ore table is created by migration `002_install_ore_data.sql`
11+
/// and contains 99 pre-seeded records (ids 1-99) for testing.
12+
pub async fn get_ore_encrypted(pool: &PgPool, id: i32) -> Result<String> {
13+
let sql = format!("SELECT e::text FROM ore WHERE id = {}", id);
14+
let row = sqlx::query(&sql)
15+
.fetch_one(pool)
16+
.await
17+
.with_context(|| format!("fetching ore encrypted value for id={}", id))?;
18+
19+
let result: Option<String> = row
20+
.try_get(0)
21+
.with_context(|| format!("extracting text column for id={}", id))?;
22+
23+
result.with_context(|| format!("ore table returned NULL for id={}", id))
24+
}

tests/sqlx/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@
55
use sqlx::PgPool;
66

77
pub mod assertions;
8+
pub mod helpers;
89
pub mod index_types;
910
pub mod selectors;
1011

1112
pub use assertions::QueryAssertion;
13+
pub use helpers::get_ore_encrypted;
1214
pub use index_types as IndexTypes;
1315
pub use selectors::Selectors;
1416

tests/sqlx/tests/comparison_tests.rs

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,9 @@
44
//! Tests EQL comparison operators with ORE (Order-Revealing Encryption)
55
66
use anyhow::{Context, Result};
7-
use eql_tests::QueryAssertion;
7+
use eql_tests::{get_ore_encrypted, QueryAssertion};
88
use sqlx::{PgPool, Row};
99

10-
/// Helper to fetch ORE encrypted value from pre-seeded ore table
11-
async fn get_ore_encrypted(pool: &PgPool, id: i32) -> Result<String> {
12-
let sql = format!("SELECT e::text FROM ore WHERE id = {}", id);
13-
let row = sqlx::query(&sql)
14-
.fetch_one(pool)
15-
.await
16-
.with_context(|| format!("fetching ore encrypted value for id={}", id))?;
17-
18-
let result: Option<String> = row.try_get(0).with_context(|| {
19-
format!("extracting text column for id={}", id)
20-
})?;
21-
22-
result.with_context(|| {
23-
format!("ore table returned NULL for id={}", id)
24-
})
25-
}
26-
2710

2811
/// Helper to fetch ORE encrypted value as JSONB for comparison
2912
///
@@ -307,6 +290,24 @@ async fn less_than_or_equal_with_jsonb(pool: PgPool) -> Result<()> {
307290
Ok(())
308291
}
309292

293+
#[sqlx::test]
294+
async fn less_than_or_equal_jsonb_lte_encrypted(pool: PgPool) -> Result<()> {
295+
// Test: jsonb <= e with ORE (reverse direction)
296+
// Complements e <= jsonb test for symmetry with other operators
297+
298+
let json_value = get_ore_encrypted_as_jsonb(&pool, 42).await?;
299+
300+
let sql = format!(
301+
"SELECT id FROM ore WHERE '{}'::jsonb <= e",
302+
json_value
303+
);
304+
305+
// jsonb(42) <= e means e >= 42, so 58 records (42-99)
306+
QueryAssertion::new(&pool, &sql).count(58).await;
307+
308+
Ok(())
309+
}
310+
310311
// ============================================================================
311312
// Task 5: Greater Than or Equal (>=) Operator Tests
312313
// ============================================================================

tests/sqlx/tests/order_by_tests.rs

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,24 +4,10 @@
44
//! Tests ORDER BY with ORE (Order-Revealing Encryption)
55
//! Uses ore table from migrations/002_install_ore_data.sql (ids 1-99)
66
7-
use anyhow::{Context, Result};
8-
use eql_tests::QueryAssertion;
7+
use anyhow::Result;
8+
use eql_tests::{get_ore_encrypted, QueryAssertion};
99
use sqlx::{PgPool, Row};
1010

11-
async fn get_ore_encrypted(pool: &PgPool, id: i32) -> Result<String> {
12-
let sql = format!("SELECT e::text FROM ore WHERE id = {}", id);
13-
let row = sqlx::query(&sql)
14-
.fetch_one(pool)
15-
.await
16-
.with_context(|| format!("fetching ore encrypted value for id={}", id))?;
17-
18-
let result: Option<String> = row
19-
.try_get(0)
20-
.with_context(|| format!("extracting text column for id={}", id))?;
21-
22-
result.with_context(|| format!("ore table returned NULL for id={}", id))
23-
}
24-
2511
#[sqlx::test]
2612
async fn order_by_desc_returns_highest_value_first(pool: PgPool) -> Result<()> {
2713
// Test: ORDER BY e DESC returns records in descending order
@@ -36,12 +22,11 @@ async fn order_by_desc_returns_highest_value_first(pool: PgPool) -> Result<()> {
3622
);
3723

3824
// Should return 41 records, highest first
39-
let assertion = QueryAssertion::new(&pool, &sql);
40-
assertion.count(41).await;
25+
QueryAssertion::new(&pool, &sql).count(41).await;
4126

4227
// First record should be id=41
4328
let row = sqlx::query(&sql).fetch_one(&pool).await?;
44-
let first_id: i32 = row.try_get(0)?;
29+
let first_id: i64 = row.try_get(0)?;
4530
assert_eq!(first_id, 41, "ORDER BY DESC should return id=41 first");
4631

4732
Ok(())
@@ -60,7 +45,7 @@ async fn order_by_desc_with_limit(pool: PgPool) -> Result<()> {
6045
);
6146

6247
let row = sqlx::query(&sql).fetch_one(&pool).await?;
63-
let id: i32 = row.try_get(0)?;
48+
let id: i64 = row.try_get(0)?;
6449
assert_eq!(id, 41, "Should return id=41 (highest value < 42)");
6550

6651
Ok(())
@@ -79,7 +64,7 @@ async fn order_by_asc_with_limit(pool: PgPool) -> Result<()> {
7964
);
8065

8166
let row = sqlx::query(&sql).fetch_one(&pool).await?;
82-
let id: i32 = row.try_get(0)?;
67+
let id: i64 = row.try_get(0)?;
8368
assert_eq!(id, 1, "Should return id=1 (lowest value < 42)");
8469

8570
Ok(())
@@ -116,7 +101,7 @@ async fn order_by_desc_with_greater_than_returns_highest(pool: PgPool) -> Result
116101
);
117102

118103
let row = sqlx::query(&sql).fetch_one(&pool).await?;
119-
let id: i32 = row.try_get(0)?;
104+
let id: i64 = row.try_get(0)?;
120105
assert_eq!(id, 99, "Should return id=99 (highest value > 42)");
121106

122107
Ok(())
@@ -135,7 +120,7 @@ async fn order_by_asc_with_greater_than_returns_lowest(pool: PgPool) -> Result<(
135120
);
136121

137122
let row = sqlx::query(&sql).fetch_one(&pool).await?;
138-
let id: i32 = row.try_get(0)?;
123+
let id: i64 = row.try_get(0)?;
139124
assert_eq!(id, 43, "Should return id=43 (lowest value > 42)");
140125

141126
Ok(())

0 commit comments

Comments
 (0)