From 422c0947145a565486304a915d2caf472f8f16d3 Mon Sep 17 00:00:00 2001 From: wlmyng <127570466+wlmyng@users.noreply.github.com> Date: Tue, 10 Dec 2024 13:35:18 -0800 Subject: [PATCH] decouple transactional test runner from indexer and graphql flavor (#20373) --- Cargo.lock | 4 + crates/sui-graphql-e2e-tests/Cargo.toml | 2 + crates/sui-graphql-e2e-tests/tests/tests.rs | 82 ++++- .../sui-graphql-rpc/src/test_infra/cluster.rs | 4 + .../sui-transactional-test-runner/Cargo.toml | 2 + .../sui-transactional-test-runner/src/args.rs | 9 + .../sui-transactional-test-runner/src/lib.rs | 5 +- .../src/offchain_state.rs | 30 ++ .../src/test_adapter.rs | 295 ++++++++++-------- .../src/framework.rs | 60 +++- .../src/vm_test_harness.rs | 4 - 11 files changed, 355 insertions(+), 142 deletions(-) create mode 100644 crates/sui-transactional-test-runner/src/offchain_state.rs diff --git a/Cargo.lock b/Cargo.lock index 7b501e0e4a015..ad16cbeac0f61 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13955,6 +13955,8 @@ dependencies = [ name = "sui-graphql-e2e-tests" version = "0.1.0" dependencies = [ + "anyhow", + "async-trait", "datatest-stable", "msim", "sui-graphql-rpc", @@ -15805,6 +15807,7 @@ dependencies = [ "eyre", "fastcrypto", "futures", + "http 1.1.0", "move-binary-format", "move-bytecode-utils", "move-command-line-common", @@ -15836,6 +15839,7 @@ dependencies = [ "telemetry-subscribers", "tempfile", "tokio", + "tracing", "typed-store", ] 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 812d9058fa7a7..4dde7e1677606 100644 --- a/crates/sui-graphql-e2e-tests/tests/tests.rs +++ b/crates/sui-graphql-e2e-tests/tests/tests.rs @@ -3,12 +3,60 @@ #![allow(unused_imports)] #![allow(unused_variables)] -use std::{path::Path, sync::Arc}; +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::{ - run_test_impl, + args::SuiInitArgs, + 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(result.http_headers_without_date()), + response_body: result.response_body_json_pretty(), + service_version: result.graphql_version().ok(), + }) + } +} + datatest_stable::harness!( run_test, "tests", @@ -24,7 +72,35 @@ 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?; + // 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(), + Some(offchain_config.snapshot_config.clone()), + offchain_config.retention_config.clone(), + offchain_config.data_ingestion_path.clone(), + ) + .await; + + let cluster_arc = Arc::new(cluster); + + adapter.with_offchain_reader(Box::new(OffchainReaderForAdapter { + cluster: cluster_arc.clone(), + })); + + run_tasks_with_adapter(path, adapter, output).await?; + + match Arc::try_unwrap(cluster_arc) { + Ok(cluster) => cluster.cleanup_resources().await, + Err(_) => panic!("Still other Arc references!"), + } } Ok(()) } diff --git a/crates/sui-graphql-rpc/src/test_infra/cluster.rs b/crates/sui-graphql-rpc/src/test_infra/cluster.rs index 0a05d0313e8ce..83610f6c3f011 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_rpc_api::RpcService::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/Cargo.toml b/crates/sui-transactional-test-runner/Cargo.toml index ec5a0ddf3ea2b..fbfa441d7a143 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 @@ -25,6 +26,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 95f32682d1205..0946e7169cbd8 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,13 @@ pub struct SuiInitArgs { /// the indexer. #[clap(long = "epochs-to-keep")] pub epochs_to_keep: Option, + /// Dir for simulacrum to write checkpoint files to. To be passed to the offchain indexer and + /// reader. + #[clap(long)] + pub data_ingestion_path: Option, + /// URL for the Sui REST API. To be passed to the offchain indexer and reader. + #[clap(long)] + pub rest_api_url: Option, } #[derive(Debug, clap::Parser)] diff --git a/crates/sui-transactional-test-runner/src/lib.rs b/crates/sui-transactional-test-runner/src/lib.rs index 1c9453725bbc2..760c65d1f3c4c 100644 --- a/crates/sui-transactional-test-runner/src/lib.rs +++ b/crates/sui-transactional-test-runner/src/lib.rs @@ -4,11 +4,14 @@ //! This module contains the transactional test runner instantiation for the Sui adapter pub mod args; +pub mod offchain_state; 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; 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..20ebf2b5b978b --- /dev/null +++ b/crates/sui-transactional-test-runner/src/offchain_state.rs @@ -0,0 +1,30 @@ +// 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, +} + +/// 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, + 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 5db7cede09962..b0880794cb4d7 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::offchain_state::OffchainStateReader; use crate::simulator_persisted_store::PersistedStore; use crate::{args::*, programmable_transaction_test_parser::parser::ParsedCommand}; use crate::{TransactionalAdapter, ValidatorWithFullnode}; @@ -39,6 +40,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}, @@ -48,8 +50,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::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}; @@ -66,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, RpcStateReader}; use sui_types::transaction::Command; use sui_types::transaction::ProgrammableTransaction; use sui_types::utils::to_sender_signed_transaction_with_multi_signers; @@ -124,6 +125,19 @@ 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, + /// 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, +} + pub struct SuiTestAdapter { pub(crate) compiled_state: CompiledState, /// For upgrades: maps an upgraded package name to the original package name. @@ -136,8 +150,30 @@ pub struct SuiTestAdapter { gas_price: u64, pub(crate) staged_modules: BTreeMap, is_simulator: bool, - pub(crate) cluster: Option, pub(crate) executor: Box, + /// If `is_simulator` is true, the executor will be a `Simulacrum`, and this will be a + /// `RpcStateReader` 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. + pub offchain_config: Option, + /// A trait encapsulating methods to interact with offchain state. + pub offchain_reader: 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, + /// Configuration for offchain state reader read from the file itself, and can be passed to the + /// specific indexing and reader flavor. + offchain_config: Option, } pub(crate) struct StagedPackage { @@ -167,6 +203,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; @@ -210,12 +319,7 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { fn default_syntax(&self) -> SyntaxChoice { 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( default_syntax: SyntaxChoice, pre_compiled_deps: Option>, @@ -234,7 +338,7 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { ); // Unpack the init arguments - let ( + let AdapterInitConfig { additional_mapping, account_names, protocol_config, @@ -242,80 +346,11 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { custom_validator_account, reference_gas_price, default_gas_price, - snapshot_config, flavor, - epochs_to_keep, - ) = 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, - }, - )) => { - 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"); - } - - ( - map, - accounts, - protocol_config, - simulator, - custom_validator_account, - reference_gas_price, - default_gas_price, - snapshot_config, - flavor, - epochs_to_keep, - ) - } - None => { - let protocol_config = ProtocolConfig::get_for_max_version_UNSAFE(); - ( - BTreeMap::new(), - BTreeSet::new(), - protocol_config, - false, - false, - None, - None, - SnapshotLagConfig::default(), - None, - None, - ) - } + offchain_config, + } = match task_opt.map(|t| t.command) { + Some((init_cmd, sui_args)) => AdapterInitConfig::from_args(init_cmd, sui_args), + None => AdapterInitConfig::default(), }; let ( @@ -327,13 +362,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, @@ -341,8 +371,11 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { &protocol_config, custom_validator_account, reference_gas_price, - snapshot_config, - retention_config, + offchain_config + .as_ref() + .unwrap() + .data_ingestion_path + .clone(), ) .await } else { @@ -354,8 +387,11 @@ impl<'a> MoveTestAdapter<'a> for SuiTestAdapter { let mut test_adapter = Self { is_simulator, - cluster, + // This is opt-in and instantiated later + offchain_reader: None, executor, + offchain_config, + read_replica, compiled_state: CompiledState::new( named_address_mapping, pre_compiled_deps, @@ -576,40 +612,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())?; - let cluster = self.cluster.as_ref().unwrap(); + let offchain_reader = self + .offchain_reader + .as_ref() + .ok_or_else(|| anyhow::anyhow!("Offchain reader not set"))?; let highest_checkpoint = self.executor.get_latest_checkpoint_sequence_number()?; - cluster + offchain_reader .wait_for_checkpoint_catchup(highest_checkpoint, Duration::from_secs(60)) .await; - cluster - .wait_for_objects_snapshot_catchup(Duration::from_secs(180)) - .await; + // wait_for_objects_snapshot_catchup(graphql_client, 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), - ) + if let Some(checkpoint_to_prune) = wait_for_checkpoint_pruned { + 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 = cluster - .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.unwrap())); } 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"))) } @@ -1163,6 +1199,10 @@ fn merge_output(left: Option, right: Option) -> Option { } impl<'a> SuiTestAdapter { + pub fn with_offchain_reader(&mut self, offchain_reader: Box) { + self.offchain_reader = Some(offchain_reader); + } + pub fn is_simulator(&self) -> bool { self.is_simulator } @@ -1909,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); @@ -2055,7 +2111,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(); @@ -2121,12 +2177,11 @@ async fn init_sim_executor( protocol_config: &ProtocolConfig, custom_validator_account: bool, reference_gas_price: Option, - snapshot_config: SnapshotLagConfig, - retention_config: Option, + data_ingestion_path: PathBuf, ) -> ( Box, AccountSetup, - Option, + Option>, ) { // Initial list of named addresses with specified values let mut named_address_mapping = NAMED_ADDRESSES.clone(); @@ -2188,16 +2243,8 @@ async fn init_sim_executor( 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; + sim.set_data_ingestion_path(data_ingestion_path.clone()); // Get the actual object values from the simulator for (name, (addr, kp)) in account_kps { @@ -2256,7 +2303,7 @@ async fn init_sim_executor( account_objects, accounts, }, - Some(cluster), + Some(Arc::new(read_replica)), ) } 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..8bf0a4c666337 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 @@ -125,7 +125,6 @@ pub trait MoveTestAdapter<'a>: Sized + Send { path: &Path, ) -> (Self, Option); - async fn cleanup_resources(&mut self) -> Result<()>; async fn publish_modules( &mut self, modules: Vec, @@ -704,7 +703,9 @@ 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`. Returns the adapter and the output string. +pub async fn create_adapter<'a, Adapter>( path: &Path, fully_compiled_program_opt: Option>, ) -> Result<(String, Adapter), Box> @@ -756,23 +757,62 @@ where None } }; - let (mut adapter, result_opt) = + + let (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 - adapter.cleanup_resources().await?; return Err(Box::new(e)); } } + + 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, + mut output: String, +) -> Result<()> +where + Adapter: MoveTestAdapter<'a>, + Adapter::ExtraInitArgs: Debug, + Adapter::ExtraPublishArgs: Debug, + Adapter::ExtraValueArgs: Debug, + Adapter::ExtraRunArgs: Debug, + Adapter::Subcommand: Debug, +{ + let mut tasks = taskify::< + TaskCommand< + Adapter::ExtraInitArgs, + Adapter::ExtraPublishArgs, + Adapter::ExtraValueArgs, + Adapter::ExtraRunArgs, + Adapter::Subcommand, + >, + >(path)? + .into_iter() + .collect::>(); + assert!(!tasks.is_empty()); + + // Pop off init command if present, this has already been handled before this function was + // called to initialize the adapter + 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; } - adapter.cleanup_resources().await?; - Ok((output, adapter)) + + handle_expected_output(path, output)?; + 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>, @@ -785,8 +825,8 @@ where Adapter::ExtraRunArgs: Debug, Adapter::Subcommand: Debug, { - let output = handle_actual_output::(path, fully_compiled_program_opt).await?; - handle_expected_output(path, output.0)?; + let (output, adapter) = create_adapter::(path, fully_compiled_program_opt).await?; + run_tasks_with_adapter(path, adapter, output).await?; Ok(()) } diff --git a/external-crates/move/crates/move-transactional-test-runner/src/vm_test_harness.rs b/external-crates/move/crates/move-transactional-test-runner/src/vm_test_harness.rs index 05b3b5d281926..333905bbc24d6 100644 --- a/external-crates/move/crates/move-transactional-test-runner/src/vm_test_harness.rs +++ b/external-crates/move/crates/move-transactional-test-runner/src/vm_test_harness.rs @@ -65,10 +65,6 @@ impl<'a> MoveTestAdapter<'a> for SimpleVMTestAdapter { self.default_syntax } - async fn cleanup_resources(&mut self) -> Result<()> { - Ok(()) - } - async fn init( default_syntax: SyntaxChoice, pre_compiled_deps: Option>,