Skip to content

Commit

Permalink
Fix move stdlib warnings and tests (MystenLabs#18101)
Browse files Browse the repository at this point in the history
## Description 

- Move stdlib tests were not properly run
- Fixed issues with move stdlib not being built direclty, leading to
warnings not being caught

## Test plan 

- Ran tests 

---

## Release notes

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

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

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
  • Loading branch information
tnowacki authored Jun 6, 2024
1 parent e0d77fd commit 50d489d
Show file tree
Hide file tree
Showing 86 changed files with 2,232 additions and 2,171 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ async fn test_manage_package_update() {
latest_id,
version_number: 5,
};
let _ = manage_package.execute(Some(lock_file_path.clone()), build_config.config);
let _ = manage_package.execute(Some(&lock_file_path), build_config.config);

let mut lock_file_contents = String::new();
File::open(lock_file_path)
Expand Down
25 changes: 19 additions & 6 deletions crates/sui-framework-tests/src/unit_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,25 @@
use move_cli::base::test::UnitTestResult;
use move_package::LintFlag;
use move_unit_test::UnitTestingConfig;
use std::{fs, io, path::PathBuf};
use std::{
fs, io,
path::{Path, PathBuf},
};
use sui_move::unit_test::run_move_unit_tests;
use sui_move_build::BuildConfig;

const FILTER_ENV: &str = "FILTER";

#[test]
#[cfg_attr(msim, ignore)]
fn run_move_stdlib_unit_tests() {
check_move_unit_tests({
let mut buf = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
buf.extend(["..", "sui-framework", "packages", "move-stdlib"]);
buf
});
}

#[test]
#[cfg_attr(msim, ignore)]
fn run_sui_framework_tests() {
Expand Down Expand Up @@ -69,16 +82,16 @@ fn run_sui_programmability_examples_move_unit_tests() {
buf
};

check_package_builds(path.clone());
check_package_builds(&path);
check_move_unit_tests(path);
}
}

