feat: unify test utils to a single feature (#1755)

* feat: unify test utils to a single feature * Update changelog * Also cover arbitrary * Fixes
lambdaclass · May 8, 2024 · 5da72d9 · 5da72d9
1 parent a896797
commit 5da72d9
Show file tree

Hide file tree

Showing 21 changed files with 81 additions and 84 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 #### Upcoming Changes
 
+* feat: unify `arbitrary`, `hooks`, `print` and `skip_next_instruction_hint` features as a single `test_utils` feature [#1755](https://github.com/lambdaclass/cairo-vm/pull/1755)
+  * BREAKING: removed the above features
+
 * bugfix: cairo1-run CLI: Set finalize_builtins to true when using --air_public_input flag [#1744](https://github.com/lambdaclass/cairo-vm/pull/1752)
 
 * feat: Add hint `U256InvModN` to `Cairo1HintProcessor` [#1744](https://github.com/lambdaclass/cairo-vm/pull/1744)

diff --git a/cairo-vm-tracer/Cargo.toml b/cairo-vm-tracer/Cargo.toml
@@ -12,7 +12,7 @@ std = []
 alloc = []
 
 [dependencies]
-cairo-vm = { workspace = true, features = ["arbitrary", "std"] }
+cairo-vm = { workspace = true, features = ["test_utils"] }
 thiserror-no-std = { workspace = true }
 num-bigint = { workspace = true }
 num-traits = { workspace = true }

diff --git a/fuzzer/Cargo.toml b/fuzzer/Cargo.toml
@@ -13,7 +13,7 @@ members = ["."]
 
 [dependencies]
 arbitrary = { version = "1.3.0", features = ["derive"] }
-cairo-vm = { path = "../vm", features = ["arbitrary"] }
+cairo-vm = { path = "../vm", features = ["test_utils"] }
 honggfuzz = "0.5.55"
 libfuzzer-sys = "0.4"
 num-bigint = "0.4"

diff --git a/vm/Cargo.toml b/vm/Cargo.toml
@@ -31,18 +31,10 @@ tracer = []
 mod_builtin = []
 
 # Note that these features are not retro-compatible with the cairo Python VM.
-test_utils = [
-    "skip_next_instruction_hint",
-    "hooks",
-    "print",
-] # This feature will reference every test-oriented feature
-skip_next_instruction_hint = []
-hooks = []
-arbitrary = ["dep:arbitrary", "std", "starknet-types-core/arbitrary", "starknet-types-core/std"]
+test_utils = ["std", "dep:arbitrary", "starknet-types-core/arbitrary", "starknet-types-core/std"] # This feature will reference every test-oriented feature
 # Allows extending the set of hints for the current vm run from within a hint.
 # For a usage example checkout vm/src/tests/run_deprecated_contract_class_simplified.rs
 extensive_hints = []
-print = ["std"]
 
 [dependencies]
 zip = {version = "0.6.6", optional = true }

diff --git a/vm/src/cairo_run.rs b/vm/src/cairo_run.rs
@@ -16,12 +16,12 @@ use bincode::enc::write::Writer;
 
 use thiserror_no_std::Error;
 
-#[cfg(feature = "arbitrary")]
+#[cfg(feature = "test_utils")]
 use arbitrary::{self, Arbitrary};
 
-#[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
 pub struct CairoRunConfig<'a> {
-    #[cfg_attr(feature = "arbitrary", arbitrary(value = "main"))]
+    #[cfg_attr(feature = "test_utils", arbitrary(value = "main"))]
     pub entrypoint: &'a str,
     pub trace_enabled: bool,
     pub relocate_mem: bool,
@@ -177,7 +177,7 @@ pub fn cairo_run_pie(
     Ok((cairo_runner, vm))
 }
 
-#[cfg(feature = "arbitrary")]
+#[cfg(feature = "test_utils")]
 pub fn cairo_run_fuzzed_program(
     program: Program,
     cairo_run_config: &CairoRunConfig,

diff --git a/vm/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs b/vm/src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs
@@ -114,10 +114,10 @@ use crate::{
     vm::{errors::hint_errors::HintError, vm_core::VirtualMachine},
 };
 
-#[cfg(feature = "skip_next_instruction_hint")]
+#[cfg(feature = "test_utils")]
 use crate::hint_processor::builtin_hint_processor::skip_next_instruction::skip_next_instruction;
 
-#[cfg(feature = "print")]
+#[cfg(feature = "test_utils")]
 use crate::hint_processor::builtin_hint_processor::print::{print_array, print_dict, print_felt};
 use crate::hint_processor::builtin_hint_processor::secp::secp_utils::{
     SECP256R1_ALPHA, SECP256R1_P,
@@ -856,13 +856,13 @@ impl HintProcessorLogic for BuiltinHintProcessor {
                     constants,
                 )
             }
-            #[cfg(feature = "skip_next_instruction_hint")]
+            #[cfg(feature = "test_utils")]
             hint_code::SKIP_NEXT_INSTRUCTION => skip_next_instruction(vm),
-            #[cfg(feature = "print")]
+            #[cfg(feature = "test_utils")]
             hint_code::PRINT_FELT => print_felt(vm, &hint_data.ids_data, &hint_data.ap_tracking),
-            #[cfg(feature = "print")]
+            #[cfg(feature = "test_utils")]
             hint_code::PRINT_ARR => print_array(vm, &hint_data.ids_data, &hint_data.ap_tracking),
-            #[cfg(feature = "print")]
+            #[cfg(feature = "test_utils")]
             hint_code::PRINT_DICT => {
                 print_dict(vm, exec_scopes, &hint_data.ids_data, &hint_data.ap_tracking)
             }

diff --git a/vm/src/hint_processor/builtin_hint_processor/hint_code.rs b/vm/src/hint_processor/builtin_hint_processor/hint_code.rs
@@ -1427,15 +1427,18 @@ if x % 2 != 0:
     x = PRIME - x
 ids.x.low = x & ((1<<128)-1)
 ids.x.high = x >> 128";
-#[cfg(feature = "skip_next_instruction_hint")]
+#[cfg(feature = "test_utils")]
 pub const SKIP_NEXT_INSTRUCTION: &str = "skip_next_instruction()";
 
+#[cfg(feature = "test_utils")]
 pub const PRINT_FELT: &str = "print(ids.x)";
 
+#[cfg(feature = "test_utils")]
 pub const PRINT_ARR: &str = r#"print(bytes.fromhex(f"{ids.name:062x}").decode().replace('\x00',''))
 arr = [memory[ids.arr + i] for i in range(ids.arr_len)]
 print(arr)"#;
 
+#[cfg(feature = "test_utils")]
 pub const PRINT_DICT: &str = r#"print(bytes.fromhex(f"{ids.name:062x}").decode().replace('\x00',''))
 data = __dict_manager.get_dict(ids.dict_ptr)
 print(

diff --git a/vm/src/hint_processor/builtin_hint_processor/mod.rs b/vm/src/hint_processor/builtin_hint_processor/mod.rs
@@ -19,15 +19,16 @@ pub mod memset_utils;
 mod mod_circuit;
 pub mod poseidon_utils;
 pub mod pow_utils;
-#[cfg(feature = "print")]
+#[cfg(feature = "test_utils")]
+#[cfg_attr(docsrs, doc(cfg(feature = "test_utils")))]
 pub mod print;
 pub mod secp;
 pub mod segments;
 pub mod set;
 pub mod sha256_utils;
 pub mod signature;
-#[cfg(feature = "skip_next_instruction_hint")]
-#[cfg_attr(docsrs, doc(cfg(feature = "skip_next_instruction_hint")))]
+#[cfg(feature = "test_utils")]
+#[cfg_attr(docsrs, doc(cfg(feature = "test_utils")))]
 pub mod skip_next_instruction;
 pub mod squash_dict_utils;
 pub mod uint256_utils;

diff --git a/vm/src/hint_processor/hint_processor_definition.rs b/vm/src/hint_processor/hint_processor_definition.rs
@@ -15,7 +15,7 @@ use crate::vm::vm_core::VirtualMachine;
 use super::builtin_hint_processor::builtin_hint_processor_definition::HintProcessorData;
 use crate::Felt252;
 
-#[cfg(feature = "arbitrary")]
+#[cfg(feature = "test_utils")]
 use arbitrary::Arbitrary;
 
 pub trait HintProcessorLogic {
@@ -98,7 +98,7 @@ fn get_ids_data(
     Ok(ids_data)
 }
 
-#[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub struct HintReference {
     pub offset1: OffsetValue,

diff --git a/vm/src/lib.rs b/vm/src/lib.rs
@@ -2,11 +2,12 @@
 //!
 //! ## Feature Flags
 //! - `std`: Enables usage of the [`std`] standard library. Enabled by default.
-//! - `skip_next_instruction_hint`: Enable the `skip_next_instruction()` hint. Not enabled by default.
-//! - `hooks`: Enable [`Hooks`](crate::vm::hooks::Hooks) support for the [VirtualMachine](vm::vm_core::VirtualMachine). Not enabled by default.
-//! - `test_utils`: Enables test utils (`hooks` and `skip_next_instruction` features). Not enabled by default.
+//! - `test_utils`: Enables the following to help with tests (not enabled by default):
+//!    - [`Hooks`](crate::vm::hooks::Hooks) support for the [VirtualMachine](vm::vm_core::VirtualMachine);
+//!    - the `print_*` family of hints;
+//!    - the `skip_next_instruction()` hints;
+//!    - implementations of [`arbitrary::Arbitrary`](https://docs.rs/arbitrary/latest/arbitrary/) for some structs.
 //! - `cairo-1-hints`: Enable hints that were introduced in Cairo 1. Not enabled by default.
-//! - `arbitrary`: Enables implementations of [`arbitrary::Arbitrary`](https://docs.rs/arbitrary/latest/arbitrary/) for some structs. Not enabled by default.
 
 #![cfg_attr(docsrs, feature(doc_cfg))]
 #![deny(warnings)]

diff --git a/vm/src/serde/deserialize_program.rs b/vm/src/serde/deserialize_program.rs
@@ -33,10 +33,10 @@ use num_traits::{float::FloatCore, Num};
 use serde::{de, de::MapAccess, de::SeqAccess, Deserialize, Deserializer, Serialize};
 use serde_json::Number;
 
-#[cfg(all(feature = "arbitrary", feature = "std"))]
+#[cfg(feature = "test_utils")]
 use arbitrary::{self, Arbitrary, Unstructured};
 
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary, Clone))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary, Clone))]
 #[derive(Deserialize, Debug)]
 pub struct ProgramJson {
     pub prime: String,
@@ -51,23 +51,23 @@ pub struct ProgramJson {
     pub debug_info: Option<DebugInfo>,
 }
 
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
 #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
 pub struct HintParams {
     pub code: String,
     pub accessible_scopes: Vec<String>,
     pub flow_tracking_data: FlowTrackingData,
 }
 
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
 #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
 pub struct FlowTrackingData {
     pub ap_tracking: ApTracking,
     #[serde(deserialize_with = "deserialize_map_to_string_and_usize_hashmap")]
     pub reference_ids: HashMap<String, usize>,
 }
 
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
 #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
 pub struct ApTracking {
     pub group: usize,
@@ -89,7 +89,7 @@ impl Default for ApTracking {
     }
 }
 
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
 pub struct Identifier {
     pub pc: Option<usize>,
@@ -104,24 +104,21 @@ pub struct Identifier {
     pub cairo_type: Option<String>,
 }
 
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
 pub struct Member {
     pub cairo_type: String,
     pub offset: usize,
 }
 
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
 pub struct Attribute {
     pub name: String,
     pub start_pc: usize,
     pub end_pc: usize,
     pub value: String,
-    #[cfg_attr(
-        all(feature = "arbitrary", feature = "std"),
-        serde(skip_serializing_if = "Option::is_none")
-    )]
+    #[cfg_attr(feature = "test_utils", serde(skip_serializing_if = "Option::is_none"))]
     pub flow_tracking_data: Option<FlowTrackingData>,
 }
 
@@ -135,14 +132,14 @@ pub struct Location {
     pub start_col: u32,
 }
 
-#[cfg(all(feature = "arbitrary", feature = "std"))]
+#[cfg(feature = "test_utils")]
 impl<'a> Arbitrary<'a> for Location {
     fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
         arbitrary_parent_location(u, 20)
     }
 }
 
-#[cfg(all(feature = "arbitrary", feature = "std"))]
+#[cfg(feature = "test_utils")]
 fn arbitrary_parent_location(u: &mut Unstructured, depth: u8) -> arbitrary::Result<Location> {
     let parent_location = if depth > 0 {
         Some((
@@ -162,7 +159,7 @@ fn arbitrary_parent_location(u: &mut Unstructured, depth: u8) -> arbitrary::Resu
     })
 }
 
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary, Clone))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary, Clone))]
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq)]
 pub struct DebugInfo {
     pub(crate) instruction_locations: HashMap<usize, InstructionLocation>,
@@ -179,14 +176,14 @@ impl DebugInfo {
     }
 }
 
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
 pub struct InstructionLocation {
     pub inst: Location,
     pub hints: Vec<HintLocation>,
 }
 
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
 pub struct InputFile {
     pub filename: String,
@@ -203,7 +200,7 @@ impl InputFile {
     }
 }
 
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
 pub struct HintLocation {
     pub location: Location,
@@ -251,13 +248,13 @@ fn deserialize_scientific_notation(n: Number) -> Option<Felt252> {
     }
 }
 
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Default)]
 pub struct ReferenceManager {
     pub references: Vec<Reference>,
 }
 
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
 pub struct Reference {
     pub ap_tracking_data: ApTracking,
@@ -267,15 +264,15 @@ pub struct Reference {
     pub value_address: ValueAddress,
 }
 
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
 pub enum OffsetValue {
     Immediate(Felt252),
     Value(i32),
     Reference(Register, i32, bool),
 }
 
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
+#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)]
 pub struct ValueAddress {
     pub offset1: OffsetValue,    // A in cast(A + B, type)

diff --git a/vm/src/tests/cairo_run_test.rs b/vm/src/tests/cairo_run_test.rs
@@ -1043,28 +1043,28 @@ fn divmod_igcdex_not_one() {
 }
 
 #[test]
-#[cfg(feature = "print")]
+#[cfg(feature = "test_utils")]
 fn cairo_run_print_felt() {
     let program_data = include_bytes!("../../../cairo_programs/print_feature/print_felt.json");
     run_program_simple(program_data);
 }
 
 #[test]
-#[cfg(feature = "print")]
+#[cfg(feature = "test_utils")]
 fn cairo_run_print_array() {
     let program_data = include_bytes!("../../../cairo_programs/print_feature/print_array.json");
     run_program_simple(program_data);
 }
 
 #[test]
-#[cfg(feature = "print")]
+#[cfg(feature = "test_utils")]
 fn cairo_run_print_dict_felt() {
     let program_data = include_bytes!("../../../cairo_programs/print_feature/print_dict_felt.json");
     run_program_simple(program_data);
 }
 
 #[test]
-#[cfg(feature = "print")]
+#[cfg(feature = "test_utils")]
 fn cairo_run_print_dict_array() {
     let program_data =
         include_bytes!("../../../cairo_programs/print_feature/print_dict_array.json");

diff --git a/vm/src/tests/mod.rs b/vm/src/tests/mod.rs
@@ -42,7 +42,7 @@ mod pedersen_test;
 mod struct_test;
 
 mod cairo_pie_test;
-#[cfg(feature = "skip_next_instruction_hint")]
+#[cfg(feature = "test_utils")]
 mod skip_instruction_test;
 
 //For simple programs that should just succeed and have no special needs.