From 39ace002e9174555597f758dbabbc6675610c4b1 Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 30 May 2024 12:17:44 +0200 Subject: [PATCH 1/6] test: getters for Branches - derive(PartialEq) for Branch --- consensus/cryptarchia-engine/src/lib.rs | 36 ++++++++++++++++++++----- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/consensus/cryptarchia-engine/src/lib.rs b/consensus/cryptarchia-engine/src/lib.rs index 0ab515f4..ea3f2295 100644 --- a/consensus/cryptarchia-engine/src/lib.rs +++ b/consensus/cryptarchia-engine/src/lib.rs @@ -21,6 +21,7 @@ pub struct Branches { tips: HashSet, } +#[derive(PartialEq)] #[derive(Clone, Debug)] pub struct Branch { id: Id, @@ -46,8 +47,8 @@ impl Branch { } impl Branches -where - Id: Eq + std::hash::Hash + Copy, + where + Id: Eq + std::hash::Hash + Copy, { pub fn from_genesis(genesis: Id) -> Self { let mut branches = HashMap::new(); @@ -139,8 +140,8 @@ pub enum Error { } impl Cryptarchia -where - Id: Eq + std::hash::Hash + Copy, + where + Id: Eq + std::hash::Hash + Copy, { pub fn from_genesis(id: Id, config: Config) -> Self { Self { @@ -220,7 +221,7 @@ where #[cfg(test)] pub mod tests { - use super::Cryptarchia; + use super::{Cryptarchia}; use crate::Config; use std::hash::{DefaultHasher, Hash, Hasher}; @@ -297,7 +298,7 @@ pub mod tests { k, engine.config.s() ) - .id, + .id, long_p ); @@ -320,4 +321,27 @@ pub mod tests { res[..8].copy_from_slice(&hash.to_be_bytes()); res } + + #[test] + fn test_getters() { + let engine = Cryptarchia::from_genesis([0; 32], config()); + let parent = engine.genesis(); + + // Get branch directly from HashMap + let branch1 = engine.branches.get(&parent).ok_or("At least one branch should be there"); + + let branches = engine.branches(); + + // Get branch using getter + let branch2 = branches.get(&parent).ok_or("At least one branch should be there"); + + assert_eq!(branch1, branch2); + assert_eq!(branch1.expect("id is set").id(), branch2.expect("id is set").id()); + assert_eq!(branch1.expect("parent is set").parent(), branch2.expect("parent is set").parent()); + assert_eq!(branch1.expect("slot is set").slot(), branch2.expect("slot is set").slot()); + assert_eq!(branch1.expect("length is set").length(), branch2.expect("length is set").length()); + + } } + + From 59169be873dec2dd6d3148461af94b878a2ad014 Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 30 May 2024 12:44:42 +0200 Subject: [PATCH 2/6] fix: format code --- consensus/cryptarchia-engine/src/lib.rs | 47 ++++++++++++++++--------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/consensus/cryptarchia-engine/src/lib.rs b/consensus/cryptarchia-engine/src/lib.rs index ea3f2295..5c7b1e8e 100644 --- a/consensus/cryptarchia-engine/src/lib.rs +++ b/consensus/cryptarchia-engine/src/lib.rs @@ -21,8 +21,7 @@ pub struct Branches { tips: HashSet, } -#[derive(PartialEq)] -#[derive(Clone, Debug)] +#[derive(PartialEq, Clone, Debug)] pub struct Branch { id: Id, parent: Id, @@ -47,8 +46,8 @@ impl Branch { } impl Branches - where - Id: Eq + std::hash::Hash + Copy, +where + Id: Eq + std::hash::Hash + Copy, { pub fn from_genesis(genesis: Id) -> Self { let mut branches = HashMap::new(); @@ -140,8 +139,8 @@ pub enum Error { } impl Cryptarchia - where - Id: Eq + std::hash::Hash + Copy, +where + Id: Eq + std::hash::Hash + Copy, { pub fn from_genesis(id: Id, config: Config) -> Self { Self { @@ -221,7 +220,7 @@ impl Cryptarchia #[cfg(test)] pub mod tests { - use super::{Cryptarchia}; + use super::Cryptarchia; use crate::Config; use std::hash::{DefaultHasher, Hash, Hasher}; @@ -298,7 +297,7 @@ pub mod tests { k, engine.config.s() ) - .id, + .id, long_p ); @@ -328,20 +327,34 @@ pub mod tests { let parent = engine.genesis(); // Get branch directly from HashMap - let branch1 = engine.branches.get(&parent).ok_or("At least one branch should be there"); + let branch1 = engine + .branches + .get(&parent) + .ok_or("At least one branch should be there"); let branches = engine.branches(); // Get branch using getter - let branch2 = branches.get(&parent).ok_or("At least one branch should be there"); + let branch2 = branches + .get(&parent) + .ok_or("At least one branch should be there"); assert_eq!(branch1, branch2); - assert_eq!(branch1.expect("id is set").id(), branch2.expect("id is set").id()); - assert_eq!(branch1.expect("parent is set").parent(), branch2.expect("parent is set").parent()); - assert_eq!(branch1.expect("slot is set").slot(), branch2.expect("slot is set").slot()); - assert_eq!(branch1.expect("length is set").length(), branch2.expect("length is set").length()); - + assert_eq!( + branch1.expect("id is set").id(), + branch2.expect("id is set").id() + ); + assert_eq!( + branch1.expect("parent is set").parent(), + branch2.expect("parent is set").parent() + ); + assert_eq!( + branch1.expect("slot is set").slot(), + branch2.expect("slot is set").slot() + ); + assert_eq!( + branch1.expect("length is set").length(), + branch2.expect("length is set").length() + ); } } - - From 2e4ec9a5861011cb0e55c56e489fe3e07571ca08 Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 30 May 2024 21:44:43 +0200 Subject: [PATCH 3/6] test: Slot genesis and add --- consensus/cryptarchia-engine/src/lib.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/consensus/cryptarchia-engine/src/lib.rs b/consensus/cryptarchia-engine/src/lib.rs index 5c7b1e8e..cffe98f2 100644 --- a/consensus/cryptarchia-engine/src/lib.rs +++ b/consensus/cryptarchia-engine/src/lib.rs @@ -220,7 +220,7 @@ where #[cfg(test)] pub mod tests { - use super::Cryptarchia; + use super::{Cryptarchia, Slot}; use crate::Config; use std::hash::{DefaultHasher, Hash, Hasher}; @@ -356,5 +356,9 @@ pub mod tests { branch1.expect("length is set").length(), branch2.expect("length is set").length() ); + + let slot = Slot::genesis(); + + assert_eq!(slot + 10u64, Slot::from(10)); } } From f1a621fa11a4b6fa24e95bc1698a5251372db24a Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 31 May 2024 12:06:50 +0200 Subject: [PATCH 4/6] fix: change expect message --- consensus/cryptarchia-engine/src/lib.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/consensus/cryptarchia-engine/src/lib.rs b/consensus/cryptarchia-engine/src/lib.rs index cffe98f2..d437270d 100644 --- a/consensus/cryptarchia-engine/src/lib.rs +++ b/consensus/cryptarchia-engine/src/lib.rs @@ -341,20 +341,20 @@ pub mod tests { assert_eq!(branch1, branch2); assert_eq!( - branch1.expect("id is set").id(), - branch2.expect("id is set").id() + branch1.expect("id is not set").id(), + branch2.expect("id is not set").id() ); assert_eq!( - branch1.expect("parent is set").parent(), - branch2.expect("parent is set").parent() + branch1.expect("parent is not set").parent(), + branch2.expect("parent is not set").parent() ); assert_eq!( - branch1.expect("slot is set").slot(), - branch2.expect("slot is set").slot() + branch1.expect("slot is not set").slot(), + branch2.expect("slot is not set").slot() ); assert_eq!( - branch1.expect("length is set").length(), - branch2.expect("length is set").length() + branch1.expect("length is not set").length(), + branch2.expect("length is not set").length() ); let slot = Slot::genesis(); From f51a763ca341514c81e95f6b4a2d16dc25901259 Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 31 May 2024 12:57:56 +0200 Subject: [PATCH 5/6] test: add ParentMissing error case - derive(PartialEq) only for tests --- consensus/cryptarchia-engine/src/lib.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/consensus/cryptarchia-engine/src/lib.rs b/consensus/cryptarchia-engine/src/lib.rs index d437270d..b48c4b96 100644 --- a/consensus/cryptarchia-engine/src/lib.rs +++ b/consensus/cryptarchia-engine/src/lib.rs @@ -21,7 +21,8 @@ pub struct Branches { tips: HashSet, } -#[derive(PartialEq, Clone, Debug)] +#[derive(Clone, Debug)] +#[cfg_attr(test, derive(PartialEq))] pub struct Branch { id: Id, parent: Id, @@ -131,6 +132,7 @@ where } #[derive(Debug, Clone, Error)] +#[cfg_attr(test, derive(PartialEq))] pub enum Error { #[error("Parent block: {0:?} is not know to this node")] ParentMissing(Id), @@ -220,7 +222,7 @@ where #[cfg(test)] pub mod tests { - use super::{Cryptarchia, Slot}; + use super::{Cryptarchia, Error, Slot}; use crate::Config; use std::hash::{DefaultHasher, Hash, Hasher}; @@ -360,5 +362,18 @@ pub mod tests { let slot = Slot::genesis(); assert_eq!(slot + 10u64, Slot::from(10)); + + let strange_engine = Cryptarchia::from_genesis([100; 32], config()); + let not_a_parent = strange_engine.genesis(); + + _ = match branches + .get(¬_a_parent) + .ok_or(Error::ParentMissing(not_a_parent)) + { + Ok(_) => (), + Err(e) => { + assert_ne!(e, Error::ParentMissing(parent)); + } + }; } } From 1612a7d83d4b6c2e6be95739fe74505502518249 Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 31 May 2024 13:05:49 +0200 Subject: [PATCH 6/6] fix: false positive for ParentMissing --- consensus/cryptarchia-engine/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/consensus/cryptarchia-engine/src/lib.rs b/consensus/cryptarchia-engine/src/lib.rs index b48c4b96..4bc05f61 100644 --- a/consensus/cryptarchia-engine/src/lib.rs +++ b/consensus/cryptarchia-engine/src/lib.rs @@ -370,7 +370,7 @@ pub mod tests { .get(¬_a_parent) .ok_or(Error::ParentMissing(not_a_parent)) { - Ok(_) => (), + Ok(_) => panic!("Parent should not be related to this branch"), Err(e) => { assert_ne!(e, Error::ParentMissing(parent)); }