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

Fix experimental aura config and comment tests #3057

Merged
merged 4 commits into from
Aug 24, 2023

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Aug 24, 2023

(preparation for the monorepo)

Changes:

  • When the experimental feature is enabled in the workspace, then pallet-aura expects an additional config item.
  • Comment out some tests that fail when the runtime-benchmarks feature is enabled in the workspace. Fix all ignored system parachain integration tests #3027 is linked in the code.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez changed the title Fix experimental aura config Fix experimental aura config Aug 24, 2023
@ggwpez ggwpez added B0-silent Changes should not be mentioned in any release notes A3-in_progress Pull request is in progress. No review needed at this stage. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Aug 24, 2023
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez changed the title Fix experimental aura config Fix experimental aura config and comment tests Aug 24, 2023
@ggwpez ggwpez self-assigned this Aug 24, 2023
@@ -185,6 +185,7 @@ fn limited_teleport_native_assets_from_relay_to_system_para_works() {
/// Limited Teleport of native asset from System Parachain to Relay Chain
/// should work when there is enough balance in Relay Chain's `CheckAccount`
#[test]
#[cfg(feature = "FIXME-IGNORED")] // <https://github.com/paritytech/cumulus/issues/3027>
Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed seems fine as long as runtime-benchmarks is not enabled:

Suggested change
#[cfg(feature = "FIXME-IGNORED")] // <https://github.com/paritytech/cumulus/issues/3027>
#[cfg(not(feature = "runtime-benchmarks"))] // <https://github.com/paritytech/cumulus/issues/3027>

Copy link
Member Author

Choose a reason for hiding this comment

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

The crate does not have that feature and is also not propagating it downwards...

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez marked this pull request as ready for review August 24, 2023 15:41
@ggwpez ggwpez added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Aug 24, 2023
@paritytech-ci paritytech-ci requested review from a team August 24, 2023 15:42
@@ -124,6 +124,8 @@ impl pallet_aura::Config for Test {
type MaxAuthorities = ConstU32<100_000>;
type DisabledValidators = ();
type AllowMultipleBlocksPerSlot = ConstBool<false>;
#[cfg(feature = "experimental")]
type SlotDuration = pallet_aura::MinimumPeriodTimesTwo<Self>;
Copy link
Member

Choose a reason for hiding this comment

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

Is there a difference between this and ConstU64<SLOT_DURATION>? @rphmeier

Copy link
Contributor

@rphmeier rphmeier Aug 24, 2023

Choose a reason for hiding this comment

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

Not in practice. Though semantically they have different meanings.

Copy link
Member Author

Choose a reason for hiding this comment

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

Does the change look good to you @rphmeier? The MR is looking for review approvals since we want to do the monorepo migration tomorrow.

Copy link
Contributor

@rphmeier rphmeier left a comment

Choose a reason for hiding this comment

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

Looks fine to me

@ggwpez
Copy link
Member Author

ggwpez commented Aug 24, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 60171c6 into master Aug 24, 2023
50 of 51 checks passed
@paritytech-processbot paritytech-processbot bot deleted the oty-fix-aura-config branch August 24, 2023 18:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants