Skip to content

Commit

Permalink
test(graphql): Remove explicit cleanup_resources (#19201)
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 due to an issue with `inotify` on macOS, 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.

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] REST API:
  • Loading branch information
amnn authored Sep 5, 2024
1 parent e1abe09 commit 1ebf5f8
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 28 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
1 change: 0 additions & 1 deletion crates/sui-graphql-rpc/tests/dot_move_e2e.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ mod tests {
test_results(internal_resolution, &v1, &v2, &v3, "internal resolution");
test_results(external_resolution, &v1, &v2, &v3, "external resolution");

network_cluster.cleanup_resources().await;
eprintln!("Tests have finished successfully now!");
}

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 1ebf5f8

Please sign in to comment.