gnd(test): Address PR review comments

- Add -s short flag for --skip-build
- Print deprecation warning when --matchstick is used; update README
- Replace npm with pnpm in integration test setup
- Replace hand-rolled r_value_to_json with serde_json::to_value via
  r::Value's built-in Serialize impl (also fixes Timestamp serialization)
- Make find_asc_binary pub and re-export from compiler module; reuse it
  in gnd_test.rs instead of duplicating the lookup logic
- Skip DB cleanup on test failure for persistent databases and print the
  connection URL so the data can be inspected

Author: dimitrovmaksim

gnd/README.md

@@ -315,9 +315,9 @@ gnd test [TEST_FILES...]
 | Flag | Short | Description |
 |------|-------|-------------|
 | `--manifest` | `-m` | Path to subgraph manifest (default: `subgraph.yaml`) |
-| `--skip-build` | | Skip building the subgraph before testing |
+| `--skip-build` | `-s` | Skip building the subgraph before testing |
 | `--postgres-url` | | PostgreSQL connection URL (env: `POSTGRES_URL`) |
-| `--matchstick` | | Use legacy Matchstick runner |
+| `--matchstick` | | Use legacy Matchstick runner (**deprecated** — migrate to JSON-based tests) |
 | `--docker` | `-d` | Run Matchstick in Docker (requires `--matchstick`) |
 | `--coverage` | `-c` | Run with coverage reporting (requires `--matchstick`) |
 | `--recompile` | `-r` | Force recompilation (requires `--matchstick`) |
@@ -340,7 +340,7 @@ gnd test my-tests/
 gnd test -m subgraph.staging.yaml tests/transfer.json
 
 # Skip automatic build
