Skip to content

Commit 117e2bb

Browse files
gnd(test): Replace TempPgHandle with TestDatabase enum
Makes the two database lifecycle paths explicit and self-documenting. `TestDatabase::Temporary` vs `TestDatabase::Persistent` (--postgres-url, needs cleanup) replaces the opaque `Option<TempPgHandle>`. Cleanup in `setup_stores` is now gated on `db.needs_cleanup()` instead of running unconditionally.
1 parent e64f1ba commit 117e2bb

File tree

2 files changed

+77
-39
lines changed

2 files changed

+77
-39
lines changed

gnd/src/commands/test/runner.rs

Lines changed: 64 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
//!
33
//! This is the core of `gnd test`. For each test file, it:
44
//!
5-
//! 1. Creates a temporary PostgreSQL database (pgtemp) for complete test isolation
5+
//! 1. Creates a test database (`TestDatabase::Temporary` via pgtemp, or
6+
//! `TestDatabase::Persistent` via `--postgres-url`) for test isolation
67
//! 2. Initializes graph-node stores (entity storage, block storage, chain store)
78
//! 3. Constructs a mock Ethereum chain that feeds pre-defined blocks
89
//! 4. Deploys the subgraph and starts the indexer
@@ -38,7 +39,9 @@ use graph::blockchain::{BlockPtr, BlockchainMap, ChainIdentifier};
3839
use graph::cheap_clone::CheapClone;
3940
use graph::components::link_resolver::{ArweaveClient, FileLinkResolver};
4041
use graph::components::metrics::MetricsRegistry;
41-
use graph::components::network_provider::{ChainName, ProviderCheckStrategy, ProviderManager};
42+
use graph::components::network_provider::{
43+
AmpChainNames, ChainName, ProviderCheckStrategy, ProviderManager,
44+
};
4245
use graph::components::store::DeploymentLocator;
4346
use graph::components::subgraph::{Settings, SubgraphInstanceManager as _};
4447
use graph::data::graphql::load_manager::LoadManager;
@@ -314,16 +317,16 @@ pub async fn run_single_test(
314317
// test blocks without explicit numbers fall in the subgraph's indexed range.
315318
let blocks = build_blocks_with_triggers(test_file, manifest_info.min_start_block)?;
316319

317-
// Create a temporary database for this test. The `_temp_db` handle must
318-
// be kept alive for the duration of the test — dropping it destroys the database.
319-
let (db_url, _temp_db) = get_database_url(opt, &manifest_info.build_dir)?;
320+
// Create the database for this test. For pgtemp, the `db` value must
321+
// stay alive for the duration of the test — dropping it destroys the database.
322+
let db = create_test_database(opt, &manifest_info.build_dir)?;
320323

321324
let logger = make_test_logger(opt.verbose).new(o!("test" => test_file.name.clone()));
322325

323326
// Initialize stores with the network name from the manifest.
324327
let stores = setup_stores(
325328
&logger,
326-
&db_url,
329+
&db,
327330
&manifest_info.network_name,
328331
&manifest_info.subgraph_name,
329332
&manifest_info.hash,
@@ -403,17 +406,17 @@ pub async fn run_single_test(
403406
result
404407
}
405408

406-
/// Get a PostgreSQL connection URL for the test.
409+
/// Create the database for this test run.
407410
///
408-
/// If `--postgres-url` was provided, uses that directly.
409-
/// Otherwise, on Unix, creates a temporary database via pgtemp in the build
410-
/// directory (matching `gnd dev`'s pattern). The database is automatically
411-
/// destroyed when `TempPgHandle` is dropped.
411+
/// If `--postgres-url` was provided, returns a `Persistent` database that
412+
/// requires cleanup between tests. Otherwise, on Unix, creates a `Temporary`
413+
/// pgtemp database in the build directory (matching `gnd dev`'s pattern)
414+
/// dropped automatically when the returned value goes out of scope.
412415
///
413416
/// On non-Unix systems, `--postgres-url` is required.
414-
fn get_database_url(opt: &TestOpt, build_dir: &Path) -> Result<(String, Option<TempPgHandle>)> {
417+
fn create_test_database(opt: &TestOpt, build_dir: &Path) -> Result<TestDatabase> {
415418
if let Some(url) = &opt.postgres_url {
416-
return Ok((url.clone(), None));
419+
return Ok(TestDatabase::Persistent { url: url.clone() });
417420
}
418421

419422
#[cfg(unix)]
@@ -439,7 +442,7 @@ fn get_database_url(opt: &TestOpt, build_dir: &Path) -> Result<(String, Option<T
439442
.start();
440443

441444
let url = db.connection_uri().to_string();
442-
Ok((url, Some(TempPgHandle(db))))
445+
Ok(TestDatabase::Temporary { url, _handle: db })
443446
}
444447

445448
#[cfg(not(unix))]
@@ -451,15 +454,42 @@ fn get_database_url(opt: &TestOpt, build_dir: &Path) -> Result<(String, Option<T
451454
}
452455
}
453456

454-
/// RAII handle that keeps a pgtemp database alive for the test's duration.
457+
/// Database used for a single test run.
455458
///
456-
/// The inner `PgTempDB` is never read directly — its purpose is to prevent
457-
/// the temporary database from being destroyed until this handle is dropped.
458-
#[cfg(unix)]
459-
struct TempPgHandle(#[allow(dead_code)] pgtemp::PgTempDB);
459+
/// Encapsulates both the connection URL and the lifecycle semantics:
460+
/// - `Temporary`: pgtemp-managed, RAII — dropped when this value goes out of scope
461+
/// - `Persistent`: user-provided URL, requires explicit cleanup between tests
462+
enum TestDatabase {
463+
#[cfg(unix)]
464+
Temporary {
465+
url: String,
466+
_handle: pgtemp::PgTempDB,
467+
},
468+
Persistent {
469+
url: String,
470+
},
471+
}
460472

461-
#[cfg(not(unix))]
462-
struct TempPgHandle;
473+
impl TestDatabase {
474+
fn url(&self) -> &str {
475+
match self {
476+
#[cfg(unix)]
477+
Self::Temporary { url, .. } => url,
478+
Self::Persistent { url } => url,
479+
}
480+
}
481+
482+
/// Persistent databases accumulate state across test runs and need
483+
/// explicit cleanup (remove prior deployments, drop chains) before
484+
/// each test. Temporary databases start fresh — no cleanup needed.
485+
fn needs_cleanup(&self) -> bool {
486+
match self {
487+
#[cfg(unix)]
488+
Self::Temporary { .. } => false,
489+
Self::Persistent { .. } => true,
490+
}
491+
}
492+
}
463493

464494
/// Initialize graph-node stores from a database URL.
465495
///
@@ -472,7 +502,7 @@ struct TempPgHandle;
472502
/// error that occurs when pgtemp is dropped during cleanup.
473503
async fn setup_stores(
474504
logger: &Logger,
475-
db_url: &str,
505+
db: &TestDatabase,
476506
network_name: &ChainName,
477507
subgraph_name: &SubgraphName,
478508
hash: &DeploymentHash,
@@ -495,7 +525,7 @@ indexers = [ "default" ]
495525
[chains]
496526
ingestor = "default"
497527
"#,
498-
db_url
528+
db.url()
499529
);
500530

501531
let config = Config::from_str(&config_str, "default")
@@ -512,14 +542,17 @@ ingestor = "default"
512542
let network_identifiers: Vec<ChainName> = vec![network_name.clone()];
513543
let network_store = store_builder.network_store(network_identifiers).await;
514544

515-
// Clean up any leftover state from a previous run on this persistent database.
516-
// Order matters: deployments must be removed before the chain can be dropped,
517-
// because deployment_schemas has a FK constraint on the chains table.
518-
let subgraph_store = network_store.subgraph_store();
519-
cleanup(&subgraph_store, subgraph_name, hash).await.ok();
520-
545+
// Persistent databases accumulate state across test runs and need cleanup.
546+
// Temporary (pgtemp) databases start fresh — no cleanup needed.
521547
let block_store = network_store.block_store();
522-
let _ = block_store.drop_chain(network_name).await;
548+
if db.needs_cleanup() {
549+
// Order matters: deployments must be removed before the chain can be dropped,
550+
// because deployment_schemas has a FK constraint on the chains table.
551+
let subgraph_store = network_store.subgraph_store();
552+
cleanup(&subgraph_store, subgraph_name, hash).await.ok();
553+
554+
let _ = block_store.drop_chain(network_name).await;
555+
}
523556

524557
// Synthetic chain identifier — net_version "1" with zero genesis hash.
525558
let ident = ChainIdentifier {
@@ -767,6 +800,7 @@ async fn setup_context(
767800
node_id.clone(),
768801
SubgraphVersionSwitchingMode::Instant,
769802
Arc::new(Settings::default()),
803+
Arc::new(AmpChainNames::default()),
770804
));
771805

772806
// Register the subgraph name (e.g., "test/TransferCreatesEntity").

gnd/tests/gnd_test.rs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,22 @@
2121
//! ```bash
2222
//! just test-gnd-test
2323
//! ```
24+
//!
25+
//! Tests run with `--test-threads=1` to avoid races when sharing a Postgres
26+
//! instance via `--postgres-url` (CI). With pgtemp (default) each test gets
27+
//! its own isolated database, but serial execution keeps things simple.
2428
2529
use std::fs;
2630
use std::path::{Path, PathBuf};
2731
use std::process::Command;
28-
use std::sync::LazyLock;
2932

3033
use tempfile::TempDir;
3134
use walkdir::WalkDir;
3235

33-
/// Shared fixture: copied once, npm-installed once, codegen'd once.
34-
/// The `TempDir` is kept alive for the entire test binary lifetime.
35-
static FIXTURE: LazyLock<(TempDir, PathBuf)> = LazyLock::new(|| {
36+
/// Copy the fixture subgraph into a fresh temp directory, install npm
37+
/// dependencies, and run `gnd codegen`. Returns the temp dir handle (to
38+
/// keep it alive) and the path to the prepared subgraph directory.
39+
fn setup_fixture() -> (TempDir, PathBuf) {
3640
let temp_dir = TempDir::new().expect("Failed to create temp directory");
3741
let subgraph_dir = temp_dir.path().join("subgraph");
3842
fs::create_dir_all(&subgraph_dir).unwrap();
@@ -77,7 +81,7 @@ static FIXTURE: LazyLock<(TempDir, PathBuf)> = LazyLock::new(|| {
7781
);
7882

7983
(temp_dir, subgraph_dir)
80-
});
84+
}
8185

8286
/// Get the path to the gnd binary.
8387
fn gnd_binary_path() -> PathBuf {
@@ -173,7 +177,7 @@ fn run_gnd_test(args: &[&str], cwd: &Path) -> std::process::Output {
173177

174178
#[test]
175179
fn test_gnd_test_all() {
176-
let subgraph_dir = &FIXTURE.1;
180+
let (_temp_dir, subgraph_dir) = setup_fixture();
177181

178182
// Run only the passing test files (exclude failing.json which is used by the negative test).
179183
let output = run_gnd_test(
@@ -182,7 +186,7 @@ fn test_gnd_test_all() {
182186
"tests/blocks.json",
183187
"tests/templates.json",
184188
],
185-
subgraph_dir,
189+
&subgraph_dir,
186190
);
187191

188192
assert!(
@@ -200,9 +204,9 @@ fn test_gnd_test_all() {
200204

201205
#[test]
202206
fn test_gnd_test_failing_assertions() {
203-
let subgraph_dir = &FIXTURE.1;
207+
let (_temp_dir, subgraph_dir) = setup_fixture();
204208

205-
let output = run_gnd_test(&["tests/failing.json"], subgraph_dir);
209+
let output = run_gnd_test(&["tests/failing.json"], &subgraph_dir);
206210

207211
assert!(
208212
!output.status.success(),

0 commit comments

Comments
 (0)