Timestamp::set_timestamp does not set DidUpdate

[kylezs]

Is there an existing issue?

• I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

• This is not a support question.

Description of bug

When runtime/integration testing using AllPalletsWithSystem::on_initialize()it makes sense to also use AllPalletsWithSystem::on_finalize(), and require timestamp updates as you progress in blocks. However, because Timestamp::set_timestamp does not also set DidUpdate to true, the on_finalize for the Timestamp pallet fails, as it is just an assert

fn on_finalize(_n: BlockNumberFor<T>) {
	assert!(DidUpdate::<T>::take(), "Timestamp must be updated once in the block");
}

I have a feeling there might be a reason for this, but it does strike me as a little strange, and at the very least, there should be something that could be done to improve this, potentially by improving the flexibility of AllPallets... or something of that nature.

Steps to reproduce

No response

[shawntabrizi]

This is for a unit test? Why not just set this variable to true manually?

[kylezs]

Yeah, I could, my point is more that the utility (AllPallets...) isn't as useful/intuitive as it could be if I have to do manual steps in between... and I have to know which manual steps to do. Defeats the purpose of the utility a little.

[bkchr]

Yeah, I also think that this utility function is a little bit weird. I'm open a pr to fix this.