Merge branch 'tomas/more-tests' (#3704)

* tomas/more-tests:
  ci: run integration tests with coverage
  make: add `test-unit-with-coverage`
  tests: remove "integration" feature
  codecov: ignore more utils & testing code
  test/core/masp: test basics
  test/core/ibc: add basic unit tests
  core: remove `ByteBuf` and re-export `HEXLOWER` and `HEXUPPER`

.github/workflows/ci.yml

@@ -290,12 +290,11 @@ jobs:
         # run: cargo +${{ env.NIGHTLY }} llvm-cov nextest run -E 'not test(e2e)' -E 'not test(integration)' -E 'not test(pos_state_machine_test)' --features namada/testing --no-fail-fast --lcov --output-path lcov.info
         env:
           RUSTFLAGS: "-C linker=clang -C link-arg=-fuse-ld=/usr/local/bin/mold -Z threads=8"
-      - name: Upload coverage
-        uses: codecov/codecov-action@v4
+      - name: Store coverage file artifact
+        uses: actions/upload-artifact@v4
         with:
-          files: lcov.info
-          fail_ci_if_error: true
-          token: ${{ secrets.CODECOV_TOKEN }}
+          name: unit-cov-${{ github.event.pull_request.head.sha || github.sha }}.info
+          path: lcov.info # output of `make test-unit-with-coverage` 
       - name: Clean cargo cache
         if: steps.cache.outputs.cache-hit != 'true'
         run: cargo cache --autoclean-expensive
@@ -387,10 +386,16 @@ jobs:
         with:
           name: wasm-for-tests-${{ github.event.pull_request.head.sha|| github.sha }}
           path: wasm_for_tests
-      - name: Run integration tests
-        run: cargo +${{ env.NIGHTLY }} nextest run -E 'test(integration)' --test-threads 1 --no-fail-fast --features integration
+      - name: Run integration tests with coverage
+        run: make test-integration-with-coverage
+        # run: cargo +${{ env.NIGHTLY }} nextest run -E 'test(integration)' --test-threads 1 --no-fail-fast
         env:
           RUSTFLAGS: "-C linker=clang -C link-arg=-fuse-ld=/usr/local/bin/mold -Z threads=8"
+      - name: Store coverage file artifact
+        uses: actions/upload-artifact@v4
+        with:
+          name: integration-cov-${{ github.event.pull_request.head.sha || github.sha }}.info
+          path: lcov.info # output of `make test-integration-with-coverage` 
       - name: Clean cargo cache
         if: steps.cache.outputs.cache-hit != 'true'
         run: cargo cache --autoclean-expensive
@@ -607,4 +612,27 @@ jobs:
         run: cargo cache --autoclean-expensive
       - name: Stop sccache
         if: always() && steps.sccache.conclusion == 'success'
-        run: sccache --stop-server || true
+        run: sccache --stop-server || true
+
+  upload-coverage:
+    runs-on: [ubuntu-latest]
+    timeout-minutes: 20
+    needs: [test-unit, test-integration]
+
+    steps:
+      - name: Download unit coverage artifacts
+        uses: actions/download-artifact@v4
+        with:
+          name: unit-cov-${{ github.event.pull_request.head.sha || github.sha }}.info
+          path: unit-cov.info
+      - name: Download integration coverage artifacts
+        uses: actions/download-artifact@v4
+        with:
+          name: integration-cov-${{ github.event.pull_request.head.sha || github.sha }}.info
+          path: integration-cov.info
+      - name: Upload coverage to codecov
+        uses: codecov/codecov-action@v4
+        with:
+          files: integration-cov.info, unit-cov.info
+          fail_ci_if_error: true
+          token: ${{ secrets.CODECOV_TOKEN }}

Makefile

@@ -155,15 +155,10 @@ audit:
 test: test-unit test-e2e test-wasm test-benches
 
 test-coverage:
-	# Run integration tests separately because they require `integration`
-	# feature (and without coverage)
 	$(cargo) +$(nightly) llvm-cov --output-path lcov.info \
 		--lcov \
-		-- --skip e2e --skip pos_state_machine_test --skip integration \
-		-Z unstable-options --report-time && \
-	$(cargo) +$(nightly) test integration:: \
-		--features integration \
-		-- -Z unstable-options --report-time
+		-- --skip e2e --skip pos_state_machine_test \
+		-Z unstable-options --report-time
 
 # NOTE: `TEST_FILTER` is prepended with `e2e::`. Since filters in `cargo test`
 # work with a substring search, TEST_FILTER only works if it contains a string
@@ -183,7 +178,7 @@ test-e2e:
 # Run integration tests
 test-integration:
 	RUST_BACKTRACE=$(RUST_BACKTRACE) \
-	$(cargo) +$(nightly) test --lib $(jobs) integration::$(TEST_FILTER)  --features integration \
+	$(cargo) +$(nightly) test --lib $(jobs) integration::$(TEST_FILTER) \
 	-Z unstable-options \
 	-- \
 	--test-threads=1 \
@@ -205,11 +200,18 @@ test-unit-with-eth-bridge:
 		-Z unstable-options --report-time
 
 test-unit-with-coverage:
-	$(cargo) +$(nightly) llvm-cov --output-path lcov.info \
+	$(cargo) +$(nightly) llvm-cov --lib --output-path lcov.info \
 		--lcov \
 		-- --skip e2e --skip pos_state_machine_test --skip integration \
 		-Z unstable-options --report-time
 
+test-integration-with-coverage:
+	$(cargo) +$(nightly) llvm-cov --lib --output-path lcov.info \
+		--lcov \
+		-- integration \
+		--test-threads=1 \
+		-Z unstable-options --report-time
+
 test-unit-mainnet:
 	$(cargo) +$(nightly) test --lib \
 		--features "mainnet" \

codecov.yml

@@ -37,4 +37,6 @@ ignore:
   - crates/apps/src/bin
   - crates/apps_lib/src/cli
   - crates/apps_lib/src/client
-  - crates/apps_lib/src/wasm_loader
+  - crates/apps_lib/src/wasm_loader
+  - crates/node/src/bench_utils
+  - crates/node/src/shell/testing

crates/apps_lib/Cargo.toml

@@ -17,10 +17,9 @@ default = ["migrations"]
 mainnet = [
   "namada_sdk/mainnet",
 ]
-# for integration tests and test utilities
+# for tests and test utilities
 testing = ["lazy_static", "namada_sdk/testing"]
 benches = ["lazy_static", "namada_sdk/benches"]
-integration = []
 migrations = [
   "namada_migrations",
   "namada_sdk/migrations",

crates/apps_lib/src/config/genesis.rs

@@ -422,10 +422,7 @@ pub struct Parameters {
 /// This includes adding the Ethereum bridge parameters and
 /// adding a specified number of validators.
 #[allow(clippy::arithmetic_side_effects)]
-#[cfg(all(
-    any(test, feature = "benches", feature = "testing"),
-    not(feature = "integration")
-))]
+#[cfg(any(test, feature = "benches", feature = "testing"))]
 pub fn make_dev_genesis(
     num_validators: u64,
     target_chain_dir: &std::path::Path,

crates/core/Cargo.toml

@@ -19,7 +19,7 @@ rand = ["dep:rand", "rand_core"]
 ethers-derive = [
   "ethbridge-structs/ethers-derive"
 ]
-# for integration tests and test utilities
+# for tests and test utilities
 testing = [
   "rand",
   "proptest",
@@ -94,6 +94,7 @@ tokio = { workspace = true, optional = true, default-features = false, features
 
 [dev-dependencies]
 assert_matches.workspace = true
+lazy_static.workspace = true
 proptest.workspace = true
 rand.workspace = true
 rand_core.workspace = true

crates/core/src/bytes.rs

@@ -1,24 +1,3 @@
-//! A helper module for dealing with bytes
+//! Bytes hex formatting
 
-use std::fmt::Display;
-
-/// A helper to show bytes in hex
-pub struct ByteBuf<'a>(pub &'a [u8]);
-
-impl<'a> std::fmt::LowerHex for ByteBuf<'a> {
-    fn fmt(
-        &self,
-        f: &mut std::fmt::Formatter<'_>,
-    ) -> std::result::Result<(), std::fmt::Error> {
-        for byte in self.0 {
-            f.write_fmt(format_args!("{:02x}", byte))?;
-        }
-        Ok(())
-    }
-}
-
-impl<'a> Display for ByteBuf<'a> {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{:x}", self)
-    }
-}
+pub use data_encoding::{HEXLOWER, HEXUPPER};

crates/core/src/chain.rs

@@ -5,15 +5,14 @@ use std::num::ParseIntError;
 use std::str::FromStr;
 
 use borsh::{BorshDeserialize, BorshSchema, BorshSerialize};
-use data_encoding::HEXUPPER;
+use data_encoding::{HEXLOWER, HEXUPPER};
 use namada_macros::BorshDeserializer;
 #[cfg(feature = "migrations")]
 use namada_migrations::*;
 use serde::{Deserialize, Serialize};
 use sha2::{Digest, Sha256};
 use thiserror::Error;
 
-use crate::bytes::ByteBuf;
 use crate::hash::Hash;
 use crate::time::DateTimeUtc;
 
@@ -287,7 +286,7 @@ pub enum ParseBlockHashError {
 
 impl core::fmt::Debug for BlockHash {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let hash = format!("{}", ByteBuf(&self.0));
+        let hash = HEXLOWER.encode(&self.0);
         f.debug_tuple("BlockHash").field(&hash).finish()
     }
 }

crates/core/src/ibc.rs

@@ -164,3 +164,41 @@ impl borsh::BorshSchema for PGFIbcTarget {
         std::any::type_name::<Self>().into()
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::{decode, encode};
+
+    #[test]
+    fn test_ibc_token_hash() {
+        let hash = IbcTokenHash([
+            0_u8, 255, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        ]);
+
+        let hash_str = hash.to_string();
+        assert_eq!("00ff000000000000000000000000000000000000", &hash_str);
+
+        let decoded = IbcTokenHash::from_str(&hash_str).unwrap();
+        assert_eq!(decoded, hash);
+
+        // Hex decoding is case-insensitive
+        let decoded =
+            IbcTokenHash::from_str("00FF000000000000000000000000000000000000")
+                .unwrap();
+        assert_eq!(decoded, hash);
+    }
+
+    #[test]
+    fn test_ibc_pgf_target() {
+        let target = PGFIbcTarget {
+            target: "123".to_string(),
+            amount: token::Amount::from_u64(123456789),
+            port_id: PortId::new("10".to_string()).unwrap(),
+            channel_id: ChannelId::new(5),
+        };
+        let bytes = encode(&target);
+        let decoded: PGFIbcTarget = decode(bytes).unwrap();
+        assert_eq!(target, decoded);
+    }
+}