Skip to content

Commit

Permalink
fix bytecode deps for unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Zekun Wang committed Nov 12, 2024
1 parent 9a6fc2a commit 43cb926
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 24 deletions.
6 changes: 3 additions & 3 deletions aptos-move/framework/src/built_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use codespan_reporting::{
use itertools::Itertools;
use move_binary_format::{file_format_common::VERSION_7, CompiledModule};
use move_command_line_common::files::MOVE_COMPILED_EXTENSION;
use move_compiler::compiled_unit::{CompiledUnit, NamedCompiledModule};
use move_compiler::{compiled_unit::{CompiledUnit, NamedCompiledModule}, shared::NumericalAddress};
use move_compiler_v2::{external_checks::ExternalChecks, options::Options, Experiment};
use move_core_types::{language_storage::ModuleId, metadata::Metadata};
use move_model::{
Expand Down Expand Up @@ -507,8 +507,8 @@ impl BuiltPackage {
self.package
.bytecode_deps
.iter()
.map(|(name, address)| PackageDep {
account: address.into_inner(),
.map(|(name, module)| PackageDep {
account: NumericalAddress::from_account_address(*module.self_addr()).into_inner(),
package_name: name.as_str().to_string(),
}),
)
Expand Down
21 changes: 19 additions & 2 deletions third_party/move/move-compiler/src/unit_test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,35 @@ use crate::{
diagnostics::FilesSourceText,
shared::NumericalAddress,
};
use move_binary_format::CompiledModule;
use move_core_types::{
account_address::AccountAddress, identifier::Identifier, language_storage::ModuleId,
value::MoveValue, vm_status::StatusCode,
};
use move_symbol_pool::Symbol;
use std::{collections::BTreeMap, fmt};

pub mod filter_test_members;
pub mod plan_builder;

pub type TestName = String;

#[derive(Debug, Clone)]
pub enum NamedOrBytecodeModule {
// Compiled from source
Named(NamedCompiledModule),
// Bytecode dependency
Bytecode(CompiledModule),
}

#[derive(Debug, Clone)]
pub struct TestPlan {
pub files: FilesSourceText,
pub module_tests: BTreeMap<ModuleId, ModuleTestPlan>,
pub module_info: BTreeMap<ModuleId, NamedCompiledModule>,
// `NamedCompiledModule` for compiled modules with source,
// `CompiledModule` for modules with bytecode only
// pub module_info: BTreeMap<ModuleId, Either<NamedCompiledModule, CompiledModule>>,
pub module_info: BTreeMap<ModuleId, NamedOrBytecodeModule>,
}

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -85,6 +98,7 @@ impl TestPlan {
tests: Vec<ModuleTestPlan>,
files: FilesSourceText,
units: Vec<AnnotatedCompiledUnit>,
bytecode_modules: BTreeMap<Symbol, CompiledModule>,
) -> Self {
let module_tests: BTreeMap<_, _> = tests
.into_iter()
Expand All @@ -97,12 +111,15 @@ impl TestPlan {
if let AnnotatedCompiledUnit::Module(annot_module) = unit {
Some((
annot_module.named_module.module.self_id(),
annot_module.named_module,
NamedOrBytecodeModule::Named(annot_module.named_module),
))
} else {
None
}
})
.chain(bytecode_modules.into_iter().map(|(_name, module)| {
(module.self_id(), NamedOrBytecodeModule::Bytecode(module))
}))
.collect();

Self {
Expand Down
4 changes: 2 additions & 2 deletions third_party/move/tools/move-cli/src/base/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ pub fn run_move_unit_tests_with_factory<W: Write + Send, F: UnitTestFactory + Se
// Move package system, to first grab the compilation env, construct the test plan from it, and
// then save it, before resuming the rest of the compilation and returning the results and
// control back to the Move package system.
let (_compiled_package, model_opt) = build_plan.compile_with_driver(
let (compiled_package, model_opt) = build_plan.compile_with_driver(
writer,
&build_config.compiler_config,
vec![],
Expand Down Expand Up @@ -306,7 +306,7 @@ pub fn run_move_unit_tests_with_factory<W: Write + Send, F: UnitTestFactory + Se
files.extend(dep_file_map);
let test_plan = test_plan.unwrap();
let no_tests = test_plan.is_empty();
let test_plan = TestPlan::new(test_plan, files, units);
let test_plan = TestPlan::new(test_plan, files, units, compiled_package.bytecode_deps);

let trace_path = pkg_path.join(".trace");
let coverage_map_path = pkg_path
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pub struct CompiledPackage {
/// The output compiled bytecode for dependencies
pub deps_compiled_units: Vec<(PackageName, CompiledUnitWithSource)>,
/// Bytecode dependencies of this compiled package
pub bytecode_deps: BTreeMap<PackageName, NumericalAddress>,
pub bytecode_deps: BTreeMap<PackageName, CompiledModule>,

// Optional artifacts from compilation
//
Expand Down Expand Up @@ -809,8 +809,8 @@ impl CompiledPackage {
.flat_map(|package| {
let name = package.name.unwrap();
package.paths.iter().map(move |pkg_path| {
get_addr_from_module_in_package(name, pkg_path.as_str())
.map(|addr| (name, addr))
get_module_in_package(name, pkg_path.as_str())
.map(|module| (name, module))
})
})
.try_collect()?,
Expand Down Expand Up @@ -1164,8 +1164,8 @@ pub fn build_and_report_no_exit_v2_driver(
))
}

/// Returns the address of the module
fn get_addr_from_module_in_package(pkg_name: Symbol, pkg_path: &str) -> Result<NumericalAddress> {
/// Returns the deserialized module from the bytecode file
fn get_module_in_package(pkg_name: Symbol, pkg_path: &str) -> Result<CompiledModule> {
// Read the bytecode file
let mut bytecode = Vec::new();
std::fs::File::open(pkg_path)
Expand All @@ -1181,5 +1181,4 @@ fn get_addr_from_module_in_package(pkg_name: Symbol, pkg_path: &str) -> Result<N
pkg_name
))
})
.map(|module| NumericalAddress::from_account_address(*module.self_addr()))
}
2 changes: 1 addition & 1 deletion third_party/move/tools/move-unit-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ impl UnitTestingConfig {
let (units, warnings) =
diagnostics::unwrap_or_report_diagnostics(&files, compilation_result);
diagnostics::report_warnings(&files, warnings);
test_plan.map(|tests| TestPlan::new(tests, files, units))
test_plan.map(|tests| TestPlan::new(tests, files, units, todo!()))
}

/// Build a test plan from a unit test config
Expand Down
22 changes: 14 additions & 8 deletions third_party/move/tools/move-unit-test/src/test_reporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use move_command_line_common::{env::read_bool_env_var, files::FileHash};
pub use move_compiler::unit_test::ExpectedMoveError as MoveError;
use move_compiler::{
diagnostics::{self, Diagnostic, Diagnostics},
unit_test::{ModuleTestPlan, TestName, TestPlan},
unit_test::{ModuleTestPlan, NamedOrBytecodeModule, TestName, TestPlan},
};
use move_core_types::{effects::ChangeSet, language_storage::ModuleId, vm_status::StatusType};
use move_ir_types::location::Loc;
Expand Down Expand Up @@ -357,7 +357,10 @@ impl TestFailure {
None => return "\tmalformed stack trace (no module ID)".to_string(),
};
let named_module = match test_plan.module_info.get(module_id) {
Some(v) => v,
Some(NamedOrBytecodeModule::Named(v)) => v,
Some(NamedOrBytecodeModule::Bytecode(_)) => {
return "\tno source map for bytecode module".to_string()
},
None => return "\tmalformed stack trace (no module)".to_string(),
};
let function_source_map =
Expand Down Expand Up @@ -408,12 +411,15 @@ impl TestFailure {
let mut diags = match vm_error.location() {
Location::Module(module_id) => {
let diag_opt = vm_error.offsets().first().and_then(|(fdef_idx, offset)| {
let function_source_map = test_plan
.module_info
.get(module_id)?
.source_map
.get_function_source_map(*fdef_idx)
.ok()?;
let function_source_map = match test_plan.module_info.get(module_id)? {
NamedOrBytecodeModule::Named(named_compiled_module) => {
named_compiled_module
.source_map
.get_function_source_map(*fdef_idx)
.ok()?
},
NamedOrBytecodeModule::Bytecode(_compiled_module) => return None,
};
let loc = function_source_map.get_code_location(*offset).unwrap();
let msg = format!("In this function in {}", format_module_id(module_id));
// TODO(tzakian) maybe migrate off of move-langs diagnostics?
Expand Down
7 changes: 5 additions & 2 deletions third_party/move/tools/move-unit-test/src/test_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use anyhow::Result;
use colored::*;
use move_binary_format::{errors::VMResult, file_format::CompiledModule};
use move_bytecode_utils::Modules;
use move_compiler::unit_test::{ExpectedFailure, ModuleTestPlan, TestCase, TestPlan};
use move_compiler::unit_test::{ExpectedFailure, ModuleTestPlan, NamedOrBytecodeModule, TestCase, TestPlan};
use move_core_types::{
account_address::AccountAddress,
effects::{ChangeSet, Op},
Expand Down Expand Up @@ -127,7 +127,10 @@ impl TestRunner {
.values()
.map(|(filepath, _)| filepath.to_string())
.collect();
let modules = tests.module_info.values().map(|info| &info.module);
let modules = tests.module_info.values().map(|info| match info {
NamedOrBytecodeModule::Named(named_compiled_module) => &named_compiled_module.module,
NamedOrBytecodeModule::Bytecode(compiled_module) => compiled_module,
});
let mut starting_storage_state = setup_test_storage(modules)?;
if let Some(genesis_state) = genesis_state {
starting_storage_state.apply(genesis_state)?;
Expand Down

0 comments on commit 43cb926

Please sign in to comment.