fn check_packages_recursively(path: &PathBuf) -> io::Result<()> {
fn check_packages_recursively(path: &Path) -> io::Result<()> {
for entry in fs::read_dir(path).unwrap() {
let entry = entry?;
if entry.path().join("Move.toml").exists() {
check_package_builds(entry.path());
check_package_builds(&entry.path());
check_move_unit_tests(entry.path());
} else if entry.file_type()?.is_dir() {
check_packages_recursively(&entry.path())?;
Expand All @@ -102,7 +115,7 @@ fn run_examples_move_unit_tests() -> io::Result<()> {
}

/// Ensure packages build outside of test mode.
fn check_package_builds(path: PathBuf) {
fn check_package_builds(path: &Path) {
let mut config = BuildConfig::new_for_testing();
config.config.dev_mode = true;
config.run_bytecode_verifier = true;
Expand All @@ -111,7 +124,7 @@ fn check_package_builds(path: PathBuf) {
config.config.silence_warnings = false;
config.config.lint_flag = LintFlag::LEVEL_DEFAULT;
config
.build(path.clone())
.build(path.to_owned())
.unwrap_or_else(|e| panic!("Building package {}.\nWith error {e}", path.display()));
}

Expand Down
66 changes: 37 additions & 29 deletions crates/sui-framework/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,15 @@ fn main() {
let deepbook_path = packages_path.join("deepbook");
let sui_system_path = packages_path.join("sui-system");
let sui_framework_path = packages_path.join("sui-framework");
let bridge_path_clone = bridge_path.clone();
let deepbook_path_clone = deepbook_path.clone();
let sui_system_path_clone = sui_system_path.clone();
let sui_framework_path_clone = sui_framework_path.clone();
let move_stdlib_path = packages_path.join("move-stdlib");

build_packages(
bridge_path_clone,
deepbook_path_clone,
sui_system_path_clone,
sui_framework_path_clone,
out_dir,
&bridge_path,
&deepbook_path,
&sui_system_path,
&sui_framework_path,
&move_stdlib_path,
&out_dir,
);

println!("cargo:rerun-if-changed=build.rs");
Expand Down Expand Up @@ -84,11 +81,12 @@ fn main() {
}

fn build_packages(
bridge_path: PathBuf,
deepbook_path: PathBuf,
sui_system_path: PathBuf,
sui_framework_path: PathBuf,
out_dir: PathBuf,
bridge_path: &Path,
deepbook_path: &Path,
sui_system_path: &Path,
sui_framework_path: &Path,
stdlib_path: &Path,
out_dir: &Path,
) {
let config = MoveBuildConfig {
generate_docs: true,
Expand All @@ -100,11 +98,12 @@ fn build_packages(
};
debug_assert!(!config.test_mode);
build_packages_with_move_config(
bridge_path.clone(),
deepbook_path.clone(),
sui_system_path.clone(),
sui_framework_path.clone(),
out_dir.clone(),
bridge_path,
deepbook_path,
sui_system_path,
sui_framework_path,
stdlib_path,
out_dir,
"bridge",
"deepbook",
"sui-system",
Expand All @@ -127,6 +126,7 @@ fn build_packages(
deepbook_path,
sui_system_path,
sui_framework_path,
stdlib_path,
out_dir,
"bridge-test",
"deepbook-test",
Expand All @@ -139,11 +139,12 @@ fn build_packages(
}

fn build_packages_with_move_config(
bridge_path: PathBuf,
deepbook_path: PathBuf,
sui_system_path: PathBuf,
sui_framework_path: PathBuf,
out_dir: PathBuf,
bridge_path: &Path,
deepbook_path: &Path,
sui_system_path: &Path,
sui_framework_path: &Path,
stdlib_path: &Path,
out_dir: &Path,
bridge_dir: &str,
deepbook_dir: &str,
system_dir: &str,
Expand All @@ -152,40 +153,47 @@ fn build_packages_with_move_config(
config: MoveBuildConfig,
write_docs: bool,
) {
let stdlib_pkg = BuildConfig {
config: config.clone(),
run_bytecode_verifier: true,
print_diags_to_stderr: false,
}
.build(stdlib_path.to_owned())
.unwrap();
let framework_pkg = BuildConfig {
config: config.clone(),
run_bytecode_verifier: true,
print_diags_to_stderr: false,
}
.build(sui_framework_path)
.build(sui_framework_path.to_owned())
.unwrap();
let system_pkg = BuildConfig {
config: config.clone(),
run_bytecode_verifier: true,
print_diags_to_stderr: false,
}
.build(sui_system_path)
.build(sui_system_path.to_owned())
.unwrap();
let deepbook_pkg = BuildConfig {
config: config.clone(),
run_bytecode_verifier: true,
print_diags_to_stderr: false,
}
.build(deepbook_path)
.build(deepbook_path.to_owned())
.unwrap();
let bridge_pkg = BuildConfig {
config,
run_bytecode_verifier: true,
print_diags_to_stderr: false,
}
.build(bridge_path)
.build(bridge_path.to_owned())
.unwrap();

let move_stdlib = stdlib_pkg.get_stdlib_modules();
let sui_system = system_pkg.get_sui_system_modules();
let sui_framework = framework_pkg.get_sui_framework_modules();
let deepbook = deepbook_pkg.get_deepbook_modules();
let bridge = bridge_pkg.get_bridge_modules();
let move_stdlib = framework_pkg.get_stdlib_modules();

let sui_system_members =
serialize_modules_to_file(sui_system, &out_dir.join(system_dir)).unwrap();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
/// The `ASCII` module defines basic string and char newtypes in Move that verify
/// that characters are valid ASCII, and that strings consist of only valid ASCII characters.
module std::ascii {
use std::option::{Self, Option};

// Allows calling `.to_string()` to convert an `ascii::String` into as `string::String`
public use fun std::string::from_ascii as String.to_string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
/// The `string` module defines the `String` type which represents UTF8 encoded strings.
module std::string {
use std::ascii;
use std::option::{Self, Option};

/// An invalid UTF8 encoding.
const EINVALID_UTF8: u64 = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@

#[test_only]
module std::option_tests {
use std::option;

#[test]
fun option_none_is_none() {
let none = option::none<u64>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@

#[test_only]
module std::vector_tests {
use std::vector;

public struct R has store { }
public struct Droppable has drop {}
public struct NotDroppable {}
Expand Down
8 changes: 4 additions & 4 deletions crates/sui-framework/published_api.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4000,6 +4000,9 @@ get_current_seq_num_and_increment
get_parsed_token_transfer_message
fun
0xb::bridge
to_bytes
public fun
0x1::bcs
print
public fun
0x1::debug
Expand Down Expand Up @@ -4281,7 +4284,4 @@ get_module
0x1::type_name
into_string
public fun
0x1::type_name
to_bytes
public fun
0x1::bcs
0x1::type_name
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,20 @@ Response: {
"data": {
"availableRange": {
"first": {
"digest": "CxPW4F6QmtoZtsRyoRHGrQjp5zGmo46LCsKrR3uwwPbT",
"digest": "8EradUuxwvf5DUGJa58kMeq5S5TMGNUVSRnmefTHJLUV",
"sequenceNumber": 0
},
"last": {
"digest": "CxPW4F6QmtoZtsRyoRHGrQjp5zGmo46LCsKrR3uwwPbT",
"digest": "8EradUuxwvf5DUGJa58kMeq5S5TMGNUVSRnmefTHJLUV",
"sequenceNumber": 0
}
},
"first": {
"digest": "CxPW4F6QmtoZtsRyoRHGrQjp5zGmo46LCsKrR3uwwPbT",
"digest": "8EradUuxwvf5DUGJa58kMeq5S5TMGNUVSRnmefTHJLUV",
"sequenceNumber": 0
},
"last": {
"digest": "CxPW4F6QmtoZtsRyoRHGrQjp5zGmo46LCsKrR3uwwPbT",
"digest": "8EradUuxwvf5DUGJa58kMeq5S5TMGNUVSRnmefTHJLUV",
"sequenceNumber": 0
}
}
Expand All @@ -35,20 +35,20 @@ Response: {
"data": {
"availableRange": {
"first": {
"digest": "CxPW4F6QmtoZtsRyoRHGrQjp5zGmo46LCsKrR3uwwPbT",
"digest": "8EradUuxwvf5DUGJa58kMeq5S5TMGNUVSRnmefTHJLUV",
"sequenceNumber": 0
},
"last": {
"digest": "7yCEVprArX5v8zgncdQ9v2NnhS8HkiibzKTP2oRyP3Sd",
"digest": "GeZhBD8gDKeRjhR9PhKfbx1AqmzoPeGJ8i1eTKVGH9F6",
"sequenceNumber": 2
}
},
"first": {
"digest": "CxPW4F6QmtoZtsRyoRHGrQjp5zGmo46LCsKrR3uwwPbT",
"digest": "8EradUuxwvf5DUGJa58kMeq5S5TMGNUVSRnmefTHJLUV",
"sequenceNumber": 0
},
"last": {
"digest": "7yCEVprArX5v8zgncdQ9v2NnhS8HkiibzKTP2oRyP3Sd",
"digest": "GeZhBD8gDKeRjhR9PhKfbx1AqmzoPeGJ8i1eTKVGH9F6",
"sequenceNumber": 2
}
}
Expand Down
Loading

0 comments on commit 50d489d

Please sign in to comment.