Browse Source

benchmarks: blog: fix formatting and assertions

conectado 4 years ago
parent
commit
914c78c6e6

+ 4 - 4
runtime-modules/blog/Cargo.toml

@@ -1,8 +1,8 @@
 [package]
-name = "pallet-blog"
-version = "3.0.0"
-authors = ["iorveth"]
-edition = "2018"
+name = 'pallet-blog'
+version = '3.0.0'
+authors = ['Joystream contributors']
+edition = '2018'
 
 [dependencies]
 sp-std = { package = 'sp-std', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = 'a200cdb93c6af5763b9c7bf313fa708764ac88ca'}

+ 50 - 61
runtime-modules/blog/src/benchmarking.rs

@@ -95,7 +95,10 @@ fn generate_post<T: Trait<I>, I: Instance>() -> PostId {
 
     assert_eq!(Blog::<T, I>::post_count(), 1);
 
-    assert!(Blog::<T, I>::post_by_id(post_id) == Post::<T, I>::new(&vec![0u8], &vec![0u8]));
+    assert_eq!(
+        Blog::<T, I>::post_by_id(post_id),
+        Post::<T, I>::new(&vec![0u8], &vec![0u8])
+    );
 
     post_id
 }
@@ -115,9 +118,9 @@ fn generate_reply<T: Trait<I>, I: Instance>(
     )
     .unwrap();
 
