-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: e2e tests ah between current spiritnet runtime #668
Conversation
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.
Minor comments. These tests definitely need a overhaul, and am happy to see some work has gone into that already.
@@ -26,6 +26,14 @@ export function setGovernance(addr: string[]) { | |||
} | |||
} | |||
|
|||
export function setSafeXcmVersion3() { |
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.
This only returns something, right? It does not set
anything, as far as I can see.
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.
Yes, not the best naming. I renamed in: 3c3d182
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.
Let's go with getSafeXcmVersion3StorageEntry
, and make it unambiguous.
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.
Or XcmPalletSafeVersion3StorageEntry
.
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.
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.
I guess TS practice is to start the name lowercase 😇
...ks/src/tests/xcm/assetHub/__snapshots__/limitedReserveTransferAssetHubSpiritnet.test.ts.snap
Outdated
Show resolved
Hide resolved
...tion-tests/chopsticks/src/tests/xcm/assetHub/limitedReserveTransferAssetHubSpiritnet.test.ts
Show resolved
Hide resolved
...tion-tests/chopsticks/src/tests/xcm/assetHub/limitedReserveTransferSpiritnetAssetHub.test.ts
Outdated
Show resolved
Hide resolved
2140bc7
to
9e3487e
Compare
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.
One minor thing still open, otherwise looks good! We can go ahead with the new channel opening proposal.
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.
Actually let's try to fix the CI as well.
…t-node into ag_ah_integration_tests
Updated snapshots in: 5f11861 |
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.
Let's keep ignoring the broken integration tests 😎😎😎
## fixes [#3507](KILTprotocol/ticket#3507) Checks possible interaction between AH and Spiritnet. In the emulated tests, it is checked that messages with the `Transact` instruction are filtered out. Currently, there are only tests for extrinsics which require sudo rights. If it is wished, I can add a test for that too.
fixes #3507
Checks possible interaction between AH and Spiritnet. In the emulated tests, it is checked that messages with the
Transact
instruction are filtered out. Currently, there are only tests for extrinsics which require sudo rights. If it is wished, I can add a test for that too.