-gnd test --skip-build
+gnd test -s
 ```
 
 ### `gnd clean`

gnd/src/commands/test/assertion.rs

@@ -4,7 +4,7 @@ use super::runner::TestContext;
 use super::schema::{Assertion, AssertionFailure, AssertionOutcome, TestResult};
 use anyhow::{anyhow, Result};
 use graph::data::query::{Query, QueryResults, QueryTarget};
-use graph::prelude::{q, r, ApiVersion, GraphQlRunner as GraphQlRunnerTrait};
+use graph::prelude::{q, ApiVersion, GraphQlRunner as GraphQlRunnerTrait};
 
 pub(super) async fn run_assertions(
     ctx: &TestContext,
@@ -62,7 +62,7 @@ async fn run_single_assertion(
         .map_err(|errors| anyhow!("Query errors: {:?}", errors))?;
 
     let actual_json = match result {
-        Some(value) => r_value_to_json(&value),
+        Some(value) => serde_json::to_value(&value)?,
         None => serde_json::Value::Null,
     };
 
@@ -77,32 +77,6 @@ async fn run_single_assertion(
     }
 }
 
-/// Convert graph-node's internal `r::Value` (GraphQL result) to `serde_json::Value`.
-///
-/// Graph-node uses its own value type for GraphQL results. This converts to
-/// standard JSON for comparison with the expected values in the test file.
-fn r_value_to_json(value: &r::Value) -> serde_json::Value {
-    match value {
-        r::Value::Null => serde_json::Value::Null,
-        r::Value::Boolean(b) => serde_json::Value::Bool(*b),
-        r::Value::Int(n) => serde_json::Value::Number((*n).into()),
-        r::Value::Float(f) => serde_json::json!(*f),
-        r::Value::String(s) => serde_json::Value::String(s.clone()),
-        r::Value::Enum(s) => serde_json::Value::String(s.clone()),
-        r::Value::List(list) => {
-            serde_json::Value::Array(list.iter().map(r_value_to_json).collect())
-        }
-        r::Value::Object(obj) => {
-            let map: serde_json::Map<String, serde_json::Value> = obj
-                .iter()
-                .map(|(k, v)| (k.to_string(), r_value_to_json(v)))
-                .collect();
-            serde_json::Value::Object(map)
-        }
-        r::Value::Timestamp(t) => serde_json::Value::String(t.to_string()),
-    }
-}
-
 /// Reorder `actual` arrays to align with `expected`'s element ordering.
 ///
 /// When a test fails, the raw diff can be misleading if array elements appear

gnd/src/commands/test/mod.rs

@@ -59,14 +59,14 @@ pub struct TestOpt {
     pub manifest: PathBuf,
 
     /// Skip building the subgraph before testing
-    #[clap(long)]
+    #[clap(short = 's', long)]
     pub skip_build: bool,
 
     /// PostgreSQL connection URL. If not provided, a temporary database will be created (Unix only).
     #[clap(long, env = "POSTGRES_URL")]
     pub postgres_url: Option<String>,
 
-    /// Use Matchstick runner instead (legacy mode)
+    /// Use Matchstick runner instead (deprecated legacy mode — migrate to JSON-based tests)
     #[clap(long)]
     pub matchstick: bool,
 
@@ -102,6 +102,11 @@ pub struct TestOpt {
 
 pub async fn run_test(opt: TestOpt) -> Result<()> {
     if opt.matchstick {
+        step(
+            Step::Warn,
+            "Matchstick is deprecated and will be removed in a future release. \
+            Migrate to the JSON-based test format (see: gnd/docs/gnd-test.md).",
+        );
         return matchstick::run(&opt).await;
     }
 

gnd/src/commands/test/runner.rs

@@ -310,17 +310,25 @@ pub async fn run_single_test(
         .await;
 
     // For persistent databases, clean up the deployment after the test so the
-    // database is left in a clean state. Each run generates a unique hash and
-    // subgraph name (via the seed), so pre-test cleanup is not needed — only
-    // post-test cleanup of the current run's deployment.
+    // database is left in a clean state. On failure, skip cleanup so the data
+    // is preserved for inspection.
     if db.needs_cleanup() {
-        cleanup(
-            &ctx.store,
-            &manifest_info.subgraph_name,
-            &manifest_info.hash,
-        )
-        .await
-        .ok();
+        let test_passed = result.as_ref().map(|r| r.is_passed()).unwrap_or(false);
+        if test_passed {
+            cleanup(
+                &ctx.store,
+                &manifest_info.subgraph_name,
+                &manifest_info.hash,
+            )
+            .await
+            .ok();
+        } else {
+            eprintln!(
+                "  {} Test failed — database preserved for inspection: {}",
+                console::style("ℹ").cyan(),
+                db.url()
+            );
+        }
     }
 
     result

gnd/src/compiler/asc.rs

@@ -181,7 +181,7 @@ pub fn find_graph_ts(source_dir: &Path) -> Result<(Vec<PathBuf>, PathBuf)> {
 
 /// Find the `asc` binary by checking the global PATH first, then the project's
 /// root `node_modules/.bin/asc`.
-fn find_asc_binary(base_dir: &Path) -> Option<PathBuf> {
+pub fn find_asc_binary(base_dir: &Path) -> Option<PathBuf> {
     // Check global PATH first
     if Command::new("asc")
         .arg("--version")

gnd/src/compiler/mod.rs

@@ -5,4 +5,4 @@
 
 mod asc;
 
-pub use asc::{compile_mapping, find_graph_ts, AscCompileOptions};
+pub use asc::{compile_mapping, find_asc_binary, find_graph_ts, AscCompileOptions};

gnd/tests/gnd_test.rs

@@ -14,7 +14,7 @@
 //!
 //! - Build the gnd binary: `cargo build -p gnd`
 //! - AssemblyScript compiler (`asc`) in PATH
-//! - npm available for dependency installation
+//! - pnpm available for dependency installation
 //!
 //! # Running
 //!
@@ -33,7 +33,7 @@ use std::process::Command;
 use tempfile::TempDir;
 use walkdir::WalkDir;
 
-/// Copy the fixture subgraph into a fresh temp directory, install npm
+/// Copy the fixture subgraph into a fresh temp directory, install pnpm
 /// dependencies, and run `gnd codegen`. Returns the temp dir handle (to
 /// keep it alive) and the path to the prepared subgraph directory.
 fn setup_fixture() -> (TempDir, PathBuf) {
@@ -50,16 +50,16 @@ fn setup_fixture() -> (TempDir, PathBuf) {
 
     copy_dir_recursive(&fixture, &subgraph_dir).expect("Failed to copy fixture to temp directory");
 
-    // Install npm dependencies (graph-ts, graph-cli)
-    let npm_output = Command::new("npm")
+    // Install dependencies (graph-ts, graph-cli)
+    let npm_output = Command::new("pnpm")
         .arg("install")
         .current_dir(&subgraph_dir)
         .output()
-        .expect("Failed to run `npm install`. Is npm available?");
+        .expect("Failed to run `pnpm install`. Is pnpm available?");
 
     assert!(
         npm_output.status.success(),
-        "npm install failed in fixture:\nstdout: {}\nstderr: {}",
+        "pnpm install failed in fixture:\nstdout: {}\nstderr: {}",
         String::from_utf8_lossy(&npm_output.stdout),
         String::from_utf8_lossy(&npm_output.stderr),
     );
@@ -116,23 +116,11 @@ fn fixture_path() -> PathBuf {
 
 /// Assert that `asc` (AssemblyScript compiler) is available in PATH or in local node_modules.
 fn verify_asc_available(subgraph_dir: &Path) {
-    // Check global PATH first
-    if Command::new("asc")
-        .arg("--version")
-        .output()
-        .map(|o| o.status.success())
-        .unwrap_or(false)
-    {
-        return;
-    }
-
-    // Fall back to local node_modules/.bin/asc
-    let local_asc = subgraph_dir.join("node_modules").join(".bin").join("asc");
     assert!(
-        local_asc.exists(),
-        "asc compiler not found globally or at {}. \
+        gnd::compiler::find_asc_binary(subgraph_dir).is_some(),
+        "asc compiler not found globally or in {}/node_modules/.bin. \
          Install it with: npm install -g assemblyscript@0.19.23",
-        local_asc.display()
+        subgraph_dir.display()
     );
 }