test: use separate database for testing #227

danielwpz · 2025-12-04T02:22:18Z

Note

Use a separate test database with automatic reset for API integration tests, update CI to run them uniformly, and adjust tests/utilities accordingly.

Testing infrastructure:
- Add tests/common/db_setup.rs to manage a dedicated test DB (TEST_DATABASE_NAME, default platform_api_test) with safe reset via tokio-postgres and OnceCell to run once per suite.
- Update tests/common/mod.rs to use test DB name, perform one-time reset and migrations, and return Database from setup_test_server_with_pool.
- Refactor repository tests to reuse the shared test server DB pool instead of manual configs.
E2E tests:
- Add setup_qwen_model(&server) initialization to various tests; minor adjustments (e.g., VPC login creates org; tuple returns now include Database).
CI:
- Simplify integration test step to cargo test -p api --tests and set TEST_DATABASE_NAME in workflow.
Dependencies:
- Add tokio-postgres to dev-dependencies (and lockfile) for test DB management.

^{Written by Cursor Bugbot for commit c3ee363. This will update automatically on new commits. Configure here.}

Copilot

Pull request overview

This PR introduces a separate test database configuration to isolate test execution from the development database, preventing data corruption and ensuring test independence.

Key Changes:

Implemented database reset logic that creates a fresh test database before each test run
Refactored test setup to use the new test database configuration
Updated CI workflow to run all tests in a single command rather than looping through individual test files

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
env.example	Adds `DATABASE_TEST_NAME` environment variable for configuring the test database name
crates/api/tests/common/db_setup.rs	New module implementing test database reset logic with safety checks
crates/api/tests/common/mod.rs	Updates test setup to use the test database and returns database instance from `setup_test_server_with_pool`
crates/api/tests/e2e_repositories.rs	Refactors to use centralized test pool setup instead of custom database configuration
crates/api/tests/e2e_vpc_login.rs	Adds `create_org` calls to satisfy VPC login requirements
crates/api/tests/e2e_signature_verification.rs	Adds `setup_qwen_model` call to ensure model exists for tests
crates/api/tests/e2e_response_signature_verification.rs	Adds `setup_qwen_model` call to ensure model exists for tests
crates/api/tests/e2e_org_system_prompt.rs	Adds `setup_qwen_model` call alongside existing `setup_glm_model`
crates/api/tests/e2e_files.rs	Updates test setup calls and adds `setup_qwen_model` for tests using the model
crates/api/tests/e2e_conversations.rs	Adds `setup_qwen_model` calls to tests that use the Qwen model
crates/api/Cargo.toml	Adds `tokio-postgres` dependency for database administration operations
Cargo.lock	Records the `tokio-postgres` dependency addition
.github/workflows/test.yml	Simplifies test execution to use `cargo test -p api --tests` and adds `DATABASE_TEST_NAME` environment variable

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-08T08:07:18Z

crates/api/tests/common/db_setup.rs

+    let _ = client
+        .execute(
+            &format!(
+                "SELECT pg_terminate_backend(pid) FROM pg_stat_activity
+             WHERE datname = '{test_db_name}' AND pid <> pg_backend_pid()"
+            ),
+            &[],
+        )
+        .await;
+
+    // Drop database if exists
+    let drop_result = client
+        .execute(&format!("DROP DATABASE IF EXISTS {test_db_name}"), &[])
+        .await;
+
+    if let Err(e) = drop_result {
+        warn!("Failed to drop test database (may not exist): {}", e);
+    }
+
+    // Create fresh database
+    client
+        .execute(&format!("CREATE DATABASE {test_db_name}"), &[])
+        .await
+        .map_err(|e| format!("Failed to create test database: {e}"))?;


SQL injection vulnerability: The database name test_db_name is directly interpolated into SQL queries without proper sanitization or quoting. While there's a safety check that the name contains "test", a malicious database name like test'; DROP TABLE users; -- could bypass this check and execute arbitrary SQL.

Use parameterized queries or properly quote/escape the database name. PostgreSQL identifiers should be wrapped in double quotes and escaped. Consider using the format!() macro with proper identifier escaping or a library function to safely handle database identifiers.

think-in-universe · 2025-12-08T08:09:11Z

crates/api/tests/common/db_setup.rs

