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

refactor: replace rococo to paseo name #333

Merged
merged 2 commits into from
Nov 21, 2024

Conversation

CocDap
Copy link
Contributor

@CocDap CocDap commented Nov 5, 2024

Relevant issue: #322

Two manually check on local:

  1. Run pop new parachain my-para . Check relay chain is paseo runtime on the polkadot js explorer ✅
image
  1. Run cargo test
image

P/S: I am very happy to contribute for the first time to this project

@AlexD10S AlexD10S self-requested a review November 6, 2024 10:26
@AlexD10S
Copy link
Collaborator

AlexD10S commented Nov 6, 2024

Thank you for your contribution! Really cool.
The test failing in the image is because you probably have a parachain running in the port 9944 and is the one the test is using for testing.

I run the CI to run all the unit and integration tests and are failing.
It doesn't seem is something to change in your code, the integration tests https://github.com/r0gue-io/pop-cli/actions/runs/11682223509/job/32588340601?pr=333 seems something is missing in the Paseo chain spec generator: "Unknown chain, only supported: paseo-dev, asset-hub-paseo-local, paseo-local, bridge-hub-paseo, people-paseo, bridge-hub-paseo-local, people-paseo-local or a json file"

Let me investigate it.

@CocDap
Copy link
Contributor Author

CocDap commented Nov 7, 2024

Thank you for reviewing this PR. I believe I've identified the issue, which originates from the launch_paseo_and_system_parachain test case. The original name is launch_rococo_and_system_parachain

It seems that Zombienet only supports coretime-rococo-local and does not recognize coretime-paseo-local
Code: https://github.com/CocDap/pop-cli/blob/chore/replace-rococo-to-paseo/tests/networks/paseo%2Bcoretime.toml#L16
CC: @AlexD10S

Copy link
Collaborator

@AlexD10S AlexD10S left a comment

Choose a reason for hiding this comment

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

@CocDap I’ve rerun the integration tests after the Paseo v1.3.3 release, and it looks like the issue is resolved ,thanks for your patience!

However, the unit tests still need some adjustments. I left a comment about the chain_spec_generator_returns_none_when_no_match test you changed.

And the other one it will need adjustment is binaries_works (https://github.com/r0gue-io/pop-cli/blob/main/crates/pop-parachains/src/up/mod.rs#L1238), note that the Paseo chain spec generators are now binaries we are fetching increasing the binaries count to 6 instead of 4. Just change the following line(https://github.com/r0gue-io/pop-cli/blob/main/crates/pop-parachains/src/up/mod.rs#L1272), to:

assert_eq!(zombienet.binaries().count(), 6);

crates/pop-parachains/src/up/chain_specs.rs Outdated Show resolved Hide resolved
Copy link

codecov bot commented Nov 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.47%. Comparing base (2af82eb) to head (9d21c84).
Report is 25 commits behind head on main.

@@            Coverage Diff             @@
##             main     #333      +/-   ##
==========================================
+ Coverage   70.33%   70.47%   +0.14%     
==========================================
  Files          53       54       +1     
  Lines        9098     9250     +152     
  Branches     9098     9250     +152     
==========================================
+ Hits         6399     6519     +120     
- Misses       1718     1737      +19     
- Partials      981      994      +13     
Files with missing lines Coverage Δ
crates/pop-parachains/src/build.rs 88.77% <100.00%> (ø)
crates/pop-parachains/src/up/mod.rs 87.18% <100.00%> (-0.29%) ⬇️
crates/pop-parachains/src/up/parachains.rs 86.38% <100.00%> (+0.14%) ⬆️
crates/pop-parachains/src/up/relay.rs 85.93% <100.00%> (+0.33%) ⬆️

... and 12 files with indirect coverage changes


🚨 Try these New Features:

@CocDap
Copy link
Contributor Author

CocDap commented Nov 19, 2024

It seems that the launch_paseo_and_system_parachain test case continues to fail on MacOS.
CC: @AlexD10S

@AlexD10S
Copy link
Collaborator

It seems that the launch_paseo_and_system_parachain test case continues to fail on MacOS. CC: @AlexD10S

Thanks for the changes. I don't think is anything you have to change in the code, and more the chain generator of Paseo. I am going to investigate why only fails in MacOS.

@AlexD10S AlexD10S self-requested a review November 20, 2024 09:55
@AlexD10S
Copy link
Collaborator

Fixed! Thanks again fro contributing @CocDap

@AlexD10S AlexD10S merged commit 258a4b2 into r0gue-io:main Nov 21, 2024
19 checks passed
@AlexD10S AlexD10S changed the title chore: replace rococo to paseo name refactor: replace rococo to paseo name Nov 21, 2024
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.

2 participants