From 8af105f69e64d69fa88717a5c934a123552f9943 Mon Sep 17 00:00:00 2001 From: Will Yang Date: Thu, 21 Nov 2024 12:26:26 -0800 Subject: [PATCH 01/12] split out functionality to initialize an adapter from the rest of the task commands --- .../src/framework.rs | 67 +++++++++++++++---- 1 file changed, 54 insertions(+), 13 deletions(-) diff --git a/external-crates/move/crates/move-transactional-test-runner/src/framework.rs b/external-crates/move/crates/move-transactional-test-runner/src/framework.rs index 673845432f165..bc4a83b5d6015 100644 --- a/external-crates/move/crates/move-transactional-test-runner/src/framework.rs +++ b/external-crates/move/crates/move-transactional-test-runner/src/framework.rs @@ -704,10 +704,12 @@ pub fn compile_ir_module( .into_compiled_module(&code) } -pub async fn handle_actual_output<'a, Adapter>( +/// Creates an adapter for the given tasks, using the first task command to initialize the adapter +/// if it is a `TaskCommand::Init`. +pub async fn create_adapter<'a, Adapter>( path: &Path, fully_compiled_program_opt: Option>, -) -> Result<(String, Adapter), Box> +) -> Result> where Adapter: MoveTestAdapter<'a>, Adapter::ExtraInitArgs: Debug, @@ -736,14 +738,6 @@ where .into_iter() .collect::>(); assert!(!tasks.is_empty()); - let num_tasks = tasks.len(); - writeln!( - &mut output, - "processed {} task{}", - num_tasks, - if num_tasks > 1 { "s" } else { "" } - ) - .unwrap(); let first_task = tasks.pop_front().unwrap(); let init_opt = match &first_task.command { @@ -756,16 +750,62 @@ where None } }; + let (mut adapter, result_opt) = Adapter::init(default_syntax, fully_compiled_program_opt, init_opt, path).await; + if let Some(result) = result_opt { if let Err(e) = writeln!(output, "\ninit:\n{}", result) { - // TODO: if this fails, it masks the actual error, need better error handling - // in case cleanup_resources() fails + // TODO: if this fails, it masks the actual error, need better error handling in case + // cleanup_resources() fails adapter.cleanup_resources().await?; return Err(Box::new(e)); } } + + Ok(adapter) +} + +/// Consumes the adapter to run tasks from path. +pub async fn run_tasks_with_adapter<'a, Adapter>( + path: &Path, + mut adapter: Adapter, +) -> Result<(String, Adapter), Box> +where + Adapter: MoveTestAdapter<'a>, + Adapter::ExtraInitArgs: Debug, + Adapter::ExtraPublishArgs: Debug, + Adapter::ExtraValueArgs: Debug, + Adapter::ExtraRunArgs: Debug, + Adapter::Subcommand: Debug, +{ + let mut output = String::new(); + let mut tasks = taskify::< + TaskCommand< + Adapter::ExtraInitArgs, + Adapter::ExtraPublishArgs, + Adapter::ExtraValueArgs, + Adapter::ExtraRunArgs, + Adapter::Subcommand, + >, + >(path)? + .into_iter() + .collect::>(); + assert!(!tasks.is_empty()); + let num_tasks = tasks.len(); + writeln!( + &mut output, + "processed {} task{}", + num_tasks, + if num_tasks > 1 { "s" } else { "" } + ) + .unwrap(); + + // Pop off init command if present + if let Some(TaskCommand::Init(_, _)) = tasks.front().map(|t| &t.command) { + tasks.pop_front(); + } + for task in tasks { handle_known_task(&mut output, &mut adapter, task).await; } @@ -785,7 +825,8 @@ where Adapter::ExtraRunArgs: Debug, Adapter::Subcommand: Debug, { - let output = handle_actual_output::(path, fully_compiled_program_opt).await?; + let adapter = create_adapter::(path, fully_compiled_program_opt).await?; + let output = run_tasks_with_adapter(path, adapter).await?; handle_expected_output(path, output.0)?; Ok(()) } From 698f3c42508f64ee4fb4139ae01066afc5ae0026 Mon Sep 17 00:00:00 2001 From: Will Yang Date: Thu, 21 Nov 2024 12:30:55 -0800 Subject: [PATCH 02/12] move handle_expected_output into run_tasks_with_adapter --- .../move-transactional-test-runner/src/framework.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/external-crates/move/crates/move-transactional-test-runner/src/framework.rs b/external-crates/move/crates/move-transactional-test-runner/src/framework.rs index bc4a83b5d6015..64bfe6c36b159 100644 --- a/external-crates/move/crates/move-transactional-test-runner/src/framework.rs +++ b/external-crates/move/crates/move-transactional-test-runner/src/framework.rs @@ -767,10 +767,7 @@ where } /// Consumes the adapter to run tasks from path. -pub async fn run_tasks_with_adapter<'a, Adapter>( - path: &Path, - mut adapter: Adapter, -) -> Result<(String, Adapter), Box> +pub async fn run_tasks_with_adapter<'a, Adapter>(path: &Path, mut adapter: Adapter) -> Result<()> where Adapter: MoveTestAdapter<'a>, Adapter::ExtraInitArgs: Debug, @@ -810,7 +807,9 @@ where handle_known_task(&mut output, &mut adapter, task).await; } adapter.cleanup_resources().await?; - Ok((output, adapter)) + + handle_expected_output(path, output)?; + Ok(()) } pub async fn run_test_impl<'a, Adapter>( @@ -826,8 +825,7 @@ where Adapter::Subcommand: Debug, { let adapter = create_adapter::(path, fully_compiled_program_opt).await?; - let output = run_tasks_with_adapter(path, adapter).await?; - handle_expected_output(path, output.0)?; + run_tasks_with_adapter(path, adapter).await?; Ok(()) } From 395cb829cb23559af29d8612bcfaebf762f07fbc Mon Sep 17 00:00:00 2001 From: Will Yang Date: Thu, 21 Nov 2024 12:50:23 -0800 Subject: [PATCH 03/12] change where we write to output around slightly to match original output logic --- .../src/framework.rs | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/external-crates/move/crates/move-transactional-test-runner/src/framework.rs b/external-crates/move/crates/move-transactional-test-runner/src/framework.rs index 64bfe6c36b159..df280f9ac3074 100644 --- a/external-crates/move/crates/move-transactional-test-runner/src/framework.rs +++ b/external-crates/move/crates/move-transactional-test-runner/src/framework.rs @@ -705,11 +705,11 @@ pub fn compile_ir_module( } /// Creates an adapter for the given tasks, using the first task command to initialize the adapter -/// if it is a `TaskCommand::Init`. +/// if it is a `TaskCommand::Init`. Returns the adapter and the output string. pub async fn create_adapter<'a, Adapter>( path: &Path, fully_compiled_program_opt: Option>, -) -> Result> +) -> Result<(String, Adapter), Box> where Adapter: MoveTestAdapter<'a>, Adapter::ExtraInitArgs: Debug, @@ -738,6 +738,14 @@ where .into_iter() .collect::>(); assert!(!tasks.is_empty()); + let num_tasks = tasks.len(); + writeln!( + &mut output, + "processed {} task{}", + num_tasks, + if num_tasks > 1 { "s" } else { "" } + ) + .unwrap(); let first_task = tasks.pop_front().unwrap(); let init_opt = match &first_task.command { @@ -763,11 +771,15 @@ where } } - Ok(adapter) + Ok((output, adapter)) } /// Consumes the adapter to run tasks from path. -pub async fn run_tasks_with_adapter<'a, Adapter>(path: &Path, mut adapter: Adapter) -> Result<()> +pub async fn run_tasks_with_adapter<'a, Adapter>( + path: &Path, + mut adapter: Adapter, + mut output: String, +) -> Result<()> where Adapter: MoveTestAdapter<'a>, Adapter::ExtraInitArgs: Debug, @@ -776,7 +788,6 @@ where Adapter::ExtraRunArgs: Debug, Adapter::Subcommand: Debug, { - let mut output = String::new(); let mut tasks = taskify::< TaskCommand< Adapter::ExtraInitArgs, @@ -789,14 +800,6 @@ where .into_iter() .collect::>(); assert!(!tasks.is_empty()); - let num_tasks = tasks.len(); - writeln!( - &mut output, - "processed {} task{}", - num_tasks, - if num_tasks > 1 { "s" } else { "" } - ) - .unwrap(); // Pop off init command if present if let Some(TaskCommand::Init(_, _)) = tasks.front().map(|t| &t.command) { @@ -824,8 +827,8 @@ where Adapter::ExtraRunArgs: Debug, Adapter::Subcommand: Debug, { - let adapter = create_adapter::(path, fully_compiled_program_opt).await?; - run_tasks_with_adapter(path, adapter).await?; + let (output, adapter) = create_adapter::(path, fully_compiled_program_opt).await?; + run_tasks_with_adapter(path, adapter, output).await?; Ok(()) } From a2b4ffc2a32f7ffaf86c10fc639b7b7a6e793a80 Mon Sep 17 00:00:00 2001 From: Will Yang Date: Thu, 21 Nov 2024 12:50:37 -0800 Subject: [PATCH 04/12] expose move-transactional-test-runner fns on sui-transactional-test-runner --- crates/sui-transactional-test-runner/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/sui-transactional-test-runner/src/lib.rs b/crates/sui-transactional-test-runner/src/lib.rs index 1c9453725bbc2..3e041e68b4f1f 100644 --- a/crates/sui-transactional-test-runner/src/lib.rs +++ b/crates/sui-transactional-test-runner/src/lib.rs @@ -8,7 +8,9 @@ pub mod programmable_transaction_test_parser; mod simulator_persisted_store; pub mod test_adapter; -pub use move_transactional_test_runner::framework::run_test_impl; +pub use move_transactional_test_runner::framework::{ + create_adapter, run_tasks_with_adapter, run_test_impl, +}; use rand::rngs::StdRng; use simulacrum::Simulacrum; use simulacrum::SimulatorStore; From 078863e8b94381967162b4a133e9c382bf55c320 Mon Sep 17 00:00:00 2001 From: Will Yang Date: Thu, 21 Nov 2024 12:51:31 -0800 Subject: [PATCH 05/12] use new fns instead of run_test_impl, add doc comment explaining that the latter is a convenience fn --- crates/sui-graphql-e2e-tests/tests/tests.rs | 6 ++++-- .../crates/move-transactional-test-runner/src/framework.rs | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/sui-graphql-e2e-tests/tests/tests.rs b/crates/sui-graphql-e2e-tests/tests/tests.rs index 812d9058fa7a7..0218f26af9e09 100644 --- a/crates/sui-graphql-e2e-tests/tests/tests.rs +++ b/crates/sui-graphql-e2e-tests/tests/tests.rs @@ -5,7 +5,7 @@ #![allow(unused_variables)] use std::{path::Path, sync::Arc}; use sui_transactional_test_runner::{ - run_test_impl, + create_adapter, run_tasks_with_adapter, test_adapter::{SuiTestAdapter, PRE_COMPILED}, }; @@ -24,7 +24,9 @@ datatest_stable::harness!( async fn run_test(path: &Path) -> Result<(), Box> { telemetry_subscribers::init_for_testing(); if !cfg!(msim) { - run_test_impl::(path, Some(Arc::new(PRE_COMPILED.clone()))).await?; + let (output, adapter) = + create_adapter::(path, Some(Arc::new(PRE_COMPILED.clone()))).await?; + run_tasks_with_adapter(path, adapter, output).await?; } Ok(()) } diff --git a/external-crates/move/crates/move-transactional-test-runner/src/framework.rs b/external-crates/move/crates/move-transactional-test-runner/src/framework.rs index df280f9ac3074..e3b26527e8415 100644 --- a/external-crates/move/crates/move-transactional-test-runner/src/framework.rs +++ b/external-crates/move/crates/move-transactional-test-runner/src/framework.rs @@ -815,6 +815,8 @@ where Ok(()) } +/// Convenience function that creates an adapter and runs the tasks, to be used when a caller does +/// not need to extend the adapter. pub async fn run_test_impl<'a, Adapter>( path: &Path, fully_compiled_program_opt: Option>, From ad75463211df7d2275c3c107c9a9a0bdc863e0ce Mon Sep 17 00:00:00 2001 From: Will Yang Date: Thu, 21 Nov 2024 13:41:55 -0800 Subject: [PATCH 06/12] accept a graphql url, wrap the SimpleClient over it. in the future we may consider making this generic as well.. --- .../src/test_adapter.rs | 52 ++++++++++++------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/crates/sui-transactional-test-runner/src/test_adapter.rs b/crates/sui-transactional-test-runner/src/test_adapter.rs index 2e1bff08dbb06..2c9801ccb9460 100644 --- a/crates/sui-transactional-test-runner/src/test_adapter.rs +++ b/crates/sui-transactional-test-runner/src/test_adapter.rs @@ -48,6 +48,7 @@ use std::{ use sui_core::authority::test_authority_builder::TestAuthorityBuilder; use sui_core::authority::AuthorityState; use sui_framework::DEFAULT_FRAMEWORK_PATH; +use sui_graphql_rpc::client::simple_client::SimpleClient; use sui_graphql_rpc::test_infra::cluster::ExecutorCluster; use sui_graphql_rpc::test_infra::cluster::{serve_executor, RetentionConfig, SnapshotLagConfig}; use sui_json_rpc_api::QUERY_MAX_RESULT_LIMIT; @@ -136,7 +137,9 @@ pub struct SuiTestAdapter { gas_price: u64, pub(crate) staged_modules: BTreeMap, is_simulator: bool, - pub(crate) cluster: Option, + // pub(crate) cluster: Option, + /// A barebones GraphQL client that should be schema-agnostic. + pub(crate) graphql_client: Option, pub(crate) executor: Box, } @@ -211,9 +214,9 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { self.default_syntax } async fn cleanup_resources(&mut self) -> anyhow::Result<()> { - if let Some(cluster) = self.cluster.take() { - cluster.cleanup_resources().await; - } + // if let Some(cluster) = self.cluster.take() { + // cluster.cleanup_resources().await; + // } Ok(()) } async fn init( @@ -354,7 +357,8 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { let mut test_adapter = Self { is_simulator, - cluster, + // cluster, + graphql_client: None, executor, compiled_state: CompiledState::new( named_address_mapping, @@ -576,29 +580,33 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { }) => { let file = data.ok_or_else(|| anyhow::anyhow!("Missing GraphQL query"))?; let contents = std::fs::read_to_string(file.path())?; - let cluster = self.cluster.as_ref().unwrap(); + // Extract graphql client, and raise an error if it's not set + let graphql_client = self + .graphql_client + .as_ref() + .ok_or_else(|| anyhow::anyhow!("GraphQL client not set"))?; + // let cluster = self.cluster.as_ref().unwrap(); let highest_checkpoint = self.executor.get_latest_checkpoint_sequence_number()?; - cluster - .wait_for_checkpoint_catchup(highest_checkpoint, Duration::from_secs(60)) - .await; + // cluster + // .wait_for_checkpoint_catchup(highest_checkpoint, Duration::from_secs(60)) + // .await; - cluster - .wait_for_objects_snapshot_catchup(Duration::from_secs(180)) - .await; + // cluster + // .wait_for_objects_snapshot_catchup(Duration::from_secs(180)) + // .await; if let Some(wait_for_checkpoint_pruned) = wait_for_checkpoint_pruned { - cluster - .wait_for_checkpoint_pruned( - wait_for_checkpoint_pruned, - Duration::from_secs(60), - ) - .await; + // cluster + // .wait_for_checkpoint_pruned( + // wait_for_checkpoint_pruned, + // Duration::from_secs(60), + // ) + // .await; } let interpolated = self.interpolate_query(&contents, &cursors, highest_checkpoint)?; - let resp = cluster - .graphql_client + let resp = graphql_client .execute_to_graphql(interpolated.trim().to_owned(), show_usage, vec![], vec![]) .await?; @@ -1163,6 +1171,10 @@ fn merge_output(left: Option, right: Option) -> Option { } impl<'a> SuiTestAdapter { + pub fn with_graphql_rpc(&mut self, url: String) { + self.graphql_client = Some(SimpleClient::new(url)); + } + pub fn is_simulator(&self) -> bool { self.is_simulator } From 9ab365e85ddcb9b83155566f798e662bcaa1f546 Mon Sep 17 00:00:00 2001 From: Will Yang Date: Thu, 21 Nov 2024 18:17:34 -0800 Subject: [PATCH 07/12] The idea is that sui-test-adapter is responsible at most for starting simulacrum. creating a rest-api, and hooking into the indexer and graphql, should be deferred to the crate caller. but this is probably not the correct approach, because by adding to SuiInitArgs -> Offchain extensions we still have a dependency on the crates... --- .../sui-transactional-test-runner/src/args.rs | 6 ++ .../src/test_adapter.rs | 91 ++++++++++--------- 2 files changed, 56 insertions(+), 41 deletions(-) diff --git a/crates/sui-transactional-test-runner/src/args.rs b/crates/sui-transactional-test-runner/src/args.rs index 59c6a0c4ec816..a3f732dabb27b 100644 --- a/crates/sui-transactional-test-runner/src/args.rs +++ b/crates/sui-transactional-test-runner/src/args.rs @@ -1,6 +1,8 @@ // Copyright (c) Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 +use std::path::PathBuf; + use crate::test_adapter::{FakeID, SuiTestAdapter}; use anyhow::{bail, ensure}; use clap; @@ -73,6 +75,10 @@ pub struct SuiInitArgs { /// the indexer. #[clap(long = "epochs-to-keep")] pub epochs_to_keep: Option, + #[clap(long = "data-ingestion-path")] + pub data_ingestion_path: Option, + #[clap(long = "rest-api-url")] + pub rest_api_url: Option, } #[derive(Debug, clap::Parser)] diff --git a/crates/sui-transactional-test-runner/src/test_adapter.rs b/crates/sui-transactional-test-runner/src/test_adapter.rs index 2c9801ccb9460..cf81eb031fa60 100644 --- a/crates/sui-transactional-test-runner/src/test_adapter.rs +++ b/crates/sui-transactional-test-runner/src/test_adapter.rs @@ -39,6 +39,7 @@ use move_vm_runtime::session::SerializedReturnValues; use once_cell::sync::Lazy; use rand::{rngs::StdRng, Rng, SeedableRng}; use std::fmt::{self, Write}; +use std::path::PathBuf; use std::time::Duration; use std::{ collections::{BTreeMap, BTreeSet}, @@ -49,8 +50,7 @@ use sui_core::authority::test_authority_builder::TestAuthorityBuilder; use sui_core::authority::AuthorityState; use sui_framework::DEFAULT_FRAMEWORK_PATH; use sui_graphql_rpc::client::simple_client::SimpleClient; -use sui_graphql_rpc::test_infra::cluster::ExecutorCluster; -use sui_graphql_rpc::test_infra::cluster::{serve_executor, RetentionConfig, SnapshotLagConfig}; +use sui_graphql_rpc::test_infra::cluster::{RetentionConfig, SnapshotLagConfig}; use sui_json_rpc_api::QUERY_MAX_RESULT_LIMIT; use sui_json_rpc_types::{DevInspectResults, SuiExecutionStatus, SuiTransactionBlockEffectsAPI}; use sui_protocol_config::{Chain, ProtocolConfig}; @@ -67,8 +67,8 @@ use sui_types::messages_checkpoint::{ CheckpointContents, CheckpointContentsDigest, CheckpointSequenceNumber, VerifiedCheckpoint, }; use sui_types::object::bounded_visitor::BoundedVisitor; -use sui_types::storage::ObjectStore; use sui_types::storage::ReadStore; +use sui_types::storage::{ObjectStore, RestStateReader}; use sui_types::transaction::Command; use sui_types::transaction::ProgrammableTransaction; use sui_types::utils::to_sender_signed_transaction_with_multi_signers; @@ -125,6 +125,13 @@ const GAS_FOR_TESTING: u64 = GAS_VALUE_FOR_TESTING; const DEFAULT_CHAIN_START_TIMESTAMP: u64 = 0; +pub struct OffChainConfig { + pub snapshot_config: SnapshotLagConfig, + pub retention_config: Option, + pub data_ingestion_path: PathBuf, + pub rest_api_url: Option, +} + pub struct SuiTestAdapter { pub(crate) compiled_state: CompiledState, /// For upgrades: maps an upgraded package name to the original package name. @@ -141,6 +148,8 @@ pub struct SuiTestAdapter { /// A barebones GraphQL client that should be schema-agnostic. pub(crate) graphql_client: Option, pub(crate) executor: Box, + pub read_replica: Option>, + pub offchain_config: Option, } pub(crate) struct StagedPackage { @@ -245,9 +254,12 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { custom_validator_account, reference_gas_price, default_gas_price, - snapshot_config, + // snapshot_config, flavor, - epochs_to_keep, + // epochs_to_keep, + // data_ingestion_path, + // rest_api_url, + offchain_config, ) = match task_opt.map(|t| t.command) { Some(( InitCommand { named_addresses }, @@ -263,6 +275,8 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { snapshot_config, flavor, epochs_to_keep, + data_ingestion_path, + rest_api_url, }, )) => { let map = verify_and_create_named_address_mapping(named_addresses).unwrap(); @@ -291,6 +305,21 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { panic!("Can only set reference gas price in simulator mode"); } + let offchain_config = if simulator { + let retention_config = epochs_to_keep + .map(RetentionConfig::new_with_default_retention_only_for_testing); + + Some(OffChainConfig { + snapshot_config, + retention_config, + data_ingestion_path: data_ingestion_path + .unwrap_or(tempdir().unwrap().into_path()), + rest_api_url, + }) + } else { + None + }; + ( map, accounts, @@ -299,9 +328,8 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { custom_validator_account, reference_gas_price, default_gas_price, - snapshot_config, flavor, - epochs_to_keep, + offchain_config, ) } None => { @@ -314,7 +342,6 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { false, None, None, - SnapshotLagConfig::default(), None, None, ) @@ -330,13 +357,8 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { objects, account_objects, }, - cluster, + read_replica, ) = if is_simulator { - // TODO: (wlmyng) as of right now, we can't test per-table overrides until the pruner is - // updated - let retention_config = - epochs_to_keep.map(RetentionConfig::new_with_default_retention_only_for_testing); - init_sim_executor( rng, account_names, @@ -344,8 +366,6 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { &protocol_config, custom_validator_account, reference_gas_price, - snapshot_config, - retention_config, ) .await } else { @@ -360,6 +380,8 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { // cluster, graphql_client: None, executor, + offchain_config, + read_replica, compiled_state: CompiledState::new( named_address_mapping, pre_compiled_deps, @@ -2065,7 +2087,7 @@ async fn init_val_fullnode_executor( ) -> ( Box, AccountSetup, - Option, + Option>, ) { // Initial list of named addresses with specified values let mut named_address_mapping = NAMED_ADDRESSES.clone(); @@ -2131,12 +2153,10 @@ async fn init_sim_executor( protocol_config: &ProtocolConfig, custom_validator_account: bool, reference_gas_price: Option, - snapshot_config: SnapshotLagConfig, - retention_config: Option, ) -> ( Box, AccountSetup, - Option, + Option>, ) { // Initial list of named addresses with specified values let mut named_address_mapping = NAMED_ADDRESSES.clone(); @@ -2188,26 +2208,15 @@ async fn init_sim_executor( // Create the simulator with the specific account configs, which also crates objects - let (mut sim, read_replica) = - PersistedStore::new_sim_replica_with_protocol_version_and_accounts( - rng, - DEFAULT_CHAIN_START_TIMESTAMP, - protocol_config.version, - acc_cfgs, - key_copy.map(|q| vec![q]), - reference_gas_price, - None, - ); - let data_ingestion_path = tempdir().unwrap().into_path(); - sim.set_data_ingestion_path(data_ingestion_path.clone()); - - let cluster = serve_executor( - Arc::new(read_replica), - Some(snapshot_config), - retention_config, - data_ingestion_path, - ) - .await; + let (sim, read_replica) = PersistedStore::new_sim_replica_with_protocol_version_and_accounts( + rng, + DEFAULT_CHAIN_START_TIMESTAMP, + protocol_config.version, + acc_cfgs, + key_copy.map(|q| vec![q]), + reference_gas_price, + None, + ); // Get the actual object values from the simulator for (name, (addr, kp)) in account_kps { @@ -2266,7 +2275,7 @@ async fn init_sim_executor( account_objects, accounts, }, - Some(cluster), + Some(Arc::new(read_replica)), ) } From dd2b714f7d4fba7d9c2947542f55b2d29316750c Mon Sep 17 00:00:00 2001 From: Will Yang Date: Thu, 21 Nov 2024 23:20:16 -0800 Subject: [PATCH 08/12] now we can run tests. some will fail because we're missing objects_snapshot state checking functionality, and a few things like configuring graphql based on what we passed into SuiInitArgs. I think we need to figure out a way to take the reader-specific args from SuiInitArgs and handle them in tests.rs. and maybe we hsould have a readerclient? or at least pass in the db url. or the adapter can return a tempdb for the indexer to write to... --- Cargo.lock | 1 + crates/sui-graphql-e2e-tests/tests/tests.rs | 37 ++++- .../sui-transactional-test-runner/Cargo.toml | 1 + .../sui-transactional-test-runner/src/args.rs | 2 + .../src/graphql_client.rs | 137 ++++++++++++++++++ .../sui-transactional-test-runner/src/lib.rs | 1 + .../src/test_adapter.rs | 59 +++++--- 7 files changed, 213 insertions(+), 25 deletions(-) create mode 100644 crates/sui-transactional-test-runner/src/graphql_client.rs diff --git a/Cargo.lock b/Cargo.lock index 5c8a39444f115..b01604952693d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15738,6 +15738,7 @@ dependencies = [ "telemetry-subscribers", "tempfile", "tokio", + "tracing", "typed-store", ] diff --git a/crates/sui-graphql-e2e-tests/tests/tests.rs b/crates/sui-graphql-e2e-tests/tests/tests.rs index 0218f26af9e09..5cf30588a1cb4 100644 --- a/crates/sui-graphql-e2e-tests/tests/tests.rs +++ b/crates/sui-graphql-e2e-tests/tests/tests.rs @@ -4,7 +4,9 @@ #![allow(unused_imports)] #![allow(unused_variables)] use std::{path::Path, sync::Arc}; +use sui_graphql_rpc::test_infra::cluster::serve_executor; use sui_transactional_test_runner::{ + args::SuiInitArgs, create_adapter, run_tasks_with_adapter, test_adapter::{SuiTestAdapter, PRE_COMPILED}, }; @@ -24,9 +26,42 @@ datatest_stable::harness!( async fn run_test(path: &Path) -> Result<(), Box> { telemetry_subscribers::init_for_testing(); if !cfg!(msim) { - let (output, adapter) = + // extract init args + + // if test wants to provide data-ingestion-path and rest-api-url to testadapter...this seems harder + // easier for testadapter to tell indexer where it's writing to + + // then initialize the adapter + + // snapshotconfig and retentionconfig ofc + // serve_executor(adapter.read_replica, init_opt..., init_opt..., adapter.data_ingestion_path).await?; + + // start the adapter first to start the executor + let (output, mut adapter) = create_adapter::(path, Some(Arc::new(PRE_COMPILED.clone()))).await?; + + let cluster = serve_executor( + adapter.read_replica.as_ref().unwrap().clone(), + None, + None, + adapter + .offchain_config + .as_ref() + .unwrap() + .data_ingestion_path + .clone(), + ) + .await; + + adapter.with_graphql_rpc(format!( + "http://{}:{}", + cluster.graphql_connection_config.host, cluster.graphql_connection_config.port + )); + + // serve_executor, which is kind of a misnomer since it takes the read replica run_tasks_with_adapter(path, adapter, output).await?; + + // and then cleanup } Ok(()) } diff --git a/crates/sui-transactional-test-runner/Cargo.toml b/crates/sui-transactional-test-runner/Cargo.toml index 952096d312d6c..8b32c70d0ee87 100644 --- a/crates/sui-transactional-test-runner/Cargo.toml +++ b/crates/sui-transactional-test-runner/Cargo.toml @@ -25,6 +25,7 @@ tokio.workspace = true serde_json.workspace = true futures.workspace = true criterion.workspace = true +tracing.workspace = true fastcrypto.workspace = true move-binary-format.workspace = true diff --git a/crates/sui-transactional-test-runner/src/args.rs b/crates/sui-transactional-test-runner/src/args.rs index a3f732dabb27b..bdc14aacae3f9 100644 --- a/crates/sui-transactional-test-runner/src/args.rs +++ b/crates/sui-transactional-test-runner/src/args.rs @@ -75,8 +75,10 @@ pub struct SuiInitArgs { /// the indexer. #[clap(long = "epochs-to-keep")] pub epochs_to_keep: Option, + /// TODO (wlmyng): doc comment #[clap(long = "data-ingestion-path")] pub data_ingestion_path: Option, + /// TODO (wlmyng): doc comment #[clap(long = "rest-api-url")] pub rest_api_url: Option, } diff --git a/crates/sui-transactional-test-runner/src/graphql_client.rs b/crates/sui-transactional-test-runner/src/graphql_client.rs new file mode 100644 index 0000000000000..fee918819be8d --- /dev/null +++ b/crates/sui-transactional-test-runner/src/graphql_client.rs @@ -0,0 +1,137 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +use std::time::Duration; +use sui_graphql_rpc::client::simple_client::SimpleClient; +use tracing::info; + +/// Generic polling function for GraphQL queries +async fn poll_until_condition( + client: &SimpleClient, + query: String, + timeout_msg: &str, + base_timeout: Duration, + scale: u64, + check_condition: F, +) where + F: Fn(&serde_json::Value) -> bool, +{ + let timeout = base_timeout.mul_f64(scale.max(1) as f64); + + tokio::time::timeout(timeout, async { + loop { + let resp = client + .execute_to_graphql(query.to_string(), false, vec![], vec![]) + .await + .unwrap() + .response_body_json(); + + if check_condition(&resp) { + break; + } + tokio::time::sleep(Duration::from_secs(1)).await; + } + }) + .await + .expect(timeout_msg); +} + +pub async fn wait_for_checkpoint_catchup( + client: &SimpleClient, + checkpoint: u64, + base_timeout: Duration, +) { + info!( + "Waiting for graphql to catchup to checkpoint {}, base time out is {}", + checkpoint, + base_timeout.as_secs() + ); + + let query = r#" + { + availableRange { + last { + sequenceNumber + } + } + }"# + .to_string(); + + poll_until_condition( + client, + query, + "Timeout waiting for graphql to catchup to checkpoint", + base_timeout, + checkpoint, + |resp| { + let current_checkpoint = resp["data"]["availableRange"]["last"].get("sequenceNumber"); + current_checkpoint + .and_then(|cp| cp.as_u64()) + .map_or(false, |cp| cp >= checkpoint) + }, + ) + .await; +} + +pub async fn wait_for_epoch_catchup(client: &SimpleClient, epoch: u64, base_timeout: Duration) { + info!( + "Waiting for graphql to catchup to epoch {}, base time out is {}", + epoch, + base_timeout.as_secs() + ); + + let query = r#" + { + epoch { + epochId + } + }"# + .to_string(); + + poll_until_condition( + client, + query, + "Timeout waiting for graphql to catchup to epoch", + base_timeout, + epoch, + |resp| { + let latest_epoch = resp["data"]["epoch"].get("epochId"); + latest_epoch + .and_then(|e| e.as_u64()) + .map_or(false, |e| e >= epoch) + }, + ) + .await; +} + +pub async fn wait_for_pruned_checkpoint( + client: &SimpleClient, + checkpoint: u64, + base_timeout: Duration, +) { + info!( + "Waiting for checkpoint to be pruned {}, base time out is {}", + checkpoint, + base_timeout.as_secs() + ); + + let query = format!( + r#" + {{ + checkpoint(id: {{ sequenceNumber: {} }}) {{ + sequenceNumber + }} + }}"#, + checkpoint + ); + + poll_until_condition( + client, + query, + "Timeout waiting for checkpoint to be pruned", + base_timeout, + checkpoint, + |resp| resp["data"]["checkpoint"].is_null(), + ) + .await; +} diff --git a/crates/sui-transactional-test-runner/src/lib.rs b/crates/sui-transactional-test-runner/src/lib.rs index 3e041e68b4f1f..4f5e30141285a 100644 --- a/crates/sui-transactional-test-runner/src/lib.rs +++ b/crates/sui-transactional-test-runner/src/lib.rs @@ -4,6 +4,7 @@ //! This module contains the transactional test runner instantiation for the Sui adapter pub mod args; +mod graphql_client; pub mod programmable_transaction_test_parser; mod simulator_persisted_store; pub mod test_adapter; diff --git a/crates/sui-transactional-test-runner/src/test_adapter.rs b/crates/sui-transactional-test-runner/src/test_adapter.rs index cf81eb031fa60..15633bd63a5fb 100644 --- a/crates/sui-transactional-test-runner/src/test_adapter.rs +++ b/crates/sui-transactional-test-runner/src/test_adapter.rs @@ -3,6 +3,7 @@ //! This module contains the transactional test runner instantiation for the Sui adapter +use crate::graphql_client::*; use crate::simulator_persisted_store::PersistedStore; use crate::{args::*, programmable_transaction_test_parser::parser::ParsedCommand}; use crate::{TransactionalAdapter, ValidatorWithFullnode}; @@ -366,6 +367,11 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { &protocol_config, custom_validator_account, reference_gas_price, + offchain_config + .as_ref() + .unwrap() + .data_ingestion_path + .clone(), ) .await } else { @@ -609,21 +615,22 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { .ok_or_else(|| anyhow::anyhow!("GraphQL client not set"))?; // let cluster = self.cluster.as_ref().unwrap(); let highest_checkpoint = self.executor.get_latest_checkpoint_sequence_number()?; - // cluster - // .wait_for_checkpoint_catchup(highest_checkpoint, Duration::from_secs(60)) - // .await; - - // cluster - // .wait_for_objects_snapshot_catchup(Duration::from_secs(180)) - // .await; - - if let Some(wait_for_checkpoint_pruned) = wait_for_checkpoint_pruned { - // cluster - // .wait_for_checkpoint_pruned( - // wait_for_checkpoint_pruned, - // Duration::from_secs(60), - // ) - // .await; + wait_for_checkpoint_catchup( + graphql_client, + highest_checkpoint, + Duration::from_secs(60), + ) + .await; + + // wait_for_objects_snapshot_catchup(graphql_client, Duration::from_secs(180)).await; + + if let Some(checkpoint_to_prune) = wait_for_checkpoint_pruned { + wait_for_pruned_checkpoint( + graphql_client, + checkpoint_to_prune, + Duration::from_secs(60), + ) + .await; } let interpolated = @@ -2153,6 +2160,7 @@ async fn init_sim_executor( protocol_config: &ProtocolConfig, custom_validator_account: bool, reference_gas_price: Option, + data_ingestion_path: PathBuf, ) -> ( Box, AccountSetup, @@ -2208,15 +2216,18 @@ async fn init_sim_executor( // Create the simulator with the specific account configs, which also crates objects - let (sim, read_replica) = PersistedStore::new_sim_replica_with_protocol_version_and_accounts( - rng, - DEFAULT_CHAIN_START_TIMESTAMP, - protocol_config.version, - acc_cfgs, - key_copy.map(|q| vec![q]), - reference_gas_price, - None, - ); + let (mut sim, read_replica) = + PersistedStore::new_sim_replica_with_protocol_version_and_accounts( + rng, + DEFAULT_CHAIN_START_TIMESTAMP, + protocol_config.version, + acc_cfgs, + key_copy.map(|q| vec![q]), + reference_gas_price, + None, + ); + + sim.set_data_ingestion_path(data_ingestion_path.clone()); // Get the actual object values from the simulator for (name, (addr, kp)) in account_kps { From 50052c5f24163058f2f5be7ceebe173c0ee71618 Mon Sep 17 00:00:00 2001 From: Will Yang Date: Fri, 22 Nov 2024 11:19:40 -0800 Subject: [PATCH 09/12] slight refactoring, otherwise really difficult to see how the tuple of init args were being instantiated --- .../src/test_adapter.rs | 199 ++++++++++-------- 1 file changed, 108 insertions(+), 91 deletions(-) diff --git a/crates/sui-transactional-test-runner/src/test_adapter.rs b/crates/sui-transactional-test-runner/src/test_adapter.rs index 15633bd63a5fb..f0a49224faee9 100644 --- a/crates/sui-transactional-test-runner/src/test_adapter.rs +++ b/crates/sui-transactional-test-runner/src/test_adapter.rs @@ -126,6 +126,9 @@ const GAS_FOR_TESTING: u64 = GAS_VALUE_FOR_TESTING; const DEFAULT_CHAIN_START_TIMESTAMP: u64 = 0; +/// Extra args related to configuring the indexer and reader. +// TODO: the configs are still tied to the indexer crate, eventually we'd like a new command that is +// more agnostic. pub struct OffChainConfig { pub snapshot_config: SnapshotLagConfig, pub retention_config: Option, @@ -153,6 +156,18 @@ pub struct SuiTestAdapter { pub offchain_config: Option, } +struct AdapterInitConfig { + additional_mapping: BTreeMap, + account_names: BTreeSet, + protocol_config: ProtocolConfig, + is_simulator: bool, + custom_validator_account: bool, + reference_gas_price: Option, + default_gas_price: Option, + flavor: Option, + offchain_config: Option, +} + pub(crate) struct StagedPackage { file: NamedTempFile, syntax: SyntaxChoice, @@ -180,6 +195,79 @@ struct TxnSummary { gas_summary: GasCostSummary, } +impl AdapterInitConfig { + fn from_args(init_cmd: InitCommand, sui_args: SuiInitArgs) -> Self { + let InitCommand { named_addresses } = init_cmd; + let SuiInitArgs { + accounts, + protocol_version, + max_gas, + shared_object_deletion, + simulator, + custom_validator_account, + reference_gas_price, + default_gas_price, + snapshot_config, + flavor, + epochs_to_keep, + data_ingestion_path, + rest_api_url, + } = sui_args; + + let map = verify_and_create_named_address_mapping(named_addresses).unwrap(); + let accounts = accounts + .map(|v| v.into_iter().collect::>()) + .unwrap_or_default(); + + let mut protocol_config = if let Some(protocol_version) = protocol_version { + ProtocolConfig::get_for_version(protocol_version.into(), Chain::Unknown) + } else { + ProtocolConfig::get_for_max_version_UNSAFE() + }; + if let Some(enable) = shared_object_deletion { + protocol_config.set_shared_object_deletion_for_testing(enable); + } + if let Some(mx_tx_gas_override) = max_gas { + if simulator { + panic!("Cannot set max gas in simulator mode"); + } + protocol_config.set_max_tx_gas_for_testing(mx_tx_gas_override) + } + if custom_validator_account && !simulator { + panic!("Can only set custom validator account in simulator mode"); + } + if reference_gas_price.is_some() && !simulator { + panic!("Can only set reference gas price in simulator mode"); + } + + let offchain_config = if simulator { + let retention_config = + epochs_to_keep.map(RetentionConfig::new_with_default_retention_only_for_testing); + + Some(OffChainConfig { + snapshot_config, + retention_config, + data_ingestion_path: data_ingestion_path.unwrap_or(tempdir().unwrap().into_path()), + rest_api_url, + }) + } else { + None + }; + + Self { + additional_mapping: map, + account_names: accounts, + protocol_config, + is_simulator: simulator, + custom_validator_account, + reference_gas_price, + default_gas_price, + flavor, + offchain_config, + } + } +} + #[async_trait] impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { type ExtraPublishArgs = SuiPublishArgs; @@ -247,7 +335,7 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { ); // Unpack the init arguments - let ( + let AdapterInitConfig { additional_mapping, account_names, protocol_config, @@ -255,98 +343,11 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { custom_validator_account, reference_gas_price, default_gas_price, - // snapshot_config, flavor, - // epochs_to_keep, - // data_ingestion_path, - // rest_api_url, offchain_config, - ) = match task_opt.map(|t| t.command) { - Some(( - InitCommand { named_addresses }, - SuiInitArgs { - accounts, - protocol_version, - max_gas, - shared_object_deletion, - simulator, - custom_validator_account, - reference_gas_price, - default_gas_price, - snapshot_config, - flavor, - epochs_to_keep, - data_ingestion_path, - rest_api_url, - }, - )) => { - let map = verify_and_create_named_address_mapping(named_addresses).unwrap(); - let accounts = accounts - .map(|v| v.into_iter().collect::>()) - .unwrap_or_default(); - - let mut protocol_config = if let Some(protocol_version) = protocol_version { - ProtocolConfig::get_for_version(protocol_version.into(), Chain::Unknown) - } else { - ProtocolConfig::get_for_max_version_UNSAFE() - }; - if let Some(enable) = shared_object_deletion { - protocol_config.set_shared_object_deletion_for_testing(enable); - } - if let Some(mx_tx_gas_override) = max_gas { - if simulator { - panic!("Cannot set max gas in simulator mode"); - } - protocol_config.set_max_tx_gas_for_testing(mx_tx_gas_override) - } - if custom_validator_account && !simulator { - panic!("Can only set custom validator account in simulator mode"); - } - if reference_gas_price.is_some() && !simulator { - panic!("Can only set reference gas price in simulator mode"); - } - - let offchain_config = if simulator { - let retention_config = epochs_to_keep - .map(RetentionConfig::new_with_default_retention_only_for_testing); - - Some(OffChainConfig { - snapshot_config, - retention_config, - data_ingestion_path: data_ingestion_path - .unwrap_or(tempdir().unwrap().into_path()), - rest_api_url, - }) - } else { - None - }; - - ( - map, - accounts, - protocol_config, - simulator, - custom_validator_account, - reference_gas_price, - default_gas_price, - flavor, - offchain_config, - ) - } - None => { - let protocol_config = ProtocolConfig::get_for_max_version_UNSAFE(); - ( - BTreeMap::new(), - BTreeSet::new(), - protocol_config, - false, - false, - None, - None, - None, - None, - ) - } + } = match task_opt.map(|t| t.command) { + Some((init_cmd, sui_args)) => AdapterInitConfig::from_args(init_cmd, sui_args), + None => AdapterInitConfig::default(), }; let ( @@ -1948,6 +1949,22 @@ impl fmt::Display for FakeID { } } +impl Default for AdapterInitConfig { + fn default() -> Self { + Self { + additional_mapping: BTreeMap::new(), + account_names: BTreeSet::new(), + protocol_config: ProtocolConfig::get_for_max_version_UNSAFE(), + is_simulator: false, + custom_validator_account: false, + reference_gas_price: None, + default_gas_price: None, + flavor: None, + offchain_config: None, + } + } +} + static NAMED_ADDRESSES: Lazy> = Lazy::new(|| { let mut map = move_stdlib::move_stdlib_named_addresses(); assert!(map.get("std").unwrap().into_inner() == MOVE_STDLIB_ADDRESS); From 72f46bb3f110e3a1c6711b869711173e6079ff2e Mon Sep 17 00:00:00 2001 From: Will Yang Date: Fri, 22 Nov 2024 12:13:05 -0800 Subject: [PATCH 10/12] use a trait offchainreader instead of the simple client or something. this also means that we can actually use the result of serve_executor, with some arc magic I suppose. Now, the workflow is to start the adapter first to instantiate the simulacrum, extract the offchain_config, start the indexer and graphql services, and pass in some sort of offchain reader for the adapter. Make sure to clean up resources by reclaiming the arc --- Cargo.lock | 2 + crates/sui-graphql-e2e-tests/Cargo.toml | 2 + crates/sui-graphql-e2e-tests/tests/tests.rs | 95 ++++++++---- .../src/graphql_client.rs | 137 ------------------ .../sui-transactional-test-runner/src/lib.rs | 2 +- .../src/offchain_state.rs | 23 +++ .../src/test_adapter.rs | 60 ++++---- 7 files changed, 121 insertions(+), 200 deletions(-) delete mode 100644 crates/sui-transactional-test-runner/src/graphql_client.rs create mode 100644 crates/sui-transactional-test-runner/src/offchain_state.rs diff --git a/Cargo.lock b/Cargo.lock index b01604952693d..f5ed0498d0c56 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13891,6 +13891,8 @@ dependencies = [ name = "sui-graphql-e2e-tests" version = "0.1.0" dependencies = [ + "anyhow", + "async-trait", "datatest-stable", "msim", "sui-graphql-rpc", diff --git a/crates/sui-graphql-e2e-tests/Cargo.toml b/crates/sui-graphql-e2e-tests/Cargo.toml index 3e9dc0463dad3..fd41452210b57 100644 --- a/crates/sui-graphql-e2e-tests/Cargo.toml +++ b/crates/sui-graphql-e2e-tests/Cargo.toml @@ -11,6 +11,8 @@ edition = "2021" workspace = true [dev-dependencies] +anyhow.workspace = true +async-trait.workspace = true telemetry-subscribers.workspace = true datatest-stable.workspace = true sui-graphql-rpc.workspace = true diff --git a/crates/sui-graphql-e2e-tests/tests/tests.rs b/crates/sui-graphql-e2e-tests/tests/tests.rs index 5cf30588a1cb4..c3408e84b60fa 100644 --- a/crates/sui-graphql-e2e-tests/tests/tests.rs +++ b/crates/sui-graphql-e2e-tests/tests/tests.rs @@ -3,14 +3,60 @@ #![allow(unused_imports)] #![allow(unused_variables)] -use std::{path::Path, sync::Arc}; -use sui_graphql_rpc::test_infra::cluster::serve_executor; +use async_trait::async_trait; +use std::{path::Path, sync::Arc, time::Duration}; +use sui_graphql_rpc::test_infra::cluster::{serve_executor, ExecutorCluster}; use sui_transactional_test_runner::{ args::SuiInitArgs, - create_adapter, run_tasks_with_adapter, + create_adapter, + offchain_state::{OffchainStateReader, TestResponse}, + run_tasks_with_adapter, test_adapter::{SuiTestAdapter, PRE_COMPILED}, }; +pub struct OffchainReaderForAdapter { + cluster: Arc, +} + +#[async_trait] +impl OffchainStateReader for OffchainReaderForAdapter { + async fn wait_for_objects_snapshot_catchup(&self, base_timeout: Duration) { + self.cluster + .wait_for_objects_snapshot_catchup(base_timeout) + .await + } + + async fn wait_for_checkpoint_catchup(&self, checkpoint: u64, base_timeout: Duration) { + self.cluster + .wait_for_checkpoint_catchup(checkpoint, base_timeout) + .await + } + + async fn wait_for_pruned_checkpoint(&self, checkpoint: u64, base_timeout: Duration) { + self.cluster + .wait_for_checkpoint_pruned(checkpoint, base_timeout) + .await + } + + async fn execute_graphql( + &self, + query: String, + show_usage: bool, + ) -> Result { + let result = self + .cluster + .graphql_client + .execute_to_graphql(query, show_usage, vec![], vec![]) + .await?; + + Ok(TestResponse { + http_headers: Some(format!("{:#?}", result.http_headers_without_date())), + response_body: result.response_body_json_pretty(), + service_version: result.graphql_version().ok(), + }) + } +} + datatest_stable::harness!( run_test, "tests", @@ -26,42 +72,35 @@ datatest_stable::harness!( async fn run_test(path: &Path) -> Result<(), Box> { telemetry_subscribers::init_for_testing(); if !cfg!(msim) { - // extract init args - - // if test wants to provide data-ingestion-path and rest-api-url to testadapter...this seems harder - // easier for testadapter to tell indexer where it's writing to - - // then initialize the adapter - - // snapshotconfig and retentionconfig ofc - // serve_executor(adapter.read_replica, init_opt..., init_opt..., adapter.data_ingestion_path).await?; - - // start the adapter first to start the executor + // start the adapter first to start the executor (simulacrum) let (output, mut adapter) = create_adapter::(path, Some(Arc::new(PRE_COMPILED.clone()))).await?; + // In another crate like `sui-mvr-graphql-e2e-tests`, this would be the place to translate + // from `offchain_config` to something compatible with the indexer and graphql flavor of + // choice. + let offchain_config = adapter.offchain_config.as_ref().unwrap(); + let cluster = serve_executor( adapter.read_replica.as_ref().unwrap().clone(), - None, - None, - adapter - .offchain_config - .as_ref() - .unwrap() - .data_ingestion_path - .clone(), + Some(offchain_config.snapshot_config.clone()), + offchain_config.retention_config.clone(), + offchain_config.data_ingestion_path.clone(), ) .await; - adapter.with_graphql_rpc(format!( - "http://{}:{}", - cluster.graphql_connection_config.host, cluster.graphql_connection_config.port - )); + let cluster_arc = Arc::new(cluster); + + adapter.with_offchain_reader(Box::new(OffchainReaderForAdapter { + cluster: cluster_arc.clone(), + })); - // serve_executor, which is kind of a misnomer since it takes the read replica run_tasks_with_adapter(path, adapter, output).await?; - // and then cleanup + match Arc::try_unwrap(cluster_arc) { + Ok(cluster) => cluster.cleanup_resources().await, + Err(_) => panic!("Still other Arc references!"), + } } Ok(()) } diff --git a/crates/sui-transactional-test-runner/src/graphql_client.rs b/crates/sui-transactional-test-runner/src/graphql_client.rs deleted file mode 100644 index fee918819be8d..0000000000000 --- a/crates/sui-transactional-test-runner/src/graphql_client.rs +++ /dev/null @@ -1,137 +0,0 @@ -// Copyright (c) Mysten Labs, Inc. -// SPDX-License-Identifier: Apache-2.0 - -use std::time::Duration; -use sui_graphql_rpc::client::simple_client::SimpleClient; -use tracing::info; - -/// Generic polling function for GraphQL queries -async fn poll_until_condition( - client: &SimpleClient, - query: String, - timeout_msg: &str, - base_timeout: Duration, - scale: u64, - check_condition: F, -) where - F: Fn(&serde_json::Value) -> bool, -{ - let timeout = base_timeout.mul_f64(scale.max(1) as f64); - - tokio::time::timeout(timeout, async { - loop { - let resp = client - .execute_to_graphql(query.to_string(), false, vec![], vec![]) - .await - .unwrap() - .response_body_json(); - - if check_condition(&resp) { - break; - } - tokio::time::sleep(Duration::from_secs(1)).await; - } - }) - .await - .expect(timeout_msg); -} - -pub async fn wait_for_checkpoint_catchup( - client: &SimpleClient, - checkpoint: u64, - base_timeout: Duration, -) { - info!( - "Waiting for graphql to catchup to checkpoint {}, base time out is {}", - checkpoint, - base_timeout.as_secs() - ); - - let query = r#" - { - availableRange { - last { - sequenceNumber - } - } - }"# - .to_string(); - - poll_until_condition( - client, - query, - "Timeout waiting for graphql to catchup to checkpoint", - base_timeout, - checkpoint, - |resp| { - let current_checkpoint = resp["data"]["availableRange"]["last"].get("sequenceNumber"); - current_checkpoint - .and_then(|cp| cp.as_u64()) - .map_or(false, |cp| cp >= checkpoint) - }, - ) - .await; -} - -pub async fn wait_for_epoch_catchup(client: &SimpleClient, epoch: u64, base_timeout: Duration) { - info!( - "Waiting for graphql to catchup to epoch {}, base time out is {}", - epoch, - base_timeout.as_secs() - ); - - let query = r#" - { - epoch { - epochId - } - }"# - .to_string(); - - poll_until_condition( - client, - query, - "Timeout waiting for graphql to catchup to epoch", - base_timeout, - epoch, - |resp| { - let latest_epoch = resp["data"]["epoch"].get("epochId"); - latest_epoch - .and_then(|e| e.as_u64()) - .map_or(false, |e| e >= epoch) - }, - ) - .await; -} - -pub async fn wait_for_pruned_checkpoint( - client: &SimpleClient, - checkpoint: u64, - base_timeout: Duration, -) { - info!( - "Waiting for checkpoint to be pruned {}, base time out is {}", - checkpoint, - base_timeout.as_secs() - ); - - let query = format!( - r#" - {{ - checkpoint(id: {{ sequenceNumber: {} }}) {{ - sequenceNumber - }} - }}"#, - checkpoint - ); - - poll_until_condition( - client, - query, - "Timeout waiting for checkpoint to be pruned", - base_timeout, - checkpoint, - |resp| resp["data"]["checkpoint"].is_null(), - ) - .await; -} diff --git a/crates/sui-transactional-test-runner/src/lib.rs b/crates/sui-transactional-test-runner/src/lib.rs index 4f5e30141285a..760c65d1f3c4c 100644 --- a/crates/sui-transactional-test-runner/src/lib.rs +++ b/crates/sui-transactional-test-runner/src/lib.rs @@ -4,7 +4,7 @@ //! This module contains the transactional test runner instantiation for the Sui adapter pub mod args; -mod graphql_client; +pub mod offchain_state; pub mod programmable_transaction_test_parser; mod simulator_persisted_store; pub mod test_adapter; diff --git a/crates/sui-transactional-test-runner/src/offchain_state.rs b/crates/sui-transactional-test-runner/src/offchain_state.rs new file mode 100644 index 0000000000000..384d7789748cd --- /dev/null +++ b/crates/sui-transactional-test-runner/src/offchain_state.rs @@ -0,0 +1,23 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +use async_trait::async_trait; +use std::time::Duration; + +pub struct TestResponse { + pub response_body: String, + pub http_headers: Option, + pub service_version: Option, +} + +#[async_trait] +pub trait OffchainStateReader: Send + Sync + 'static { + async fn wait_for_objects_snapshot_catchup(&self, base_timeout: Duration); + async fn wait_for_checkpoint_catchup(&self, checkpoint: u64, base_timeout: Duration); + async fn wait_for_pruned_checkpoint(&self, checkpoint: u64, base_timeout: Duration); + async fn execute_graphql( + &self, + query: String, + show_usage: bool, + ) -> Result; +} diff --git a/crates/sui-transactional-test-runner/src/test_adapter.rs b/crates/sui-transactional-test-runner/src/test_adapter.rs index f0a49224faee9..ab6b77174df9e 100644 --- a/crates/sui-transactional-test-runner/src/test_adapter.rs +++ b/crates/sui-transactional-test-runner/src/test_adapter.rs @@ -3,7 +3,7 @@ //! This module contains the transactional test runner instantiation for the Sui adapter -use crate::graphql_client::*; +use crate::offchain_state::OffchainStateReader; use crate::simulator_persisted_store::PersistedStore; use crate::{args::*, programmable_transaction_test_parser::parser::ParsedCommand}; use crate::{TransactionalAdapter, ValidatorWithFullnode}; @@ -50,7 +50,6 @@ use std::{ use sui_core::authority::test_authority_builder::TestAuthorityBuilder; use sui_core::authority::AuthorityState; use sui_framework::DEFAULT_FRAMEWORK_PATH; -use sui_graphql_rpc::client::simple_client::SimpleClient; use sui_graphql_rpc::test_infra::cluster::{RetentionConfig, SnapshotLagConfig}; use sui_json_rpc_api::QUERY_MAX_RESULT_LIMIT; use sui_json_rpc_types::{DevInspectResults, SuiExecutionStatus, SuiTransactionBlockEffectsAPI}; @@ -148,12 +147,13 @@ pub struct SuiTestAdapter { gas_price: u64, pub(crate) staged_modules: BTreeMap, is_simulator: bool, - // pub(crate) cluster: Option, - /// A barebones GraphQL client that should be schema-agnostic. - pub(crate) graphql_client: Option, pub(crate) executor: Box, pub read_replica: Option>, + /// Configuration for offchain state reader read from the file itself, and can be passed to the + /// specific indexing and reader flavor. pub offchain_config: Option, + /// A trait encapsulating methods to interact with offchain state. + pub offchain_reader: Option>, } struct AdapterInitConfig { @@ -312,9 +312,6 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { self.default_syntax } async fn cleanup_resources(&mut self) -> anyhow::Result<()> { - // if let Some(cluster) = self.cluster.take() { - // cluster.cleanup_resources().await; - // } Ok(()) } async fn init( @@ -384,8 +381,8 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { let mut test_adapter = Self { is_simulator, - // cluster, - graphql_client: None, + // This is opt-in and instantiated later + offchain_reader: None, executor, offchain_config, read_replica, @@ -609,45 +606,40 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { }) => { let file = data.ok_or_else(|| anyhow::anyhow!("Missing GraphQL query"))?; let contents = std::fs::read_to_string(file.path())?; - // Extract graphql client, and raise an error if it's not set - let graphql_client = self - .graphql_client + let offchain_reader = self + .offchain_reader .as_ref() - .ok_or_else(|| anyhow::anyhow!("GraphQL client not set"))?; - // let cluster = self.cluster.as_ref().unwrap(); + .ok_or_else(|| anyhow::anyhow!("Offchain reader not set"))?; let highest_checkpoint = self.executor.get_latest_checkpoint_sequence_number()?; - wait_for_checkpoint_catchup( - graphql_client, - highest_checkpoint, - Duration::from_secs(60), - ) - .await; + offchain_reader + .wait_for_checkpoint_catchup(highest_checkpoint, Duration::from_secs(60)) + .await; // wait_for_objects_snapshot_catchup(graphql_client, Duration::from_secs(180)).await; if let Some(checkpoint_to_prune) = wait_for_checkpoint_pruned { - wait_for_pruned_checkpoint( - graphql_client, - checkpoint_to_prune, - Duration::from_secs(60), - ) - .await; + offchain_reader + .wait_for_pruned_checkpoint(checkpoint_to_prune, Duration::from_secs(60)) + .await; } let interpolated = self.interpolate_query(&contents, &cursors, highest_checkpoint)?; - let resp = graphql_client - .execute_to_graphql(interpolated.trim().to_owned(), show_usage, vec![], vec![]) + let resp = offchain_reader + .execute_graphql(interpolated.trim().to_owned(), show_usage) .await?; let mut output = vec![]; if show_headers { - output.push(format!("Headers: {:#?}", resp.http_headers_without_date())); + output.push(format!("Headers: {:#?}", resp.http_headers)); } if show_service_version { - output.push(format!("Service version: {}", resp.graphql_version()?)); + output.push(format!( + "Service version: {}", + resp.service_version.unwrap() + )); } - output.push(format!("Response: {}", resp.response_body_json_pretty())); + output.push(format!("Response: {}", resp.response_body)); Ok(Some(output.join("\n"))) } @@ -1201,8 +1193,8 @@ fn merge_output(left: Option, right: Option) -> Option { } impl<'a> SuiTestAdapter { - pub fn with_graphql_rpc(&mut self, url: String) { - self.graphql_client = Some(SimpleClient::new(url)); + pub fn with_offchain_reader(&mut self, offchain_reader: Box) { + self.offchain_reader = Some(offchain_reader); } pub fn is_simulator(&self) -> bool { From 18b8e7114d7f3b3389cc485bcaba1d0c1b2683ef Mon Sep 17 00:00:00 2001 From: Will Yang Date: Fri, 22 Nov 2024 12:29:18 -0800 Subject: [PATCH 11/12] use the http::HeaderMap instead of dealing with string silliness --- Cargo.lock | 1 + crates/sui-graphql-e2e-tests/tests/tests.rs | 2 +- crates/sui-transactional-test-runner/Cargo.toml | 1 + crates/sui-transactional-test-runner/src/offchain_state.rs | 2 +- crates/sui-transactional-test-runner/src/test_adapter.rs | 2 +- 5 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f5ed0498d0c56..b70ac35172d1e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15709,6 +15709,7 @@ dependencies = [ "eyre", "fastcrypto", "futures", + "http 1.1.0", "move-binary-format", "move-bytecode-utils", "move-command-line-common", diff --git a/crates/sui-graphql-e2e-tests/tests/tests.rs b/crates/sui-graphql-e2e-tests/tests/tests.rs index c3408e84b60fa..4dde7e1677606 100644 --- a/crates/sui-graphql-e2e-tests/tests/tests.rs +++ b/crates/sui-graphql-e2e-tests/tests/tests.rs @@ -50,7 +50,7 @@ impl OffchainStateReader for OffchainReaderForAdapter { .await?; Ok(TestResponse { - http_headers: Some(format!("{:#?}", result.http_headers_without_date())), + http_headers: Some(result.http_headers_without_date()), response_body: result.response_body_json_pretty(), service_version: result.graphql_version().ok(), }) diff --git a/crates/sui-transactional-test-runner/Cargo.toml b/crates/sui-transactional-test-runner/Cargo.toml index 8b32c70d0ee87..71079a81477d8 100644 --- a/crates/sui-transactional-test-runner/Cargo.toml +++ b/crates/sui-transactional-test-runner/Cargo.toml @@ -16,6 +16,7 @@ bcs.workspace = true bimap.workspace = true clap.workspace = true eyre.workspace = true +http.workspace = true once_cell.workspace = true rand.workspace = true regex.workspace = true diff --git a/crates/sui-transactional-test-runner/src/offchain_state.rs b/crates/sui-transactional-test-runner/src/offchain_state.rs index 384d7789748cd..968262ed805d1 100644 --- a/crates/sui-transactional-test-runner/src/offchain_state.rs +++ b/crates/sui-transactional-test-runner/src/offchain_state.rs @@ -6,7 +6,7 @@ use std::time::Duration; pub struct TestResponse { pub response_body: String, - pub http_headers: Option, + pub http_headers: Option, pub service_version: Option, } diff --git a/crates/sui-transactional-test-runner/src/test_adapter.rs b/crates/sui-transactional-test-runner/src/test_adapter.rs index ab6b77174df9e..fd1d0f4c72ae0 100644 --- a/crates/sui-transactional-test-runner/src/test_adapter.rs +++ b/crates/sui-transactional-test-runner/src/test_adapter.rs @@ -631,7 +631,7 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { let mut output = vec![]; if show_headers { - output.push(format!("Headers: {:#?}", resp.http_headers)); + output.push(format!("Headers: {:#?}", resp.http_headers.unwrap())); } if show_service_version { output.push(format!( From c9e08693a79f30506418669950837989523ff8e2 Mon Sep 17 00:00:00 2001 From: Will Yang Date: Fri, 22 Nov 2024 14:24:16 -0800 Subject: [PATCH 12/12] address pr comments, add doc comments --- crates/sui-graphql-rpc/src/test_infra/cluster.rs | 4 ++++ crates/sui-transactional-test-runner/src/args.rs | 9 +++++---- .../src/offchain_state.rs | 7 +++++++ .../sui-transactional-test-runner/src/test_adapter.rs | 10 +++++++++- 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/crates/sui-graphql-rpc/src/test_infra/cluster.rs b/crates/sui-graphql-rpc/src/test_infra/cluster.rs index b023997515412..313bd747692b3 100644 --- a/crates/sui-graphql-rpc/src/test_infra/cluster.rs +++ b/crates/sui-graphql-rpc/src/test_infra/cluster.rs @@ -174,12 +174,16 @@ pub async fn serve_executor( .parse() .unwrap(); + info!("Starting executor server on {}", executor_server_url); + let executor_server_handle = tokio::spawn(async move { sui_rest_api::RestService::new_without_version(executor) .start_service(executor_server_url) .await; }); + info!("spawned executor server"); + let snapshot_config = snapshot_config.unwrap_or_default(); let (pg_store, pg_handle, _) = start_indexer_writer_for_testing( diff --git a/crates/sui-transactional-test-runner/src/args.rs b/crates/sui-transactional-test-runner/src/args.rs index bdc14aacae3f9..027ccec1b1434 100644 --- a/crates/sui-transactional-test-runner/src/args.rs +++ b/crates/sui-transactional-test-runner/src/args.rs @@ -75,11 +75,12 @@ pub struct SuiInitArgs { /// the indexer. #[clap(long = "epochs-to-keep")] pub epochs_to_keep: Option, - /// TODO (wlmyng): doc comment - #[clap(long = "data-ingestion-path")] + /// Dir for simulacrum to write checkpoint files to. To be passed to the offchain indexer and + /// reader. + #[clap(long)] pub data_ingestion_path: Option, - /// TODO (wlmyng): doc comment - #[clap(long = "rest-api-url")] + /// URL for the Sui REST API. To be passed to the offchain indexer and reader. + #[clap(long)] pub rest_api_url: Option, } diff --git a/crates/sui-transactional-test-runner/src/offchain_state.rs b/crates/sui-transactional-test-runner/src/offchain_state.rs index 968262ed805d1..20ebf2b5b978b 100644 --- a/crates/sui-transactional-test-runner/src/offchain_state.rs +++ b/crates/sui-transactional-test-runner/src/offchain_state.rs @@ -10,11 +10,18 @@ pub struct TestResponse { pub service_version: Option, } +/// Trait for interacting with the offchain state of the Sui network. To reduce test flakiness, +/// these methods are used in the `RunGraphqlCommand` to stabilize the off-chain indexed state. #[async_trait] pub trait OffchainStateReader: Send + Sync + 'static { + /// Polls the objects snapshot table until it is within the allowed lag from the latest + /// checkpoint. async fn wait_for_objects_snapshot_catchup(&self, base_timeout: Duration); + /// Polls the checkpoint table until the given checkpoint is committed. async fn wait_for_checkpoint_catchup(&self, checkpoint: u64, base_timeout: Duration); + /// Polls the checkpoint table until the given checkpoint is pruned. async fn wait_for_pruned_checkpoint(&self, checkpoint: u64, base_timeout: Duration); + /// Executes a GraphQL query and returns the response. async fn execute_graphql( &self, query: String, diff --git a/crates/sui-transactional-test-runner/src/test_adapter.rs b/crates/sui-transactional-test-runner/src/test_adapter.rs index fd1d0f4c72ae0..fe2beab216c50 100644 --- a/crates/sui-transactional-test-runner/src/test_adapter.rs +++ b/crates/sui-transactional-test-runner/src/test_adapter.rs @@ -127,11 +127,14 @@ const DEFAULT_CHAIN_START_TIMESTAMP: u64 = 0; /// Extra args related to configuring the indexer and reader. // TODO: the configs are still tied to the indexer crate, eventually we'd like a new command that is -// more agnostic. +// more agnostic pub struct OffChainConfig { pub snapshot_config: SnapshotLagConfig, pub retention_config: Option, + /// Dir for simulacrum to write checkpoint files to. To be passed to the offchain indexer if it + /// uses file-based ingestion. pub data_ingestion_path: PathBuf, + /// URL for the Sui REST API. To be passed to the offchain indexer if it uses the REST API. pub rest_api_url: Option, } @@ -148,6 +151,9 @@ pub struct SuiTestAdapter { pub(crate) staged_modules: BTreeMap, is_simulator: bool, pub(crate) executor: Box, + /// If `is_simulator` is true, the executor will be a `Simulacrum`, and this will be a + /// `RestStateReader` that can be used to spawn the equivalent of a fullnode rest api. This can + /// then be used to serve an indexer that reads from said rest api service. pub read_replica: Option>, /// Configuration for offchain state reader read from the file itself, and can be passed to the /// specific indexing and reader flavor. @@ -165,6 +171,8 @@ struct AdapterInitConfig { reference_gas_price: Option, default_gas_price: Option, flavor: Option, + /// Configuration for offchain state reader read from the file itself, and can be passed to the + /// specific indexing and reader flavor. offchain_config: Option, }