From 259adf7dc215f92d1e039d655a7cb4e587055aaf Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Tue, 12 Nov 2024 16:08:38 +0000 Subject: [PATCH] cob: mutable stable time for testing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `radicle-cob` crate provides a feature flag, `stable-commit-ids` to allow for `Oid`s to remain stable for testing by always using the same author and timestamp when creating commits. This can be an issue when it interplays with COB related tests that are relying on the ordering for the DAG walk. If the `Oid` values change and get re-ordered then the test will fail. The DAG for COBs are now ordered via the `Oid` and `Timestamp`. In this change, a global timestamp is introduced that is initialised per thread. The value is read, via `read_timestamp`, and can be advanced using `with_advanced_timestamp` – while performing an action that creates a new commit. --- radicle-cob/src/backend/git.rs | 3 + radicle-cob/src/backend/git/change.rs | 17 +++--- radicle-cob/src/backend/git/stable.rs | 79 +++++++++++++++++++++++++++ radicle/src/cob/identity.rs | 31 +++++++---- 4 files changed, 111 insertions(+), 19 deletions(-) create mode 100644 radicle-cob/src/backend/git/stable.rs diff --git a/radicle-cob/src/backend/git.rs b/radicle-cob/src/backend/git.rs index d9bd98c9..0cf18802 100644 --- a/radicle-cob/src/backend/git.rs +++ b/radicle-cob/src/backend/git.rs @@ -2,6 +2,9 @@ pub mod change; +#[cfg(feature = "stable-commit-ids")] +pub mod stable; + /// Environment variable to set to overwrite the commit date for both the author and the committer. /// /// The format must be a unix timestamp. diff --git a/radicle-cob/src/backend/git/change.rs b/radicle-cob/src/backend/git/change.rs index 9943c92d..0d9695cf 100644 --- a/radicle-cob/src/backend/git/change.rs +++ b/radicle-cob/src/backend/git/change.rs @@ -301,13 +301,16 @@ fn write_commit( #[cfg(feature = "stable-commit-ids")] // Ensures the commit id doesn't change on every run. - let (author, timestamp) = ( - Author { - time: git_ext::author::Time::new(1514817556, 0), - ..author - }, - 1514817556, - ); + let (author, timestamp) = { + let stable = crate::git::stable::read_timestamp(); + ( + Author { + time: git_ext::author::Time::new(stable, 0), + ..author + }, + stable, + ) + }; let (author, timestamp) = if let Ok(s) = std::env::var(crate::git::GIT_COMMITTER_DATE) { let Ok(timestamp) = s.trim().parse::() else { panic!( diff --git a/radicle-cob/src/backend/git/stable.rs b/radicle-cob/src/backend/git/stable.rs new file mode 100644 index 00000000..0d8ea27b --- /dev/null +++ b/radicle-cob/src/backend/git/stable.rs @@ -0,0 +1,79 @@ +use std::{cell::Cell, ops::Add}; + +thread_local! { + /// The constant time used by the stable-commit-ids feature. + pub static STABLE_TIME: Cell = const { Cell::new(1514817556) }; + /// An incrementing counter to advance the `STABLE_TIME` value with in + /// [`with_advanced_timestamp`]. + pub static STEP: Cell = Cell::new(Step::default()); +} + +#[derive(Clone, Copy)] +struct Step(i64); + +impl Default for Step { + fn default() -> Self { + Self(1) + } +} + +impl Add for i64 { + type Output = i64; + + fn add(self, rhs: Step) -> Self::Output { + self + rhs.0 + } +} + +impl Add for Step { + type Output = Step; + + fn add(self, rhs: i64) -> Self::Output { + Step(self.0 + rhs) + } +} + +/// Read the current value of `STABLE_TIME`. +/// +/// # Panics +/// +/// The `STABLE_TIME` is declared in `thread_local`, and so the panic +/// information is repeated here. +/// +/// Panics if the key currently has its destructor running, and it may panic if +/// the destructor has previously been run for this thread. +#[allow(clippy::unwrap_used)] +pub fn read_timestamp() -> i64 { + STABLE_TIME.get() +} + +/// Perform an action `f` that would rely on the `STABLE_TIME` value. This will +/// advance the `STABLE_TIME` by an increment of `1` for each time it is called, +/// within the same thread. +/// +/// # Usage +/// +/// ```no_run +/// let oid1 = with_advanced_timestamp(|| cob.update("New revision OID")); +/// let oid2 = with_advanced_timestamp(|| cob.update("Another revision OID")); +/// ``` +/// +/// # Panics +/// +/// The `STABLE_TIME` is declared in `thread_local`, and so the panic +/// information is repeated here. +/// +/// Panics if the key currently has its destructor running, and it may panic if +/// the destructor has previously been run for this thread. +#[allow(clippy::unwrap_used)] +pub fn with_advanced_timestamp(f: F) -> T +where + F: FnOnce() -> T, +{ + let step = STEP.get(); + let original = read_timestamp(); + STABLE_TIME.replace(original + step); + let result = f(); + STEP.replace(step + 1); + result +} diff --git a/radicle/src/cob/identity.rs b/radicle/src/cob/identity.rs index ec44f282..550f7418 100644 --- a/radicle/src/cob/identity.rs +++ b/radicle/src/cob/identity.rs @@ -1016,7 +1016,10 @@ mod test { use radicle_crypto::test::signer::MockSigner; use radicle_crypto::Signer as _; + use crate::cob; use crate::crypto::PublicKey; + use crate::identity::did::Did; + use crate::identity::doc::PayloadId; use crate::identity::Visibility; use crate::rad; use crate::storage::git::Storage; @@ -1025,8 +1028,6 @@ mod test { use crate::test::setup::{Network, NodeWithRepo}; use super::*; - use crate::identity::did::Did; - use crate::identity::doc::PayloadId; #[quickcheck] fn prop_json_eq_str(pk: PublicKey, proj: RepoId, did: Did) { @@ -1319,20 +1320,24 @@ mod test { eve.repo.fetch(bob); let mut bob_identity = Identity::load_mut(&*bob.repo).unwrap(); - let b1 = bob_identity.accept(&a2, &bob.signer).unwrap(); + let b1 = cob::git::stable::with_advanced_timestamp(|| { + bob_identity.accept(&a2, &bob.signer).unwrap() + }); assert_eq!(bob_identity.current, a2); let mut eve_identity = Identity::load_mut(&*eve.repo).unwrap(); let mut eve_doc = eve_identity.doc().clone().edit(); eve_doc.visibility = Visibility::private([eve.signer.public_key().into()]); - let e1 = eve_identity - .update( - "Change visibility", - "", - &eve_doc.verified().unwrap(), - &eve.signer, - ) - .unwrap(); + let e1 = cob::git::stable::with_advanced_timestamp(|| { + eve_identity + .update( + "Change visibility", + "", + &eve_doc.verified().unwrap(), + &eve.signer, + ) + .unwrap() + }); // Eve's revision is active. assert!(eve_identity.revision(&e1).unwrap().is_active()); @@ -1506,7 +1511,9 @@ mod test { assert_eq!(eve_identity.revision(&e1).unwrap().state, State::Active); alice_identity.reload().unwrap(); - let a2 = alice_identity.accept(&b1, &alice.signer).unwrap(); + let a2 = cob::git::stable::with_advanced_timestamp(|| { + alice_identity.accept(&b1, &alice.signer).unwrap() + }); eve.repo.fetch(alice); eve_identity.reload().unwrap();