+
+/// Get test database name from environment or default
+pub fn get_test_db_name() -> String {
+    env::var("DATABASE_TEST_NAME").unwrap_or_else(|_| "platform_api_test".to_string())


Is the environment variable needed? I think it's fine to use a fixed test database name for testing.

We may rename the default test database name to cloud_api_test as the project has been renamed.

think-in-universe · 2025-12-08T08:18:01Z

crates/api/tests/e2e_vpc_login.rs

    let server = setup_test_server().await;

+    // VPC login requires user to have an organization, create one for the mock user
+    create_org(&server).await;


Is this necessary? A default org will be created for the VPC login user I think.

When this setup call is removed, the endpoint returns a 500 error with the message "No organization found" and the API logs show: "User has no organizations". Without calling create_org(), the test will fail
immediately.

is this unexpected? then probably there is a bug in either the test or the biz logic?

I think this is to be expected. We create a completely new DB. We have to create an organization for the user otherwise it is expected to fail the validation checks.

think-in-universe · 2025-12-08T08:24:35Z

crates/api/tests/e2e_conversations.rs

 #[tokio::test]
 async fn test_responses_api() {
    let server = setup_test_server().await;
+    setup_qwen_model(&server).await;


Why add setup_qwen_model() for the tests?

The test calls /v1/models API and asserts that at least one model is available (assert!(!models.data.is_empty())). Without setup_qwen_model(), no models are registered in the database, causing the endpoint to return an empty list and the test to fail immediately.

cursor

This is the final PR Bugbot will review for you during this billing cycle

Your free Bugbot reviews will reset on January 9

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

cursor · 2025-12-11T05:49:28Z

crates/api/tests/common/db_setup.rs

+
+    // Create fresh database
+    client
+        .execute(&format!("CREATE DATABASE {test_db_name}"), &[])


Bug: SQL injection vulnerability via unsanitized database name

The test_db_name value from the DATABASE_TEST_NAME environment variable is directly interpolated into SQL statements (DROP DATABASE IF EXISTS {test_db_name} and CREATE DATABASE {test_db_name}) without proper identifier quoting. The safety check test_db_name.contains("test") can be bypassed by crafting a malicious value like test"; DROP DATABASE production; --. Database identifiers in PostgreSQL need to be quoted with double quotes to prevent injection when used in DDL statements.

Additional Locations (1)

crates/api/tests/common/db_setup.rs#L77-L81

think-in-universe · 2025-12-11T09:44:02Z

@henrypark133 @PierreLeGuen please help review the PR

henrypark133 · 2025-12-11T16:49:10Z

crates/api/tests/e2e_vpc_login.rs

    let server = setup_test_server().await;

+    // VPC login requires user to have an organization, create one for the mock user
+    create_org(&server).await;


I think this is to be expected. We create a completely new DB. We have to create an organization for the user otherwise it is expected to fail the validation checks.

cursor

Bug: Missing model setup causes test to fail

The test test_conversation_items_include_model uses the model Qwen/Qwen3-30B-A3B-Instruct-2507 but does not call setup_qwen_model(&server).await; after setting up the test server. With the new separate test database, no models are pre-populated, so this test will fail because the model doesn't exist. Other similar tests in this file were updated to add the setup_qwen_model call, but this one was missed.

crates/api/tests/e2e_conversations.rs#L2616-L2618

cloud-api/crates/api/tests/e2e_conversations.rs

Lines 2616 to 2618 in c3ee363

    
           #[tokio::test] 
        
           async fn test_conversation_items_include_model() { 
        
               let server = setup_test_server().await;

PierreLeGuen · 2025-12-11T22:07:55Z

@claude review

chore: use separate database for testing

b86943b

danielwpz had a problem deploying to Cloud API test env December 4, 2025 02:22 — with GitHub Actions Failure

fix tests

917a86c

danielwpz had a problem deploying to Cloud API test env December 4, 2025 02:48 — with GitHub Actions Failure

fix tests

92cdbda

danielwpz had a problem deploying to Cloud API test env December 4, 2025 03:08 — with GitHub Actions Failure

fix ci test

59124ad

danielwpz had a problem deploying to Cloud API test env December 4, 2025 07:20 — with GitHub Actions Failure

fix repo tests

c3e1d79

danielwpz temporarily deployed to Cloud API test env December 4, 2025 08:02 — with GitHub Actions Inactive

think-in-universe requested a review from Copilot December 8, 2025 08:04

Copilot started reviewing on behalf of think-in-universe December 8, 2025 08:04 View session

Copilot AI reviewed Dec 8, 2025

View reviewed changes

think-in-universe requested changes Dec 8, 2025

View reviewed changes

Merge branch 'main' into test_db

10dd63e

think-in-universe had a problem deploying to Cloud API test env December 11, 2025 05:45 — with GitHub Actions Failure

think-in-universe requested review from PierreLeGuen and henrypark133 December 11, 2025 05:48

cursor bot reviewed Dec 11, 2025

View reviewed changes

think-in-universe added 2 commits December 11, 2025 14:13

fix: clippy

837e9eb

test: remove unused function

ec276fc

think-in-universe temporarily deployed to Cloud API test env December 11, 2025 06:16 — with GitHub Actions Inactive

think-in-universe added 2 commits December 11, 2025 17:27

chore: env vars

eca0861

test: rename test file

e1a459a

think-in-universe had a problem deploying to Cloud API test env December 11, 2025 09:29 — with GitHub Actions Failure

ci: test database name

1b7fb5e

think-in-universe temporarily deployed to Cloud API test env December 11, 2025 09:32 — with GitHub Actions Inactive

think-in-universe changed the title ~~chore: use separate database for testing~~ test: use separate database for testing Dec 11, 2025

think-in-universe approved these changes Dec 11, 2025

View reviewed changes

henrypark133 approved these changes Dec 11, 2025

View reviewed changes

Merge branch 'main' into test_db

c3ee363

henrypark133 temporarily deployed to Cloud API test env December 11, 2025 19:54 — with GitHub Actions Inactive

cursor bot reviewed Dec 11, 2025

View reviewed changes

henrypark133 merged commit a5555cb into main Dec 11, 2025
2 checks passed

	#[tokio::test]
	async fn test_conversation_items_include_model() {
	let server = setup_test_server().await;

test: use separate database for testing #227

test: use separate database for testing #227

Uh oh!

Conversation

danielwpz commented Dec 4, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

This is the final PR Bugbot will review for you during this billing cycle

Uh oh!

cursor bot Dec 11, 2025

Choose a reason for hiding this comment

Bug: SQL injection vulnerability via unsanitized database name

Uh oh!

think-in-universe commented Dec 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Bug: Missing model setup causes test to fail

Uh oh!

Uh oh!

PierreLeGuen commented Dec 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

danielwpz commented Dec 4, 2025 •

edited by cursor bot

Loading