Add tests for xaccounts module (paritytech#694)

* Add some tests Signed-off-by: koushiro <[email protected]> * Add test Signed-off-by: koushiro <[email protected]> * Cargo fmt Signed-off-by: koushiro <[email protected]> * Replace assert with assert_ok and assert_noop Signed-off-by: koushiro <[email protected]> * Cargo fmt Signed-off-by: koushiro <[email protected]>
chainx-org · Jun 17, 2019 · da9b6b0 · da9b6b0
1 parent 34ea817
commit da9b6b0
Show file tree

Hide file tree

Showing 7 changed files with 100 additions and 78 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/xrml/xaccounts/Cargo.toml b/xrml/xaccounts/Cargo.toml
@@ -22,9 +22,6 @@ xsystem = { package ="xrml-xsystem", path = "../../xrml/xsystem", default-featur
 [dev-dependencies]
 # Substrate
 runtime_io = { package = "sr-io", git = "https://github.com/chainpool/substrate" }
-session = { package = "srml-session", git = "https://github.com/chainpool/substrate" }
-timestamp = { package = "srml-timestamp", git = "https://github.com/chainpool/substrate" }
-indices = { package = "srml-indices", git = "https://github.com/chainpool/substrate" }
 
 [features]
 default = ["std"]

diff --git a/xrml/xaccounts/src/lib.rs b/xrml/xaccounts/src/lib.rs
@@ -78,36 +78,32 @@ impl<T: Trait> xsystem::Validator<T::AccountId> for Module<T> {
     }
 }
 
-pub fn is_valid_name<T: Trait>(name: &[u8]) -> Result {
+pub fn is_valid_name(name: &[u8]) -> Result {
     if name.len() > 12 || name.len() < 2 {
         return Err("The length of name must be in range [2, 12].");
     }
 
     Ok(())
 }
 
-pub fn is_valid_about<T: Trait>(about: &[u8]) -> Result {
+pub fn is_valid_about(about: &[u8]) -> Result {
     if about.len() > 128 {
         return Err("The length of about must be in range [0, 128].");
     }
 
     Ok(())
 }
 
-pub fn is_valid_url<T: Trait>(url: &[u8]) -> Result {
+pub fn is_valid_url(url: &[u8]) -> Result {
     if url.len() > 24 || url.len() < 4 {
         return Err("The length of url must be in range [4, 24].");
     }
-    // number, capital letter, lowercase letter, .
-    let is_valid = |n: &u8| -> bool {
-        *n >= 0x30 && *n <= 0x39
-            || *n >= 0x41 && *n <= 0x5A
-            || *n >= 0x61 && *n <= 0x7A
-            || *n == 0x2E
-    };
+    // ASCII alphanumeric character and '.'
+    let is_valid = |n: &u8| -> bool { n.is_ascii_alphanumeric() || *n == b'.' };
 
     if url.iter().filter(|n| !is_valid(n)).count() > 0 {
-        return Err("Only numbers, letters and . are allowed.");
+        return Err("Only ASCII alphanumeric character and . are allowed.");
     }
+
     Ok(())
 }
diff --git a/xrml/xaccounts/src/mock.rs b/xrml/xaccounts/src/mock.rs
@@ -2,14 +2,14 @@
 
 #![cfg(test)]
 
-use super::*;
-
-use primitives::testing::{ConvertUintAuthorityId, Digest, DigestItem, Header, UintAuthorityId};
+use primitives::testing::{Digest, DigestItem, Header, UintAuthorityId};
 use primitives::traits::{BlakeTwo256, IdentityLookup};
 use primitives::BuildStorage;
 use substrate_primitives::{Blake2Hasher, H256};
 use support::impl_outer_origin;
 
+use super::*;
+
 impl_outer_origin! {
     pub enum Origin for Test {}
 }
@@ -26,7 +26,7 @@ impl system::Trait for Test {
     type Hashing = BlakeTwo256;
     type Digest = Digest;
     type AccountId = u64;
-    type Lookup = IdentityLookup<u64>;
+    type Lookup = IdentityLookup<Self::AccountId>;
     type Header = Header;
     type Event = ();
     type Log = DigestItem;
@@ -38,33 +38,13 @@ impl consensus::Trait for Test {
     type InherentOfflineReport = ();
 }
 
-impl indices::Trait for Test {
-    type AccountIndex = u32;
-    type IsDeadAccount = ();
-    type ResolveHint = indices::SimpleResolveHint<Self::AccountId, Self::AccountIndex>;
-    type Event = ();
-}
-
-impl session::Trait for Test {
-    type ConvertAccountIdToSessionKey = ConvertUintAuthorityId;
-    type OnSessionChange = ();
-    type Event = ();
-}
-
-impl timestamp::Trait for Test {
-    type Moment = u64;
-    type OnTimestampSet = ();
-}
-
 impl Trait for Test {
-    type DetermineIntentionJackpotAccountId = MockDeterminator;
+    type DetermineIntentionJackpotAccountId = MockAccountIdDeterminator;
 }
-
-pub struct MockDeterminator;
-
-impl IntentionJackpotAccountIdFor<u64> for MockDeterminator {
+pub struct MockAccountIdDeterminator;
+impl IntentionJackpotAccountIdFor<u64> for MockAccountIdDeterminator {
     fn accountid_for(_: &u64) -> u64 {
-        1000
+        0
     }
 }
 
@@ -82,16 +62,7 @@ pub fn new_test_ext() -> runtime_io::TestExternalities<Blake2Hasher> {
         .unwrap()
         .0,
     );
-    t.extend(
-        session::GenesisConfig::<Test> {
-            session_length: 1,
-            validators: vec![10, 20],
-            keys: vec![],
-        }
-        .build_storage()
-        .unwrap()
-        .0,
-    );
-
     runtime_io::TestExternalities::new(t)
 }
+
+pub type XAccounts = Module<Test>;
diff --git a/xrml/xaccounts/src/tests.rs b/xrml/xaccounts/src/tests.rs
@@ -2,27 +2,88 @@
 
 #![cfg(test)]
 
-use super::mock::*;
-
 use runtime_io::with_externalities;
+use support::StorageMap;
+use support::{assert_noop, assert_ok};
+use xsystem::Validator;
+
+use super::mock::{new_test_ext, Test, XAccounts};
+use super::*;
+
+#[test]
+fn test_is_valid_name() {
+    assert_ok!(is_valid_name("na".as_bytes()));
+    assert_ok!(is_valid_name("nam".as_bytes()));
+    assert_ok!(is_valid_name("name".as_bytes()));
+    assert_ok!(is_valid_name("namenamename".as_bytes()));
+    assert_noop!(
+        is_valid_name("".as_bytes()),
+        "The length of name must be in range [2, 12]."
+    );
+    assert_noop!(
+        is_valid_name("n".as_bytes()),
+        "The length of name must be in range [2, 12]."
+    );
+    assert_noop!(
+        is_valid_name("namenamename_".as_bytes()),
+        "The length of name must be in range [2, 12]."
+    );
+}
+
+#[test]
+fn test_is_valid_about() {
+    assert_ok!(is_valid_about("".as_bytes()));
+    assert_ok!(is_valid_about("about".as_bytes()));
+    assert_ok!(is_valid_about("abcd".repeat(32).as_bytes()));
+    assert_noop!(
+        is_valid_about("abcd".repeat(33).as_bytes()),
+        "The length of about must be in range [0, 128]."
+    );
+}
+
+#[test]
+fn test_is_valid_url() {
+    assert_ok!(is_valid_url(".".repeat(4).as_bytes()));
+    assert_ok!(is_valid_url("1".repeat(4).as_bytes()));
+    assert_ok!(is_valid_url("a.b.c".as_bytes()));
+    assert_ok!(is_valid_url("aaa.bbb.ccc".as_bytes()));
+    assert_ok!(is_valid_url("aaa.aaa.bbb.bbb.ccc.cccc".as_bytes()));
+    assert_ok!(is_valid_url("..12..34..abc..".as_bytes()));
+
+    assert_noop!(
+        is_valid_url(".".repeat(3).as_bytes()),
+        "The length of url must be in range [4, 24]."
+    );
+    assert_noop!(
+        is_valid_url("1".repeat(3).as_bytes()),
+        "The length of url must be in range [4, 24]."
+    );
+    assert_noop!(
+        is_valid_url("a".repeat(3).as_bytes()),
+        "The length of url must be in range [4, 24]."
+    );
+    assert_noop!(
+        is_valid_url("abcde".repeat(5).as_bytes()),
+        "The length of url must be in range [4, 24]."
+    );
+    assert_noop!(
+        is_valid_url("http://a.b.c".as_bytes()),
+        "Only ASCII alphanumeric character and . are allowed."
+    );
+    assert_noop!(
+        is_valid_url("https://a.b.c".as_bytes()),
+        "Only ASCII alphanumeric character and . are allowed."
+    );
+}
 
 #[test]
-fn issue_should_work() {
+fn validator_should_work() {
     with_externalities(&mut new_test_ext(), || {
-        //        System::set_block_number(10);
-        //        assert_ok!(XAccounts::issue(b"alice".to_vec(), 1, 1));
-        //        assert_eq!(XAccounts::total_issued(), 3);
-        //        assert_eq!(
-        //            XAccounts::cert_immutable_props_of(b"alice".to_vec()),
-        //            CertImmutableProps {
-        //                issued_at: 10,
-        //                frozen_duration: 1
-        //            }
-        //        );
-        //        assert_eq!(XAccounts::remaining_shares_of(b"alice".to_vec()), 50);
-        //        assert_noop!(
-        //            XAccounts::issue(b"alice".to_vec(), 1, 1),
-        //            "Cannot issue if this cert name already exists."
-        //        );
+        assert_eq!(XAccounts::intention_of(b"test".to_vec()), None);
+        assert_eq!(XAccounts::intention_name_of(1), None);
+        <IntentionOf<Test>>::insert(b"test".to_vec(), 1);
+        <IntentionNameOf<Test>>::insert(1, b"test".to_vec());
+        assert_eq!(XAccounts::get_validator_by_name(&b"test".to_vec()), Some(1));
+        assert_eq!(XAccounts::get_validator_name(&1), Some(b"test".to_vec()));
     });
 }
diff --git a/xrml/xbridge/features/src/lib.rs b/xrml/xbridge/features/src/lib.rs
@@ -163,7 +163,7 @@ impl<T: Trait> Module<T> {
             xaccounts::Module::<T>::is_intention(&who),
             "Transactor is not an intention."
         );
-        xaccounts::is_valid_about::<T>(&about)?;
+        xaccounts::is_valid_about(&about)?;
 
         let hot_pubkey = xbitcoin::Module::<T>::check_trustee_entity(hot_entity.as_ref())?;
         let cold_pubkey = xbitcoin::Module::<T>::check_trustee_entity(cold_entity.as_ref())?;

diff --git a/xrml/xmining/staking/src/lib.rs b/xrml/xmining/staking/src/lib.rs
@@ -195,11 +195,11 @@ decl_module! {
             ensure!(Self::is_intention(&who), "Cannot refresh if transactor is not an intention.");
 
             if let Some(url) = url.as_ref() {
-                xaccounts::is_valid_url::<T>(url)?;
+                xaccounts::is_valid_url(url)?;
             }
 
             if let Some(about) = about.as_ref() {
-                xaccounts::is_valid_about::<T>(about)?;
+                xaccounts::is_valid_about(about)?;
             }
 
             if let Some(desire_to_run) = desire_to_run.as_ref() {
@@ -224,7 +224,7 @@ decl_module! {
         fn register(origin, name: Name) {
             let who = ensure_signed(origin)?;
 
-            xaccounts::is_valid_name::<T>(&name)?;
+            xaccounts::is_valid_name(&name)?;
 
             ensure!(!Self::is_intention(&who), "Cannot register if transactor is an intention already.");
             ensure!(!Self::name_exists(name.clone()), "This name has already been taken.");