Skip to content

Commit

Permalink
Remove hard coded DEFAULT_EXECUTION_BOUND as tests can provide their …
Browse files Browse the repository at this point in the history
…own gas budget parameters (anza-xyz#26)

Follow up from anza-xyz#24

Fixes: anza-xyz#20
  • Loading branch information
ksolana authored Apr 29, 2024
1 parent 465866a commit 855fd31
Show file tree
Hide file tree
Showing 16 changed files with 72 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::{env, path::Path};
use move_command_line_common::{env::read_bool_env_var, testing::EXP_EXT};
use move_prover_test_utils::baseline_test::verify_or_update_baseline;
use move_stdlib::move_stdlib_files;
use move_unit_test::{UnitTestingConfig, DEFAULT_EXECUTION_BOUND_SOLANA};
use move_unit_test::UnitTestingConfig;

fn test_runner(path: &Path) -> datatest_stable::Result<()> {
env::set_var("NO_COLOR", "1");
Expand All @@ -29,7 +29,7 @@ fn test_runner(path: &Path) -> datatest_stable::Result<()> {

let test_plan = config.build_test_plan().unwrap();
let mut buffer = vec![];
config.run_and_report_unit_tests(test_plan, None, None, &mut buffer, DEFAULT_EXECUTION_BOUND_SOLANA)?;
config.run_and_report_unit_tests(test_plan, None, None, &mut buffer)?;
let output = String::from_utf8(buffer)?;

let baseline_path = path.with_extension(EXP_EXT);
Expand Down
4 changes: 2 additions & 2 deletions external-crates/move/crates/move-cli/src/base/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use move_compiler::{
};
use move_coverage::coverage_map::{output_map_to_file, CoverageMap};
use move_package::{compilation::build_plan::BuildPlan, BuildConfig};
use move_unit_test::{UnitTestingConfig, DEFAULT_EXECUTION_BOUND_SOLANA_STDLIB_TEST};
use move_unit_test::UnitTestingConfig;
use move_vm_test_utils::gas_schedule::CostTable;
use std::{
collections::HashMap,
Expand Down Expand Up @@ -241,7 +241,7 @@ pub fn run_move_unit_tests<W: Write + Send>(
// Run the tests. If any of the tests fail, then we don't produce a coverage report, so cleanup
// the trace files.
if !unit_test_config
.run_and_report_unit_tests(test_plan, Some(natives), cost_table, writer, DEFAULT_EXECUTION_BOUND_SOLANA_STDLIB_TEST)
.run_and_report_unit_tests(test_plan, Some(natives), cost_table, writer)
.unwrap()
.1
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
Command `disassemble --Xdebug --package Test --name c`:
// Move bytecode v6
module 2.c {
use 0000000000000000000000000000000000000000000000000000000000000002::b;
use 0000000000000000000000000000000000000000000000000000000000000003::d;
use 0000000000000000000000000000000000000000000000000000000000000001::a;
use 0000000000000000000000000000000000000000000000000000000000000004::e;
use 0x0000000000000000000000000000000000000000000000000000000000000002::b;
use 0x0000000000000000000000000000000000000000000000000000000000000003::d;
use 0x0000000000000000000000000000000000000000000000000000000000000001::a;
use 0x0000000000000000000000000000000000000000000000000000000000000004::e;


struct B {
Expand Down Expand Up @@ -208,10 +208,10 @@ CompiledModule {
),
],
address_identifiers: [
0000000000000000000000000000000000000000000000000000000000000002,
0000000000000000000000000000000000000000000000000000000000000001,
0000000000000000000000000000000000000000000000000000000000000003,
0000000000000000000000000000000000000000000000000000000000000004,
0x0000000000000000000000000000000000000000000000000000000000000002,
0x0000000000000000000000000000000000000000000000000000000000000001,
0x0000000000000000000000000000000000000000000000000000000000000003,
0x0000000000000000000000000000000000000000000000000000000000000004,
],
constant_pool: [],
metadata: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ Options:
Installation directory for compiled artifacts. Defaults to current directory
--force
Force recompilation of all packages
--arch <ARCHITECTURE>

--fetch-deps-only
Only fetch dependency repos to MOVE_HOME
--skip-fetch-latest-git-deps
Expand Down
26 changes: 18 additions & 8 deletions external-crates/move/crates/move-compiler/src/unit_test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub struct TestCase {
pub test_name: TestName,
pub arguments: Vec<MoveValue>,
pub expected_failure: Option<ExpectedFailure>,
pub gas_budget: Option<GasBudgetParams>,
pub gas_budget: Option<SolanaGasBudgetParams>,
}

#[derive(Debug, Clone)]
Expand All @@ -47,13 +47,6 @@ pub enum ExpectedFailure {
ExpectedWithError(ExpectedMoveError),
}

#[derive(Debug, Clone)]
pub struct GasBudgetParams {
pub compute_budget:u64,
pub heap_size:u64,
pub max_call_depth:u64,
}

#[derive(Debug, Clone, Ord, PartialOrd, PartialEq, Eq)]
pub struct ExpectedMoveError(
pub StatusCode,
Expand All @@ -66,6 +59,23 @@ pub struct ExpectedMoveErrorDisplay<'a> {
is_past_tense: bool,
}

#[derive(Debug, Clone)]
pub struct SolanaGasBudgetParams {
pub compute_budget: u64,
pub heap_size: usize,
pub max_call_depth: usize,
}

impl SolanaGasBudgetParams {
pub fn new() -> Self {
SolanaGasBudgetParams {
compute_budget: i64::MAX as u64,
heap_size: 10000000,
max_call_depth: 8192,
}
}
}

impl ModuleTestPlan {
pub fn new(
addr: &NumericalAddress,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
known_attributes::TestingAttribute, unique_map::UniqueMap, CompilationEnv, Identifier,
NumericalAddress,
},
unit_test::{ExpectedFailure, ExpectedMoveError, ModuleTestPlan, TestCase},
unit_test::{ExpectedFailure, ExpectedMoveError, ModuleTestPlan, TestCase, SolanaGasBudgetParams},
};
use move_core_types::{
account_address::AccountAddress as MoveAddress, language_storage::ModuleId,
Expand All @@ -24,8 +24,6 @@ use move_ir_types::location::Loc;
use move_symbol_pool::Symbol;
use std::collections::BTreeMap;

use super::GasBudgetParams;

struct Context<'env> {
env: &'env mut CompilationEnv,
constants: UniqueMap<ModuleIdent, UniqueMap<ConstantName, (Loc, Option<u64>)>>,
Expand Down Expand Up @@ -184,7 +182,7 @@ fn build_test_info<'func>(
None => None,
Some(abort_attribute) => parse_failure_attribute(context, abort_attribute),
};
let gas_budget: Option<GasBudgetParams> = parse_gas_budget_attribute(context, gas_budget_attr);
let gas_budget = parse_gas_budget_attribute(context, gas_budget_attr);

Some(TestCase {
test_name: fn_name.to_string(),
Expand Down Expand Up @@ -491,12 +489,8 @@ fn parse_failure_attribute(
}
}

fn parse_gas_budget_attribute(context: &mut Context, gas_budget_attr: Option<&E::Attribute>) -> Option<GasBudgetParams> {
let mut assigned_gas_budget = GasBudgetParams {
compute_budget:1,
heap_size: 1,
max_call_depth: 1,
};
fn parse_gas_budget_attribute(context: &mut Context, gas_budget_attr: Option<&E::Attribute>) -> Option<SolanaGasBudgetParams> {
let mut assigned_gas_budget = SolanaGasBudgetParams::new();
if gas_budget_attr.is_none(){
return Some(assigned_gas_budget);
}
Expand Down Expand Up @@ -546,21 +540,22 @@ fn parse_gas_budget_attribute(context: &mut Context, gas_budget_attr: Option<&E:
}
while !gas_budget_kind_vec.is_empty() {
let (gas_budget_kind, (attr_loc, attr)) = gas_budget_kind_vec.pop().unwrap();
match gas_budget_kind.as_str() {
let gb = gas_budget_kind.as_str();
match gb {
TestingAttribute::GAS_BUDGET_COMPUTE_UNIT_LIMIT => {
let u = get_assigned_attribute_as_u64(context, TestingAttribute::GAS_BUDGET_COMPUTE_UNIT_LIMIT, attr_loc, attr)?;
let u = get_assigned_attribute_as_u64(context, gb, attr_loc, attr)?;
// TODO: Do some sanity check that u shouldn't be larger than a max value.
assigned_gas_budget.compute_budget = u;
}
TestingAttribute::GAS_BUDGET_HEAP_SIZE => {
let u = get_assigned_attribute_as_u64(context, TestingAttribute::GAS_BUDGET_HEAP_SIZE, attr_loc, attr)?;
let u = get_assigned_attribute_as_u64(context, gb, attr_loc, attr)?;
// TODO: Do some sanity check that u shouldn't be larger than a max value.
assigned_gas_budget.heap_size = u;
assigned_gas_budget.heap_size = u as usize;
}
TestingAttribute::GAS_BUDGET_MAX_CALL_DEPTH => {
let u = get_assigned_attribute_as_u64(context, TestingAttribute::GAS_BUDGET_MAX_CALL_DEPTH, attr_loc, attr)?;
let u = get_assigned_attribute_as_u64(context, gb, attr_loc, attr)?;
// TODO: Do some sanity check that u shouldn't be larger than a max value.
assigned_gas_budget.max_call_depth = u;
assigned_gas_budget.max_call_depth = u as usize;
}
_ => return None,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ module std::bit_vector_tests {
}

#[test]
#[gas_budget(compute_unit_limit=10000000, heap_size=1000, max_call_depth=10)]
#[gas_budget(compute_unit_limit=10000000, heap_size=40000, max_call_depth=10)]
fun test_set_bit_and_index_basic() {
test_bitvector_set_unset_of_size(8)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use move_command_line_common::files::find_filenames;
use move_vm_runtime::native_functions::NativeFunctionTable;
use move_vm_test_utils::gas_schedule::CostTable;

use crate::{UnitTestingConfig, DEFAULT_EXECUTION_BOUND};
use crate::UnitTestingConfig;

pub fn run_tests_with_config_and_filter(
mut config: UnitTestingConfig,
Expand Down Expand Up @@ -40,7 +40,6 @@ pub fn run_tests_with_config_and_filter(
native_function_table,
cost_table,
std::io::stdout(),
DEFAULT_EXECUTION_BOUND
)
.expect("Failed to execute tests");

Expand Down
9 changes: 4 additions & 5 deletions external-crates/move/crates/move-unit-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ impl UnitTestingConfig {
native_function_table: Option<NativeFunctionTable>,
cost_table: Option<CostTable>,
writer: W,
gas_limit: u64,
) -> Result<(W, bool)> {
let shared_writer = Mutex::new(writer);

Expand All @@ -217,14 +216,14 @@ impl UnitTestingConfig {
}

writeln!(shared_writer.lock().unwrap(), "Running Move unit tests")?;
let (num_threads, cgas_limit) = if cfg!(feature = "solana-backend") {
(1, gas_limit) // enforce single threaded execution for Solana, as llvm-sys is not re-entrant.
let num_threads = if cfg!(feature = "solana-backend") {
1 // enforce single threaded execution for Solana, as llvm-sys is not re-entrant.
} else {
(self.num_threads, self.gas_limit.unwrap())
self.num_threads
};

let mut test_runner = TestRunner::new(
cgas_limit,
self.gas_limit.unwrap_or(DEFAULT_EXECUTION_BOUND),
num_threads,
self.check_stackless_vm,
self.verbose,
Expand Down
4 changes: 2 additions & 2 deletions external-crates/move/crates/move-unit-test/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
// SPDX-License-Identifier: Apache-2.0

use clap::*;
use move_unit_test::{UnitTestingConfig, DEFAULT_EXECUTION_BOUND};
use move_unit_test::UnitTestingConfig;

pub fn main() {
let args = UnitTestingConfig::parse();

let test_plan = args.build_test_plan();
if let Some(test_plan) = test_plan {
args.run_and_report_unit_tests(test_plan, None, None, std::io::stdout(), DEFAULT_EXECUTION_BOUND)
args.run_and_report_unit_tests(test_plan, None, None, std::io::stdout())
.unwrap();
}
}
4 changes: 2 additions & 2 deletions external-crates/move/crates/move-unit-test/src/test_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,10 +529,10 @@ impl SharedTestingConfig {
}

let gen_options = move_to_solana::options::Options::default();
// TODO: Get the compute budget from the test file. anza-xyz/sui/issues/20
let compute_budget = move_to_solana::runner::compute_budget(self.execution_bound);

for (function_name, test_info) in &test_plan.tests {
let compute_budget = move_to_solana::runner::compute_budget(&test_info.gas_budget);

let shared_object = match move_to_solana::run_for_unit_test(
&gen_options,
&model,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use move_command_line_common::testing::{
add_update_baseline_fix, format_diff, read_env_update_baseline, EXP_EXT,
};
use move_unit_test::{self, UnitTestingConfig, DEFAULT_EXECUTION_BOUND, DEFAULT_EXECUTION_BOUND_SOLANA};
use move_unit_test::{self, UnitTestingConfig};
use regex::RegexBuilder;
use std::{
fs,
Expand Down Expand Up @@ -34,12 +34,12 @@ fn run_test_with_modifiers(

if !cfg!(feature = "solana-backend") {
results.push((
unit_test_config.run_and_report_unit_tests(test_plan.unwrap(), None, None, buffer, DEFAULT_EXECUTION_BOUND)?,
unit_test_config.run_and_report_unit_tests(test_plan.unwrap(), None, None, buffer)?,
path.with_extension(EXP_EXT),
));
} else {
results.push((
unit_test_config.run_and_report_unit_tests(test_plan.unwrap(), None, None, buffer, DEFAULT_EXECUTION_BOUND_SOLANA)?,
unit_test_config.run_and_report_unit_tests(test_plan.unwrap(), None, None, buffer)?,
path.with_extension(format!("{}.{}", "solana", EXP_EXT)),
));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ module 0x42::m {

#[test]
#[expected_failure(out_of_gas, location=Self)]
#[gas_budget(compute_unit_limit=10000000, heap_size=40000, max_call_depth=10)]
fun t4() {
loop {}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ module 0x42::m {
fun t0() {}

#[test]
#[expected_failure(arithmetic_error, location=Self)]
#[expected_failure(out_of_gas, location=Self)]
#[gas_budget(compute_unit_limit=10000000, heap_size=40000, max_call_depth=10)]
fun t1() {
loop {}
}

#[test]
#[expected_failure(out_of_gas, location=Self)]
#[expected_failure(arithmetic_error, location=Self)]
fun t2() {
0 - 1;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
address 0x1 {
module M {
#[test]
#[gas_budget(compute_unit_limit=10000000, heap_size=40000, max_call_depth=10)]
fun timeout_fail() {
while (true) {}
}

#[test]
#[expected_failure]
#[gas_budget(compute_unit_limit=10000000, heap_size=40000, max_call_depth=10)]
fun timeout_fail_with_expected_failure() {
while (true) {}
}
Expand Down
14 changes: 10 additions & 4 deletions external-crates/move/solana/move-to-solana/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ use std::{
time::{Duration, Instant},
};

use move_compiler::unit_test::SolanaGasBudgetParams;

#[derive(Debug, Serialize, Deserialize, Clone)]
pub struct AccountInfo {
pub key: String,
Expand Down Expand Up @@ -67,11 +69,15 @@ pub struct ExecuteResult {
pub log: String,
}

pub fn compute_budget(execution_bound: u64) -> ComputeBudget {
pub fn compute_budget(gas_budget_params: &Option<SolanaGasBudgetParams>) -> ComputeBudget {
let gb = match gas_budget_params {
Some(gb) => gb.clone(),
None => SolanaGasBudgetParams::new(),
};
ComputeBudget {
compute_unit_limit: execution_bound,
heap_size: Some(10000000),
max_call_depth: 8192,
compute_unit_limit: gb.compute_budget,
heap_size: Some(gb.heap_size),
max_call_depth: gb.max_call_depth,
..ComputeBudget::default()
}
}
Expand Down

0 comments on commit 855fd31

Please sign in to comment.