-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Conversation
let _sender = ensure_signed(origin)?; | ||
Value::put(n); | ||
Ok(()) | ||
mod pallet_test { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here the pallet is declared in its own module, but nothing has changed
verify { | ||
assert_eq!(Value::get(), Some(b)); | ||
} | ||
mod benchmarks { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here benchmark macro is called in its own module because benchmark macro uses in scope Call
for the pallet call. But construct_runtime already defines the outer Call
.
the benchmark call and the test below haven't been changed
|
||
pub const CALL: &<Test as Config>::Call = &Call; | ||
/// A simple call, which one doesn't matter. | ||
pub const CALL: &<Test as Config>::Call = &Call::System(frame_system::Call::set_heap_pages(0u64)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
previously Call was a struct which panics in its implementation of dispatchable and was meant to be not used.
Using a random call doesn't change the nature of the test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to somehow also warn if someone uses PalletInfo = ()
, because basically now this is now dangerous if you use it in any test setup that has multiple pallets.
yes but we can't deprecate a trait implementation, we can only warn during execution by printing some message, or remove it. |
bot merge |
Waiting for commit status. |
Checks failed; merge aborted. |
bot merge |
Waiting for commit status. |
Why not? |
I think adding a manual line of warning in the impl is also capable of preventing some people messing up their test setup :D |
I was thinking that once all |
💯 |
related #7949