Skip to content
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

fix: rename wsurl endpoints to asset hub #1301

Merged
merged 1 commit into from
Jul 5, 2023
Merged

fix: rename wsurl endpoints to asset hub #1301

merged 1 commit into from
Jul 5, 2023

Conversation

Imod7
Copy link
Contributor

@Imod7 Imod7 commented Jul 4, 2023

Description

The changes in this PR are the following :

  1. Replaced the wsUrl endpoints with asset-hub
  2. Replaced asset-hub-<relay_chain> to <relay_chain>-asset-hub strictly in the places that is user-facing

since :

  1. the wsUrl endpoints were also updated in polkadot-js/apps PR #9526 (so we could replace one of the two exceptions from this PR #1296 )
  2. based on the feedback from Joe whatever is related to internal structure we keep as AssetHub<Relay> or asset-hub-<relay> and whatever is related to the "user" should be in the format <relay>-asset-hub.

Assumptions

I made the 2nd change with the assumption that user is also someone who is running the tests. If this assumption is not correct then I will rollback that change.

Tests

Run the following tests :

  • yarn test:latest-e2e-tests --chain polkadot-asset-hub
  • yarn test:historical-e2e-tests --chain polkadot-asset-hub
  • yarn test:historical-e2e-tests --chain kusama-asset-hub

⚠️ Note

I had to update also this line because when I was running
yarn test:latest-e2e-tests --chain polkadot-asset-hub
I was getting the error :

Launching jest...
rimraf e2e-tests/build/
tsc --project e2e-tests/tsconfig.json
usage: index.js [-h]
                [--chain {acala,assetHubPolkadot,karura,kusama,polkadot,westend}]
                [--url URL]
index.js: error: argument --chain: invalid choice: 'asset-hub-polkadot' (choose from 'acala', 'assetHubPolkadot', 'karura', 'kusama', 'polkadot', 'westend')
Killing all processes...
Killing sidecar
Killing latest-e2e
[PASSED] All Tests Passed!

This error is also present in the previous PR #1296 so right now in master branch (if you run yarn test:latest-e2e-tests --chain asset-hub-polkadot) but I didn't catch it and neither did the CI because at the end it says [PASSED] All Tests Passed!

Taking this into account, I was thinking that maybe we could change how we check the tests results. For example, if 0/10 tests were run, maybe we could throw an error also ?

@Imod7 Imod7 requested a review from a team as a code owner July 4, 2023 23:38
Copy link
Collaborator

@marshacb marshacb left a comment

Choose a reason for hiding this comment

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

lgtm!

@Imod7 Imod7 merged commit 7a7e7de into master Jul 5, 2023
7 checks passed
@Imod7 Imod7 deleted the domi-rename-wurls branch July 5, 2023 14:30
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants