Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

add negative tests #11995

Merged
merged 4 commits into from
Aug 26, 2022
Merged

add negative tests #11995

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e1e3aaf
add negative tests
gilescope Aug 8, 2022
d29042a
Merge remote-tracking branch 'origin/pallet-alliance-retirement-notic…
muharem Aug 26, 2022
10729cd
remove tests powerless asserts checking against announcment origin
muharem Aug 26, 2022
6a8e399
assert bad origin from announcement origin checks
muharem Aug 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions frame/alliance/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,9 +441,43 @@ fn retire_works() {
member: (3),
unreserved: None,
}));

// Move time on:
System::set_block_number(System::block_number() + RetirementPeriod::get());

assert_powerless(Origin::signed(3));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are using 3, not 4

});
}

fn assert_powerless(user: Origin) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

I think right not it make sense to check if method supposed to be executed by a member.
For add_unscrupulous_items for example, its not really a case, because its checked by AnnouncementOrigin provided by client of pallet.

If you think it should be different (should check if the caller is a member for example), I think we need a different issue for it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise in this context we are not really checking than the retirement made the use powerless, but more that announcement functionality is allowed only for AnnouncementOrigin

//vote / veto with a valid propsal
let cid = test_cid();
let proposal = make_proposal(42);

assert_noop!(Alliance::init_members(user.clone(), vec![], vec![], vec![]), BadOrigin);

assert_noop!(Alliance::set_rule(user.clone(), cid.clone()), BadOrigin);

assert_noop!(Alliance::retire(user.clone()), Error::<Test, ()>::RetirementNoticeNotGiven);

assert_noop!(Alliance::retirement_notice(user.clone()), Error::<Test, ()>::NotMember);

assert_noop!(Alliance::elevate_ally(user.clone(), 4), BadOrigin);

assert_noop!(Alliance::kick_member(user.clone(), 1), BadOrigin);

assert_noop!(Alliance::nominate_ally(user.clone(), 4), Error::<Test, ()>::NoVotingRights);

assert_noop!(Alliance::propose(user.clone(), 5, Box::new(proposal), 1000), Error::<Test, ()>::NoVotingRights);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gilescope
the last one user.clone() could be just user


// Currently failing below:
assert_noop!(Alliance::add_unscrupulous_items(user.clone(), vec![]), BadOrigin);

assert_noop!(Alliance::remove_unscrupulous_items(user.clone(), vec![]), BadOrigin);

assert_noop!(Alliance::announce(user, cid.clone()), Error::<Test, ()>::NotAlly);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there three checked by AnnouncementOrigin, which is configurable by user, and for these tests its mocked to return true for user 3

}

#[test]
fn kick_member_works() {
new_test_ext().execute_with(|| {
Expand Down