-    assert!(
-        Blog::<T, I>::reply_by_id(post_id, T::ReplyId::zero())
-            == Reply::<T, I>::new(vec![0u8], participant_id, ParentId::Post(post_id))
+    assert_eq!(
+        Blog::<T, I>::reply_by_id(post_id, T::ReplyId::zero()),
+        Reply::<T, I>::new(vec![0u8], participant_id, ParentId::Post(post_id))
     );
 
     T::ReplyId::zero()
@@ -134,13 +137,14 @@ benchmarks_instance! {
         assert_eq!(Blog::<T, I>::post_count(), 0);
         let title = vec![0u8; t.try_into().unwrap()];
         let body = vec![0u8; b.try_into().unwrap()];
+        let post_id = Blog::<T, I>::post_count();
 
     }:_(RawOrigin::Root, title.clone(), body.clone())
     verify {
-        assert_eq!(Blog::<T, I>::post_count(), 1);
+        assert_eq!(Blog::<T, I>::post_count(), post_id + 1);
 
-        assert!(
-            Blog::<T, I>::post_by_id(0) ==
+        assert_eq!(
+            Blog::<T, I>::post_by_id(post_id),
             Post::<T, I>::new(&title, &body)
         );
 
@@ -176,15 +180,10 @@ benchmarks_instance! {
         let post_id = generate_post::<T, I>();
         let title = Some(vec![1u8; t.try_into().unwrap()]);
         let body = Some(vec![1u8; b.try_into().unwrap()]);
-    }: _(
-        RawOrigin::Root,
-        post_id,
-        title.clone(),
-        body.clone()
-    )
+    }: _(RawOrigin::Root, post_id, title.clone(), body.clone())
     verify {
-        assert!(
-            Blog::<T, I>::post_by_id(post_id) ==
+        assert_eq!(
+            Blog::<T, I>::post_by_id(post_id),
             Post::<T, I>::new(&vec![1u8; t.try_into().unwrap()], &vec![1u8; b.try_into().unwrap()])
         );
         assert_last_event::<T, I>(RawEvent::PostEdited(post_id, title, body).into());
@@ -197,19 +196,13 @@ benchmarks_instance! {
         let (account_id, participant_id) = member_funded_account::<T, I>("caller", 0);
         let origin = RawOrigin::Signed(account_id);
         let text = vec![0u8; t.try_into().unwrap()];
-    }: create_reply(
-        origin.clone(),
-        participant_id,
-        post_id,
-        None,
-        text.clone()
-    )
+    }: create_reply(origin.clone(), participant_id, post_id, None, text.clone())
     verify {
         let mut expected_post = Post::<T, I>::new(&vec![0u8], &vec![0u8]);
         expected_post.increment_replies_counter();
-        assert!(Blog::<T, I>::post_by_id(post_id) == expected_post);
-        assert!(
-            Blog::<T, I>::reply_by_id(post_id, T::ReplyId::zero()) ==
+        assert_eq!(Blog::<T, I>::post_by_id(post_id), expected_post);
+        assert_eq!(
+            Blog::<T, I>::reply_by_id(post_id, T::ReplyId::zero()),
             Reply::<T, I>::new(
                 text.clone(),
                 participant_id,
@@ -217,7 +210,14 @@ benchmarks_instance! {
             )
         );
 
-        assert_last_event::<T, I>(RawEvent::ReplyCreated(participant_id, post_id, Zero::zero(), text).into());
+        assert_last_event::<T, I>(
+            RawEvent::ReplyCreated(
+                participant_id,
+                post_id,
+                Zero::zero(),
+                text
+            ).into()
+        );
     }
 
     create_reply_to_reply {
@@ -229,20 +229,14 @@ benchmarks_instance! {
         let origin = RawOrigin::Signed(account_id);
         let mut expected_post = Post::<T, I>::new(&vec![0u8], &vec![0u8]);
         expected_post.increment_replies_counter();
-        assert!(Blog::<T, I>::post_by_id(post_id) == expected_post);
+        assert_eq!(Blog::<T, I>::post_by_id(post_id), expected_post);
         let text = vec![0u8; t.try_into().unwrap()];
-    }: create_reply(
-        origin.clone(),
-        participant_id,
-        post_id,
-        Some(reply_id),
-        text.clone()
-    )
+    }: create_reply(origin.clone(), participant_id, post_id, Some(reply_id), text.clone())
     verify {
         expected_post.increment_replies_counter();
-        assert!(Blog::<T, I>::post_by_id(post_id) == expected_post);
-        assert!(
-            Blog::<T, I>::reply_by_id(post_id, T::ReplyId::one()) ==
+        assert_eq!(Blog::<T, I>::post_by_id(post_id), expected_post);
+        assert_eq!(
+            Blog::<T, I>::reply_by_id(post_id, T::ReplyId::one()),
             Reply::<T, I>::new(
                 text.clone(),
                 participant_id,
@@ -251,7 +245,13 @@ benchmarks_instance! {
         );
 
         assert_last_event::<T, I>(
-            RawEvent::DirectReplyCreated(participant_id, post_id, reply_id, One::one(), text).into()
+            RawEvent::DirectReplyCreated(
+                participant_id,
+                post_id,
+                reply_id,
+                One::one(),
+                text
+            ).into()
         );
     }
 
@@ -263,20 +263,14 @@ benchmarks_instance! {
         let reply_id = generate_reply::<T, I>(account_id.clone(), participant_id, post_id.clone());
         let origin = RawOrigin::Signed(account_id);
         let updated_text = vec![1u8; t.try_into().unwrap()];
-    }: _(
-        origin.clone(),
-        participant_id,
-        post_id,
-        reply_id,
-        updated_text.clone()
-    )
+    }: _(origin.clone(), participant_id, post_id, reply_id, updated_text.clone())
     verify {
         assert_eq!(
             Blog::<T, I>::reply_by_id(post_id, reply_id).text_hash,
             T::Hashing::hash(&updated_text)
         );
-        assert!(
-            Blog::<T, I>::reply_by_id(post_id, reply_id) ==
+        assert_eq!(
+            Blog::<T, I>::reply_by_id(post_id, reply_id),
             Reply::<T, I>::new(
                 updated_text.clone(),
                 participant_id,
@@ -296,13 +290,7 @@ benchmarks_instance! {
         let post_id = generate_post::<T, I>();
         let (account_id, participant_id) = member_funded_account::<T, I>("caller", 0);
         let origin = RawOrigin::Signed(account_id);
-    }: react(
-        origin.clone(),
-        participant_id,
-        0,
-        post_id,
-        None
-        )
+    }: react(origin.clone(), participant_id, 0, post_id, None)
     verify {
         assert_last_event::<T, I>(
             RawEvent::PostReactionsUpdated(participant_id, post_id, 0).into()
@@ -314,13 +302,7 @@ benchmarks_instance! {
         let (account_id, participant_id) = member_funded_account::<T, I>("caller", 0);
         let reply_id = generate_reply::<T, I>(account_id.clone(), participant_id, post_id.clone());
         let origin = RawOrigin::Signed(account_id);
-    }: react(
-        origin.clone(),
-        participant_id,
-        0,
-        post_id,
-        Some(reply_id)
-    )
+    }: react(origin.clone(), participant_id, 0, post_id, Some(reply_id))
     verify {
         assert_last_event::<T, I>(
             RawEvent::ReplyReactionsUpdated(participant_id, post_id, reply_id, 0).into()
@@ -389,4 +371,11 @@ mod tests {
             assert_ok!(test_benchmark_react_to_post::<Runtime>());
         })
     }
+
+    #[test]
+    fn test_react_to_reply() {
+        ExtBuilder::default().build().execute_with(|| {
+            assert_ok!(test_benchmark_react_to_reply::<Runtime>());
+        })
+    }
 }

+ 87 - 4
runtime-modules/blog/src/lib.rs

@@ -122,7 +122,6 @@ pub trait Trait<I: Instance = DefaultInstance>: frame_system::Trait + common::Tr
 }
 
 /// Type, representing blog related post structure
-#[cfg_attr(feature = "std", derive(Debug))]
 #[derive(Encode, Decode, Clone)]
 pub struct Post<T: Trait<I>, I: Instance> {
     /// Locking status
@@ -133,6 +132,20 @@ pub struct Post<T: Trait<I>, I: Instance> {
     replies_count: T::ReplyId,
 }
 
+// Note: we derive it by hand because the derive isn't working because of a Rust problem
+// where the generic parameters need to comply with the bounds instead of the associated traits
+// see: https://github.com/rust-lang/rust/issues/26925
+impl<T: Trait<I>, I: Instance> sp_std::fmt::Debug for Post<T, I> {
+    fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result {
+        f.debug_struct("Post")
+            .field("locked", &self.locked)
+            .field("title_hash", &self.title_hash)
+            .field("body_hash", &self.body_hash)
+            .field("replies_count", &self.replies_count)
+            .finish()
+    }
+}
+
 // Note: we derive it by hand because the derive isn't working because of a Rust problem
 // where the generic parameters need to comply with the bounds instead of the associated traits
 // see: https://github.com/rust-lang/rust/issues/26925
@@ -210,8 +223,7 @@ impl<T: Trait<I>, I: Instance> Post<T, I> {
 }
 
 /// Enum variant, representing either reply or post id
-#[derive(Encode, Decode, Clone, PartialEq)]
-#[cfg_attr(feature = "std", derive(Debug))]
+#[derive(Encode, Decode, Clone, PartialEq, Debug)]
 pub enum ParentId<ReplyId, PostId: Default> {
     Reply(ReplyId),
     Post(PostId),
@@ -226,7 +238,6 @@ impl<ReplyId, PostId: Default> Default for ParentId<ReplyId, PostId> {
 
 /// Type, representing either root post reply or direct reply to reply
 #[derive(Encode, Decode, Clone)]
-#[cfg_attr(feature = "std", derive(Debug))]
 pub struct Reply<T: Trait<I>, I: Instance> {
     /// Reply text hash
     text_hash: T::Hash,
@@ -236,6 +247,19 @@ pub struct Reply<T: Trait<I>, I: Instance> {
     parent_id: ParentId<T::ReplyId, PostId>,
 }
 
+// Note: we derive it by hand because the derive isn't working because of a Rust problem
+// where the generic parameters need to comply with the bounds instead of the associated traits
+// see: https://github.com/rust-lang/rust/issues/26925
+impl<T: Trait<I>, I: Instance> sp_std::fmt::Debug for Reply<T, I> {
+    fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result {
+        f.debug_struct("Reply")
+            .field("text_hash", &self.text_hash)
+            .field("owner", &self.owner)
+            .field("parent_id", &self.parent_id)
+            .finish()
+    }
+}
+
 /// Reply comparator
 // Note: we derive it by hand because the derive isn't working because of a Rust problem
 // where the generic parameters need to comply with the bounds instead of the associated traits
@@ -318,6 +342,16 @@ decl_module! {
         type Error = Error<T, I>;
 
         /// Blog owner can create posts, related to a given blog, if related blog is unlocked
+        ///
+        /// <weight>
+        ///
+        /// ## Weight
+        /// `O (T + B)` where:
+        /// - `T` is the length of the title
+        /// - `B` is the length of the body
+        /// - DB:
+        ///    - O(1) doesn't depend on the state or parameters
+        /// # </weight>
         #[weight = T::WeightInfo::create_post(
                 title.len().saturated_into(),
                 body.len().saturated_into()
@@ -349,6 +383,14 @@ decl_module! {
 
         /// Blog owner can lock posts, related to a given blog,
         /// making post immutable to any actions (replies creation, post editing, reactions, etc.)
+        ///
+        /// <weight>
+        ///
+        /// ## Weight
+        /// `O (1)` doesn't depends on the state or parameters
+        /// - DB:
+        ///    - O(1) doesn't depend on the state or parameters
+        /// # </weight>
         #[weight = T::WeightInfo::lock_post()]
         pub fn lock_post(origin, post_id: PostId) -> DispatchResult {
 
@@ -372,6 +414,14 @@ decl_module! {
 
         /// Blog owner can unlock posts, related to a given blog,
         /// making post accesible to previously forbidden actions
+        ///
+        /// <weight>
+        ///
+        /// ## Weight
+        /// `O (1)` doesn't depends on the state or parameters
+        /// - DB:
+        ///    - O(1) doesn't depend on the state or parameters
+        /// # </weight>
         #[weight = T::WeightInfo::unlock_post()]
         pub fn unlock_post(origin, post_id: PostId) -> DispatchResult {
 
@@ -395,6 +445,15 @@ decl_module! {
 
         /// Blog owner can edit post, related to a given blog (if unlocked)
         /// with a new title and/or body
+        /// <weight>
+        ///
+        /// ## Weight
+        /// `O (T + B)` where:
+        /// - `T` is the length of the `new_title`
+        /// - `B` is the length of the `new_body`
+        /// - DB:
+        ///    - O(1) doesn't depend on the state or parameters
+        /// # </weight>
         #[weight = Module::<T, I>::edit_post_weight(&new_title, &new_body)]
         pub fn edit_post(
             origin,
@@ -427,6 +486,14 @@ decl_module! {
 
         /// Create either root post reply or direct reply to reply
         /// (Only accessible, if related blog and post are unlocked)
+        /// <weight>
+        ///
+        /// ## Weight
+        /// `O (T)` where:
+        /// - `T` is the length of the `text`
+        /// - DB:
+        ///    - O(1) doesn't depend on the state or parameters
+        /// # </weight>
         #[weight = Module::<T, I>::create_reply_weight(text.len())]
         pub fn create_reply(
             origin,
@@ -478,6 +545,15 @@ decl_module! {
 
         /// Reply owner can edit reply with a new text
         /// (Only accessible, if related blog and post are unlocked)
+        ///
+        /// <weight>
+        ///
+        /// ## Weight
+        /// `O (T)` where:
+        /// - `T` is the length of the `new_text`
+        /// - DB:
+        ///    - O(1) doesn't depend on the state or parameters
+        /// # </weight>
         #[weight = T::WeightInfo::edit_reply(new_text.len().saturated_into())]
         pub fn edit_reply(
             origin,
@@ -518,6 +594,13 @@ decl_module! {
 
         /// Submit either post reaction or reply reaction
         /// In case, when you resubmit reaction, it`s status will be changed to an opposite one
+        /// <weight>
+        ///
+        /// ## Weight
+        /// `O (1)` doesn't depends on the state or parameters
+        /// - DB:
+        ///    - O(1) doesn't depend on the state or parameters
+        /// # </weight>
         #[weight = Module::<T, I>::react_weight()]
         pub fn react(
             origin,