🛠️ Refactor code, update comments, and improve tests

Refactored `ziggy.rs` by removing commented code and improving function signatures. Enhanced logging in `instrumentation.rs` and added a new function to handle temporary `clippy.toml` files. Updated the GitHub workflow to separate unit and integration tests and made slight adjustments to comments and function documentation across various files.

Signed-off-by: Kevin Valerio <kevin@srlabs.de>

.github/workflows/rust.yml

@@ -55,5 +55,14 @@ jobs:
         working-directory: ./sample
         run: bash build.sh
 
-      - name: Run unit tests and integration tests
-        run: cargo test --no-fail-fast -- --show-output --test-threads=1
+      - name: Run unit tests of all crates
+        run: cargo test -- --show-output --test-threads=1
+
+      - name: Run integration tests for *instrumentation*
+        run: cargo test --test cli_instrument_integration_test -- --show-output
+
+      - name: Run integration tests for *sample*
+        run: cargo test --test sample_integration_test -- --show-output
+
+#      - name: Run integration tests for *fuzz*
+#        run: cargo test --test cli_fuzz_integration_test -- --show-output

Cargo.toml

@@ -39,12 +39,12 @@ std = [
 assert_cmd = { version = "2.0.16" }
 toml_edit = { version = "0.22.20" }
 predicates = { version = "3.1.2" }
-tempfile = { version = "3.12.0" }
 mockall = { version = "0.13.0" }
 
 
 [dependencies]
-# Standard crates
+# Standard crates and helpers
+tempfile = { version = "3.12.0" }
 seq-macro = { version = "0.3.5" }
 scale-info = { version = "2.6.0", default-features = false }
 prettytable-rs = { version = "0.10.0" }
@@ -80,7 +80,6 @@ pallet-timestamp = { git = "https://github.com/paritytech/polkadot-sdk.git", bra
 pallet-transaction-payment = { git = "https://github.com/paritytech/polkadot-sdk.git", branch = "release-polkadot-v1.10.0", default-features = false }
 pallet-contracts = { git = "https://github.com/paritytech/polkadot-sdk.git", branch = "release-polkadot-v1.10.0", default-features = false }
 pallet-insecure-randomness-collective-flip = { git = "https://github.com/paritytech/polkadot-sdk.git", branch = "release-polkadot-v1.10.0", default-features = false }
-
 # ink! crates
 ink_metadata = { version = "*" }
 ink_env = { version = "*" }

phink.toml

@@ -10,8 +10,8 @@ deployer_address = "5C62Ck4UrFPiBtoCmeSrgF7x9yv9mn38446dhCpsi2mLHiFT" # Alice (O
 constructor_payload = "9BAE9D5E5C1100007B000000ACAC0000CC5B763F7AA51000F4BD3F32F51151FF017FD22F9404D0308AFBDB3DE6F2E030E23910AC7DCDBB41BC52F1F2F923E49BAF32E9587DCD4D43D50408B62431D7B79C1A506DBEC4785423DDF36E66E2BEBA6CFEFCDD4F5708DFA3388E48"
 storage_deposit_limit = "100000000000" # this is commented by default, to set is to `None`
 instantiate_initial_value = "0"
-verbose = true
-#instrumented_contract_path.path = "instrumented_output" #commented, we want it in /tmp/
+verbose = false
+instrumented_contract_path.path = "toooooooooooz" #commented, we want it in /tmp/
 show_ui = false
 
 [default_gas_limit]

src/cli/mod.rs

@@ -29,7 +29,7 @@ pub fn format_error(e: anyhow::Error) -> String {
 
     let mut source = e.source();
     while let Some(cause) = source {
-        message = format!("{}\n\n{}: {}", message, "Caused by".cyan().bold(), cause);
+        message = format!("{}\n\n{} {}", message, "Caused by".cyan().bold(), cause);
         source = cause.source();
     }
 

src/cli/ziggy.rs

@@ -126,9 +126,6 @@ impl ZiggyConfig {
         command_builder.args(args.iter());
         command_builder.envs(env);
         command_builder.spawn()?;
-
-        // let status = ziggy_child.wait()?;
-
         Ok(())
     }
 
@@ -139,10 +136,10 @@ impl ZiggyConfig {
     ///
     /// * `command_builder`: The prepared command to which we'll add the AFL ALLOWLIST
     ///
-    /// returns: Result<(), Error>
+    /// returns: Result<()>
     fn with_allowlist(&self, command_builder: &mut Command) -> anyhow::Result<()> {
         if cfg!(not(target_os = "macos")) {
-            let allowlist = PhinkFiles::new(self.config.fuzz_output.clone().unwrap_or_default())
+            let allowlist = PhinkFiles::new(self.config.fuzz_output.to_owned().unwrap_or_default())
                 .path(AllowlistPath);
             command_builder.env(AflLLVMAllowList.to_string(), allowlist.canonicalize()?);
         }

src/instrumenter/instrumentation.rs

@@ -7,6 +7,7 @@ use anyhow::{
     bail,
     Context,
 };
+
 use quote::quote;
 use regex::Regex;
 use std::{
@@ -113,20 +114,29 @@ impl Instrumenter {
             bail!("There was probably a fork issue, as {p_display} doesn't exist.")
         }
 
-        let mut build = Command::new("cargo")
+        let clippy_d = Self::create_temp_clippy()?;
+
+        phink_log!(self, "✂️ Creating `{}` to bypass errors", clippy_d);
+
+        let output = Command::new("cargo")
             .current_dir(path.as_path())
+            .env("RUST_BACKTRACE", "1")
+            .env("CLIPPY_CONF_DIR", clippy_d)
             .args(["contract", "build", "--features=phink"])
-            .spawn()?;
-
-        phink_log!(self, "✂️ Compiling {}...", p_display);
+            .output()?;
 
-        let status = build.wait()?;
+        if output.status.success() {
+            let stdout = String::from_utf8_lossy(&output.stdout);
+            let stderr = String::from_utf8_lossy(&output.stderr);
 
-        if !status.success() {
+            phink_log!(
+                self,
+                "✂️ Compiling `{p_display}` finished successfully!\n{stdout}\n{stderr}",
+            );
+        } else {
             bail!(
-                "🙅 It seems that your instrumented smart contract did not compile properly. \
-        Please go to {p_display}, edit the source code, and run `cargo contract build` again \
-        ({status}).",
+                "It seems that your instrumented smart contract did not compile properly. \
+        Please go to {p_display}, edit the source code, and run `cargo contract build` again.",
             )
         }
 
@@ -138,6 +148,29 @@ impl Instrumenter {
         Ok(())
     }
 
+    /// Return a full path to a temporary `clippy.toml`
+    /// Create a temporary `clippy.toml` file and return its full path.
+    ///
+    /// # Returns
+    /// A `Result` containing the canonicalized path of the temporary file as a `String`.
+    fn create_temp_clippy() -> anyhow::Result<String> {
+        let temp_dir = tempfile::TempDir::new().context("Failed to create temporary directory")?;
+        let clippy_toml_path = temp_dir.path().join("clippy.toml");
+
+        let mut clippy_toml =
+            File::create(&clippy_toml_path).context("Failed to create clippy.toml file")?;
+
+        writeln!(clippy_toml, "avoid-breaking-exported-api = false")
+            .context("Failed to write to clippy.toml file")?;
+
+        let temp_dir_path = temp_dir.into_path();
+        let temp_dir_str = temp_dir_path
+            .to_str()
+            .context("Failed to convert temporary directory path to string")?
+            .to_string();
+
+        Ok(temp_dir_str + "/clippy.toml")
+    }
     fn fork(self) -> anyhow::Result<PathBuf> {
         let new_dir = &self.to_owned().z_config.instrumented_path();
 
@@ -243,8 +276,7 @@ impl Instrumenter {
         file.flush()?;
         println!("🛠️ Formatting {} with `rustfmt`...", rust_file.display());
         Command::new("rustfmt")
-            .arg(rust_file)
-            .arg("--edition=2021")
+            .args([rust_file.display().to_string().as_str(), "--edition=2021"])
             .status()?;
         Ok(())
     }
@@ -347,6 +379,19 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_create_temp_clippy() {
+        let result = Instrumenter::create_temp_clippy().expect("Failed to create temp clippy.toml");
+        assert!(result.ends_with("/clippy.toml"));
+        let path = Path::new(&result);
+        assert!(path.exists(), "clippy.toml file was not created");
+        let content = fs::read_to_string(path).expect("Failed to read clippy.toml file");
+        assert_eq!(
+            content.trim(),
+            "avoid-breaking-exported-api = false",
+            "Unexpected content in clippy.toml"
+        );
+    }
     #[test]
     fn test_find_wasm_and_specs_paths_success() {
         let config = create_temp_ziggy_config(false);

tests/cli_fuzz_integration_test.rs

@@ -142,14 +142,15 @@ mod tests {
             let fuzzing = ensure_while_fuzzing(&config, Duration::from_secs(120), || {
                 let fuzz_created = fs::metadata(fuzz_output.clone()).is_ok();
                 ensure!(fuzz_created, "Fuzz output directory wasn't created");
-                let log_path = config
-                    .fuzz_output
-                    .clone()
-                    .unwrap_or_default()
-                    .join("phink")
-                    .join("logs");
 
                 if fuzz_created {
+                    let log_path = config
+                        .fuzz_output
+                        .clone()
+                        .unwrap_or_default()
+                        .join("phink")
+                        .join("logs");
+
                     ensure!(log_path.join("afl.log").exists(), "afl.log wasn't created",);
                     ensure!(
                         log_path.join("afl_1.log").exists(),

tests/shared/mod.rs

@@ -38,7 +38,7 @@ use std::{
 pub const DEFAULT_TEST_PHINK_TOML: &str = "phink_temp_test.toml";
 
 /// Helper used to run tests, mainly checking that the instrumented
-/// folder/fuzzing output doesn't exist and removes the temporary forked
+/// folder/fuzzing output doesn't exist. It removes the temporary forked
 /// Phink config file, executes the tests and then clean everything again.
 ///
 /// # Arguments