Skip to content

Commit

Permalink
test(graphql): Remove explicit cleanup_resources
Browse files Browse the repository at this point in the history
## Description
These were originally added to try and deal with a deadlock issue, which
was actually due to an issue with `inotify` on macOS, which is fixed by
#19195.

These calls are safe to remove (also note that if the test failed, they
would never be hit, because the assert would cause a panic before they
ran).

## Test plan

Rebase on top of #19195, run the following on macOS:

```
sui$ cargo nextest run       \
  -j 1 -p sui-graphql-rpc    \
  --test e2e_tests           \
  --features pg_integration
```

This used to intermittently hang, but now succeeds.
  • Loading branch information
amnn committed Sep 4, 2024
1 parent aa9dd42 commit cf9f271
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 27 deletions.
3 changes: 0 additions & 3 deletions crates/sui-graphql-rpc/src/server/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,8 +892,6 @@ pub mod tests {
delay.as_secs_f32()
);
assert_eq!(errs, vec![exp]);

cluster.cleanup_resources().await
}

#[tokio::test]
Expand Down Expand Up @@ -1144,7 +1142,6 @@ pub mod tests {
let url_with_param = format!("{}?max_checkpoint_lag_ms=1", url);
let resp = reqwest::get(&url_with_param).await.unwrap();
assert_eq!(resp.status(), reqwest::StatusCode::GATEWAY_TIMEOUT);
cluster.cleanup_resources().await
}

/// Execute a GraphQL request with `limits` in place, expecting an error to be returned.
Expand Down
14 changes: 0 additions & 14 deletions crates/sui-graphql-rpc/src/test_infra/cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,20 +426,6 @@ impl Cluster {
pub async fn wait_for_checkpoint_pruned(&self, checkpoint: u64, base_timeout: Duration) {
wait_for_graphql_checkpoint_pruned(&self.graphql_client, checkpoint, base_timeout).await
}

/// Sends a cancellation signal to the graphql and indexer services and waits for them to
/// shutdown.
pub async fn cleanup_resources(self) {
self.network.cleanup_resources().await;
let _ = self.graphql_server_join_handle.await;
}
}

impl NetworkCluster {
pub async fn cleanup_resources(self) {
self.cancellation_token.cancel();
let _ = self.indexer_join_handle.await;
}
}

impl ExecutorCluster {
Expand Down
11 changes: 1 addition & 10 deletions crates/sui-graphql-rpc/tests/e2e_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ mod tests {
chain_id_actual
);
assert_eq!(&format!("{}", res), &exp);
cluster.cleanup_resources().await
}

#[tokio::test]
Expand Down Expand Up @@ -378,7 +377,6 @@ mod tests {
.as_str()
.unwrap();
assert_eq!(sender_read, sender.to_string());
cluster.cleanup_resources().await
}

#[tokio::test]
Expand Down Expand Up @@ -495,7 +493,6 @@ mod tests {
let binding = res.response_body().data.clone().into_json().unwrap();
let res = binding.get("verifyZkloginSignature").unwrap();
assert_eq!(res.get("success").unwrap(), false);
cluster.cleanup_resources().await
}

// TODO: add more test cases for transaction execution/dry run in transactional test runner.
Expand Down Expand Up @@ -589,7 +586,6 @@ mod tests {
.unwrap();
assert_eq!(sender_read, sender.to_string());
assert!(res.get("results").unwrap().is_array());
cluster.cleanup_resources().await
}

// Test dry run where the transaction kind is provided instead of the full transaction.
Expand Down Expand Up @@ -660,7 +656,6 @@ mod tests {
// in which case the sender is null.
assert!(sender_read.is_null());
assert!(res.get("results").unwrap().is_array());
cluster.cleanup_resources().await
}

// Test that we can handle dry run with failures at execution stage too.
Expand Down Expand Up @@ -750,8 +745,6 @@ mod tests {
.as_str()
.unwrap()
.contains("UnusedValueWithoutDrop"));

cluster.cleanup_resources().await
}

#[tokio::test]
Expand Down Expand Up @@ -793,7 +786,6 @@ mod tests {
.get("liveObjectSetDigest")
.unwrap()
.is_null());
cluster.cleanup_resources().await
}

#[tokio::test]
Expand Down Expand Up @@ -860,7 +852,6 @@ mod tests {
.await
.unwrap();

assert!(res.errors().is_empty());
cluster.cleanup_resources().await
assert!(res.errors().is_empty(), "{:#?}", res.errors());
}
}

0 comments on commit cf9f271

Please sign in to comment.