fix: Enable testing and benchmarking together in Roles pallet

pallets/roles/Cargo.toml

@@ -34,6 +34,7 @@ try-runtime = ['frame-support/try-runtime']
 [dependencies]
 codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] }
 scale-info = { version = "2.1.2", default-features = false, features = ["derive"] }
+cfg-if = "1.0.0"
 
 # Local dependencies
 pallet-permissions = { default-features = false, path = '../permissions' }

pallets/roles/src/benchmarking.rs

@@ -4,27 +4,48 @@
 
 // FIXME: refactor once SpacesInterface is added.
 
-use super::*;
+use cfg_if::cfg_if;
 use frame_benchmarking::{account, benchmarks};
 use frame_support::dispatch::DispatchError;
 use frame_system::RawOrigin;
-use pallet_permissions::SpacePermission as SP;
-use pallet_spaces::types::Space;
 use sp_std::{prelude::Vec, vec};
-use subsocial_support::{Content, User};
-use subsocial_support::mock_functions::{valid_content_ipfs, another_valid_content_ipfs};
 
-fn create_dummy_space<T: Config + pallet_spaces::Config>(
-    origin: RawOrigin<T::AccountId>,
-) -> Result<Space<T>, DispatchError> {
-    let space_id = pallet_spaces::NextSpaceId::<T>::get();
+use pallet_permissions::SpacePermission as SP;
+use subsocial_support::{
+    mock_functions::{another_valid_content_ipfs, valid_content_ipfs},
+    Content, User,
+};
+
+use super::*;
+
+fn get_dummy_space_id<T: Config + pallet_spaces::Config>(
+    #[allow(unused_variables)] origin: RawOrigin<T::AccountId>,
+) -> Result<SpaceId, DispatchError> {
+    cfg_if! {
+        if #[cfg(test)] {
+            Ok(crate::mock::SPACE1)
+        } else {
+            let space_id = pallet_spaces::NextSpaceId::<T>::get();
 
-    pallet_spaces::Pallet::<T>::create_space(origin.into(), Content::None, None)?;
+            pallet_spaces::Pallet::<T>::create_space(origin.into(), Content::None, None)?;
 
-    let space = pallet_spaces::SpaceById::<T>::get(space_id)
-        .ok_or(DispatchError::Other("Space not found"))?;
+            let space = pallet_spaces::SpaceById::<T>::get(space_id)
+                .ok_or(DispatchError::Other("Space not found"))?;
 
-    Ok(space)
+            Ok(space.id)
+        }
+    }
+}
+
+fn get_caller_account<T: Config>() -> T::AccountId {
+    cfg_if! {
+        if #[cfg(test)] {
+            let mut bytes: &[u8] = &crate::mock::ACCOUNT1.to_le_bytes();
+            T::AccountId::decode(&mut bytes).expect("failed to get caller_account")
+        } else {
+            account::<T::AccountId>("Acc1", 1, 0)
+        }
+    }
 }
 
 fn dummy_list_of_users<T: Config>(num_of_users: u32) -> Vec<User<T::AccountId>> {
@@ -68,25 +89,25 @@ benchmarks! {
     where_clause { where T: pallet_spaces::Config }
 
     create_role {
-        let caller_origin = RawOrigin::Signed(account::<T::AccountId>("Acc1", 1, 0));
-        let space = create_dummy_space::<T>(caller_origin.clone())?;
+        let caller_origin = RawOrigin::Signed(get_caller_account::<T>());
+        let space_id = get_dummy_space_id::<T>(caller_origin.clone())?;
         let time_to_live: Option<T::BlockNumber> = Some(100u32.into());
         let content = valid_content_ipfs();
         let perms = vec![SP::ManageRoles];
         let role_id = NextRoleId::<T>::get();
-    }: _(caller_origin, space.id, time_to_live, content, perms)
+    }: _(caller_origin, space_id, time_to_live, content, perms)
     verify {
         let role = RoleById::<T>::get(role_id).unwrap();
-        let space_roles_ids = RoleIdsBySpaceId::<T>::get(space.id);
+        let space_roles_ids = RoleIdsBySpaceId::<T>::get(space_id);
 
         ensure!(role.id == role_id, "Role id doesn't match");
         ensure!(space_roles_ids.contains(&role_id), "Role id not in space roles");
     }
 
     update_role {
-        let caller_origin = RawOrigin::Signed(account::<T::AccountId>("Acc1", 1, 0));
-        let space = create_dummy_space::<T>(caller_origin.clone())?;
-        let (role, _) = create_dummy_role::<T>(caller_origin.clone(), space.id, 10)?;
+        let caller_origin = RawOrigin::Signed(get_caller_account::<T>());
+        let space_id = get_dummy_space_id::<T>(caller_origin.clone())?;
+        let (role, _) = create_dummy_role::<T>(caller_origin.clone(), space_id, 10)?;
 
         ensure!(!role.disabled, "Role should be enabled");
 
@@ -103,9 +124,9 @@ benchmarks! {
 
     delete_role {
         let x in 0..T::MaxUsersToProcessPerDeleteRole::get().into();
-        let caller_origin = RawOrigin::Signed(account::<T::AccountId>("Acc1", 1, 0));
-        let space = create_dummy_space::<T>(caller_origin.clone())?;
-        let (role, _) = create_dummy_role::<T>(caller_origin.clone(), space.id, x)?;
+        let caller_origin = RawOrigin::Signed(get_caller_account::<T>());
+        let space_id = get_dummy_space_id::<T>(caller_origin.clone())?;
+        let (role, _) = create_dummy_role::<T>(caller_origin.clone(), space_id, x)?;
     }: _(caller_origin, role.id, x)
     verify {
         let deleted = RoleById::<T>::get(role.id).is_none();
@@ -114,9 +135,9 @@ benchmarks! {
 
     grant_role {
         let x in 1..500;
-        let caller_origin = RawOrigin::Signed(account::<T::AccountId>("Acc1", 1, 0));
-        let space = create_dummy_space::<T>(caller_origin.clone())?;
-        let (role, _) = create_dummy_role::<T>(caller_origin.clone(), space.id, 0)?;
+        let caller_origin = RawOrigin::Signed(get_caller_account::<T>());
+        let space_id = get_dummy_space_id::<T>(caller_origin.clone())?;
+        let (role, _) = create_dummy_role::<T>(caller_origin.clone(), space_id, 0)?;
 
         let users_to_grant = dummy_list_of_users::<T>(x);
     }: _(caller_origin, role.id, users_to_grant.clone())
@@ -129,9 +150,9 @@ benchmarks! {
 
     revoke_role {
         let x in 1..500;
-        let caller_origin = RawOrigin::Signed(account::<T::AccountId>("Acc1", 1, 0));
-        let space = create_dummy_space::<T>(caller_origin.clone())?;
-        let (role, users_to_revoke) = create_dummy_role::<T>(caller_origin.clone(), space.id, x)?;
+        let caller_origin = RawOrigin::Signed(get_caller_account::<T>());
+        let space_id = get_dummy_space_id::<T>(caller_origin.clone())?;
+        let (role, users_to_revoke) = create_dummy_role::<T>(caller_origin.clone(), space_id, x)?;
     }: _(caller_origin, role.id, users_to_revoke)
     verify {
         let granted_users = UsersByRoleId::<T>::get(role.id);

pallets/roles/src/lib.rs

@@ -17,6 +17,7 @@ use scale_info::TypeInfo;
 use sp_runtime::RuntimeDebug;
 use sp_std::{collections::btree_set::BTreeSet, prelude::*};
 
+pub use pallet::*;
 use pallet_permissions::{
     Pallet as Permissions, PermissionChecker, SpacePermission, SpacePermissionSet,
 };
@@ -25,32 +26,34 @@ use subsocial_support::{
     traits::{IsAccountBlocked, IsContentBlocked, SpaceFollowsProvider, SpacePermissionsProvider},
     Content, ModerationError, SpaceId, User, WhoAndWhenOf,
 };
+pub use types::*;
 
-pub use pallet::*;
 pub mod functions;
 
 pub mod types;
-pub use types::*;
 // pub mod rpc;
 
 #[cfg(test)]
 mod mock;
 
 #[cfg(feature = "runtime-benchmarks")]
 mod benchmarking;
-#[cfg(all(test, not(feature = "runtime-benchmarks")))]
+#[cfg(test)]
 mod tests;
 pub mod weights;
 
 #[frame_support::pallet]
 pub mod pallet {
-    use super::*;
-    use crate::weights::WeightInfo;
     use frame_support::pallet_prelude::*;
     use frame_system::pallet_prelude::*;
+
     use pallet_permissions::SpacePermissionsInfoOf;
     use subsocial_support::{remove_from_vec, WhoAndWhen};
 
+    use crate::weights::WeightInfo;
+
+    use super::*;
+
     #[pallet::config]
     pub trait Config:
         frame_system::Config + pallet_permissions::Config + pallet_timestamp::Config

pallets/roles/src/mock.rs

@@ -1,28 +1,30 @@
-use super::*;
-
-use sp_core::H256;
-use sp_io::TestExternalities;
-use sp_std::{collections::btree_set::BTreeSet, prelude::Vec};
-
 use frame_support::{
     assert_ok,
     dispatch::{DispatchError, DispatchResult},
     parameter_types,
     traits::{ConstU32, Everything},
 };
+use sp_core::H256;
+use sp_io::TestExternalities;
 use sp_runtime::{
     testing::Header,
     traits::{BlakeTwo256, IdentityLookup},
 };
+use sp_std::{collections::btree_set::BTreeSet, prelude::Vec};
 
-use pallet_permissions::{SpacePermission, SpacePermission as SP, SpacePermissions};
+use pallet_permissions::{
+    default_permissions::DefaultSpacePermissions, SpacePermission, SpacePermission as SP,
+    SpacePermissions,
+};
 use subsocial_support::{
     traits::{SpaceFollowsProvider, SpacePermissionsProvider as SpacePermissionsProviderT},
     Content, SpaceId, SpacePermissionsInfo, User,
 };
 
 use crate as roles;
 
+use super::*;
+
 type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
 type Block = frame_system::mocking::MockBlock<Test>;
 
@@ -103,8 +105,6 @@ impl pallet_balances::Config for Test {
     type ReserveIdentifier = ();
 }
 
-use pallet_permissions::default_permissions::DefaultSpacePermissions;
-
 impl pallet_permissions::Config for Test {
     type DefaultSpacePermissions = DefaultSpacePermissions;
 }
@@ -116,9 +116,6 @@ parameter_types! {
 impl Config for Test {
     type Event = Event;
     type MaxUsersToProcessPerDeleteRole = MaxUsersToProcessPerDeleteRole;
-    #[cfg(feature = "runtime-benchmarks")]
-    type SpacePermissionsProvider = Spaces;
-    #[cfg(not(feature = "runtime-benchmarks"))]
     type SpacePermissionsProvider = Self;
     type SpaceFollows = Roles;
     type IsAccountBlocked = ();