Refactor subgraph store tests (#3662)

* refactor: Rename deployment's has_non_fatal_errors to has_deterministic_errors

* store: Pass latest_ethereum_block_number in subgraph tests

* refactor: Remove unused Option from has_non_fatal_errors

* refactor: Create helper 'latest_block' fn in store::subgraph tests

* refactor: Rename missing 'non_fatal_errors' into 'has_deterministic_errors'

graph/src/components/store/traits.rs

@@ -401,8 +401,7 @@ pub trait QueryStore: Send + Sync {
 
     fn wait_stats(&self) -> Result<PoolWaitStats, StoreError>;
 
-    /// If `block` is `None`, assumes the latest block.
-    async fn has_non_fatal_errors(&self, block: Option<BlockNumber>) -> Result<bool, StoreError>;
+    async fn has_deterministic_errors(&self, block: BlockNumber) -> Result<bool, StoreError>;
 
     /// Find the current state for the subgraph deployment `id` and
     /// return details about it needed for executing queries

graphql/src/store/resolver.rs

@@ -79,7 +79,7 @@ impl StoreResolver {
         let block_ptr = Self::locate_block(store_clone.as_ref(), bc, state).await?;
 
         let has_non_fatal_errors = store
-            .has_non_fatal_errors(Some(block_ptr.block_number()))
+            .has_deterministic_errors(block_ptr.block_number())
             .await?;
 
         let resolver = StoreResolver {

store/postgres/src/deployment.rs

@@ -652,39 +652,19 @@ pub fn fail(
 }
 
 /// If `block` is `None`, assumes the latest block.
-pub(crate) fn has_non_fatal_errors(
+pub(crate) fn has_deterministic_errors(
     conn: &PgConnection,
     id: &DeploymentHash,
-    block: Option<BlockNumber>,
+    block: BlockNumber,
 ) -> Result<bool, StoreError> {
-    use subgraph_deployment as d;
     use subgraph_error as e;
-
-    match block {
-        Some(block) => select(diesel::dsl::exists(
-            e::table
-                .filter(e::subgraph_id.eq(id.as_str()))
-                .filter(e::deterministic)
-                .filter(sql("block_range @> ").bind::<Integer, _>(block)),
-        ))
-        .get_result(conn),
-        None => select(diesel::dsl::exists(
-            e::table
-                .filter(e::subgraph_id.eq(id.as_str()))
-                .filter(e::deterministic)
-                .filter(
-                    sql("block_range @> ")
-                        .bind(
-                            d::table
-                                .filter(d::deployment.eq(id.as_str()))
-                                .select(d::latest_ethereum_block_number)
-                                .single_value(),
-                        )
-                        .sql("::int"),
-                ),
-        ))
-        .get_result(conn),
-    }
+    select(diesel::dsl::exists(
+        e::table
+            .filter(e::subgraph_id.eq(id.as_str()))
+            .filter(e::deterministic)
+            .filter(sql("block_range @> ").bind::<Integer, _>(block)),
+    ))
+    .get_result(conn)
     .map_err(|e| e.into())
 }
 
@@ -732,15 +712,15 @@ pub(crate) fn error_count(conn: &PgConnection, id: &DeploymentHash) -> Result<us
 }
 
 /// Checks if the subgraph is healthy or unhealthy as of the given block, or the subgraph latest
-/// block if `None`, based on the presence of non-fatal errors. Has no effect on failed subgraphs.
+/// block if `None`, based on the presence of deterministic errors. Has no effect on failed subgraphs.
 fn check_health(
     conn: &PgConnection,
     id: &DeploymentHash,
     block: BlockNumber,
 ) -> Result<(), StoreError> {
     use subgraph_deployment as d;
 
-    let has_errors = has_non_fatal_errors(conn, id, Some(block))?;
+    let has_errors = has_deterministic_errors(conn, id, block)?;
 
     let (new, old) = match has_errors {
         true => (SubgraphHealth::Unhealthy, SubgraphHealth::Healthy),

store/postgres/src/deployment_store.rs

@@ -1337,7 +1337,7 @@ impl DeploymentStore {
             use deployment::SubgraphHealth::*;
             // Decide status based on if there are any errors for the previous/parent block
             let prev_health =
-                if deployment::has_non_fatal_errors(conn, deployment_id, Some(parent_ptr.number))? {
+                if deployment::has_deterministic_errors(conn, deployment_id, parent_ptr.number)? {
                     Unhealthy
                 } else {
                     Healthy

store/postgres/src/query_store.rs

@@ -85,11 +85,11 @@ impl QueryStoreTrait for QueryStore {
         self.store.wait_stats(self.replica_id)
     }
 
-    async fn has_non_fatal_errors(&self, block: Option<BlockNumber>) -> Result<bool, StoreError> {
+    async fn has_deterministic_errors(&self, block: BlockNumber) -> Result<bool, StoreError> {
         let id = self.site.deployment.clone();
         self.store
             .with_conn(move |conn, _| {
-                crate::deployment::has_non_fatal_errors(conn, &id, block).map_err(|e| e.into())
+                crate::deployment::has_deterministic_errors(conn, &id, block).map_err(|e| e.into())
             })
             .await
     }

store/postgres/tests/subgraph.rs

@@ -1,10 +1,11 @@
 use graph::{
     components::{
         server::index_node::VersionInfo,
-        store::{DeploymentLocator, StatusStore},
+        store::{DeploymentId, DeploymentLocator, StatusStore},
     },
     data::subgraph::schema::SubgraphHealth,
     data::subgraph::schema::{DeploymentCreate, SubgraphError},
+    prelude::BlockPtr,
     prelude::EntityChange,
     prelude::EntityChangeOperation,
     prelude::QueryStoreManager,
@@ -51,6 +52,17 @@ fn get_version_info(store: &Store, subgraph_name: &str) -> VersionInfo {
     store.version_info(&current).unwrap()
 }
 
+async fn latest_block(store: &Store, deployment_id: DeploymentId) -> BlockPtr {
+    store
+        .subgraph_store()
+        .writable(LOGGER.clone(), deployment_id)
+        .await
+        .expect("can get writable")
+        .block_ptr()
+        .await
+        .unwrap()
+}
+
 #[test]
 fn reassign_subgraph() {
     async fn setup() -> DeploymentLocator {
@@ -544,6 +556,16 @@ fn fatal_vs_non_fatal() {
             .await
             .unwrap();
 
+        // Just to make latest_ethereum_block_number be 0
+        transact_and_wait(
+            &store.subgraph_store(),
+            &deployment,
+            BLOCKS[0].clone(),
+            vec![],
+        )
+        .await
+        .unwrap();
+
         let error = || SubgraphError {
             subgraph_id: deployment.hash.clone(),
             message: "test".to_string(),
@@ -561,13 +583,19 @@ fn fatal_vs_non_fatal() {
             .await
             .unwrap();
 
-        assert!(!query_store.has_non_fatal_errors(None).await.unwrap());
+        assert!(!query_store
+            .has_deterministic_errors(latest_block(&store, deployment.id).await.number)
+            .await
+            .unwrap());
 
         transact_errors(&store, &deployment, BLOCKS[1].clone(), vec![error()])
             .await
             .unwrap();
 
-        assert!(query_store.has_non_fatal_errors(None).await.unwrap());
+        assert!(query_store
+            .has_deterministic_errors(latest_block(&store, deployment.id).await.number)
+            .await
+            .unwrap());
     })
 }
 
@@ -600,7 +628,10 @@ fn fail_unfail_deterministic_error() {
         .unwrap();
 
         // We don't have any errors and the subgraph is healthy.
-        assert!(!query_store.has_non_fatal_errors(None).await.unwrap());
+        assert!(!query_store
+            .has_deterministic_errors(latest_block(&store, deployment.id).await.number)
+            .await
+            .unwrap());
         let vi = get_version_info(&store, NAME);
         assert_eq!(&*NAME, vi.deployment_id.as_str());
         assert_eq!(false, vi.failed);
@@ -617,7 +648,10 @@ fn fail_unfail_deterministic_error() {
         .unwrap();
 
         // Still no fatal errors.
-        assert!(!query_store.has_non_fatal_errors(None).await.unwrap());
+        assert!(!query_store
+            .has_deterministic_errors(latest_block(&store, deployment.id).await.number)
+            .await
+            .unwrap());
         let vi = get_version_info(&store, NAME);
         assert_eq!(&*NAME, vi.deployment_id.as_str());
         assert_eq!(false, vi.failed);
@@ -641,7 +675,10 @@ fn fail_unfail_deterministic_error() {
         writable.fail_subgraph(error).await.unwrap();
 
         // Now we have a fatal error because the subgraph failed.
-        assert!(query_store.has_non_fatal_errors(None).await.unwrap());
+        assert!(query_store
+            .has_deterministic_errors(latest_block(&store, deployment.id).await.number)
+            .await
+            .unwrap());
         let vi = get_version_info(&store, NAME);
         assert_eq!(&*NAME, vi.deployment_id.as_str());
         assert_eq!(true, vi.failed);
@@ -655,7 +692,10 @@ fn fail_unfail_deterministic_error() {
 
         // We don't have fatal errors anymore and the block got reverted.
         assert_eq!(outcome, UnfailOutcome::Unfailed);
-        assert!(!query_store.has_non_fatal_errors(None).await.unwrap());
+        assert!(!query_store
+            .has_deterministic_errors(latest_block(&store, deployment.id).await.number)
+            .await
+            .unwrap());
         let vi = get_version_info(&store, NAME);
         assert_eq!(&*NAME, vi.deployment_id.as_str());
         assert_eq!(false, vi.failed);