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

Fix XCM bench for new substrate refs #6663

Merged
merged 4 commits into from
Feb 2, 2023

Conversation

acatangiu
Copy link
Contributor

No description provided.

@acatangiu
Copy link
Contributor Author

@muharem @ggwpez @bkchr Polkadot companion #6598 for paritytech/substrate#13146 passed CI and was merged successfully, but now all new CI jobs are failing because of it.

I can't figure out why exactly though.. test is complaining try_successful_origin not implemented in xcm/xcm-builder/src/origin_conversion.rs:247, but I see it actually is.

Both the trait fn definition and said impl are guarded by same #[cfg(feature = "runtime-benchmarks")] so that doesn't seem to be the issue either (unless we somehow compile xcm-builder without feature = "runtime-benchmarks" but compile frame-support (trait definition) with feature = "runtime-benchmarks" ???)

Any ideas?

@acatangiu
Copy link
Contributor Author

acatangiu commented Feb 2, 2023

@ggwpez Failing test: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2344373 (same test fails on all PRs)

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@acatangiu acatangiu changed the title [DNM] testing CI with new substrate refs Fix XCM bench for new substrate refs Feb 2, 2023
@acatangiu acatangiu requested a review from ggwpez February 2, 2023 12:46
@acatangiu acatangiu self-assigned this Feb 2, 2023
@acatangiu acatangiu added 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. labels Feb 2, 2023
@acatangiu acatangiu requested a review from a team February 2, 2023 12:47
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable
Logs: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2344758

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@acatangiu acatangiu removed the request for review from ggwpez February 2, 2023 13:09
Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

Looks like this is now blocked by #6528 on the cumulus check.
Probably have to ignore it.

Or maybe you can revert your Cargo update.

@ggwpez
Copy link
Member

ggwpez commented Feb 2, 2023

@muharem now one of the referenda benches fails.
PS: He is OOO… will it fix myself .

@acatangiu
Copy link
Contributor Author

Looks like this is now blocked by #6528 on the cumulus check. Probably have to ignore it.

Or maybe you can revert your Cargo update.

Also weird because the used substrate reference is one before needing #6528

@acatangiu
Copy link
Contributor Author

I'm traveling rn so have limited availability. Feel free to change substrate refs if needed to get this to pass, the point is to unblock CI for all PRs

@ggwpez
Copy link
Member

ggwpez commented Feb 2, 2023

I cannot update substrate here as it says beefy-merkle-tree, so well have to merge red and then fix it in #6528

@ggwpez ggwpez merged commit 907604b into paritytech:master Feb 2, 2023
@acatangiu
Copy link
Contributor Author

I had just pulled all of these in #6528, but it works either way.

Thanks @ggwpez !

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants