add zombienet-sdk test to parachain-template #5342

pepoviola · 2024-08-13T11:03:23Z

Add new CI machinery to smoke test the parachain-template-node using zombienet-sdk.
Thx!

rzadp · 2024-08-13T11:32:00Z

Great news!

I see that the network config is defined inside the smoke test, so it doesn't really test that the zombienet.toml file, right? Would it be possible to change that?

I was actually working on a CI workflow in the separate template repo based on the old zombienet, which would be a part of the clone'able template as a starter point, and that would use zombienet.toml.

Do you think this zombienet-sdk test should be a part of the clone'able template, or only used here to test the binaries?
If the first - we should update the instructions and the config file from zombienet to zombienet-sdk.
If the second - we should have this new crate as a sibling of the template, so it doesn't get synchronized.

rzadp · 2024-08-13T12:01:14Z

Oh and one more - do you think it would make sense to add a similar smoke test for the other two templates (only asserting the state of the relaychains)?

kianenigma

Would be great if you also check the run instructions in the README to be simple and correct.

kianenigma · 2024-08-13T13:45:01Z

Oh and one more - do you think it would make sense to add a similar smoke test for the other two templates (only asserting the state of the relaychains)?

I was thinking of the same, and initially though it is not needed as you would usually run them with a single node, but better add it everywhere.

for the solochain I think it is a good idea.

for minimal, it makes less sense as it is meant to run with just one node. @pepoviola can try it out and see if it makes sense, but I think it doesn't.

pepoviola · 2024-08-13T16:42:03Z

Hi @rzadp / @kianenigma, thanks for the feedback! I will track these changes here:

Move the test to be a sibling of the other directories (parachain / minimal / etc).
Add a simple smoke test for every version (parachain / solochain / minimal).
Update readme with instructions to run everywhere.
feature gate the tests.

Thx!

bkchr · 2024-08-13T21:13:16Z

templates/parachain/zombienet/tests/smoke.rs

+//! This test is setup to run with the `native` provider and needs these binaries in your PATH
+//! `polkadot`, `polkadot-prepare-worker`, `polkadot-execute-worker`, `parachain-template-node`.
+//! You can follow these steps to compile and export the binaries:
+//! `cargo build --release -features fast-runtime --bin polkadot --bin polkadot-execute-worker --bin
+//! polkadot-prepare-worker`
+//! `cargo build --package parachain-template-node --release`
+//! `export PATH=<path-to-polkadot-sdk-repo>/target/release:$PATH
+//!
+//! The you can run the test with
+//! `cargo test -p parachain-template-zombienet`


So if someone is running cargo test in the repo, it will fail. Then they will be confused and at some point maybe find these docs. TBH, this is not any nice experience. Generally for running these tests it is not a nice experience.

I still don't get the need for writing them in Rust. This just makes it even more complicated than it already is.

Hi @bkchr, thanks for the feedback. I'm agree that the dx needs to improve and isn't in the final shape. Wdyt about make this test feature gated and even if you pass the feature run some sanity check to ensure the preconditions (e.g the binaries) are availables and if not print a nice guide about the requirements?

i agree, it should probably be something like cargo test --features zombienet

Not sure if i have an opinion of rust / not rust.

it is nice that you dont need anything but rust to run zombienet in this situation.

rust is a bit more complicated, but that should not be limiting factor for developers who want to do this anyway.

shawntabrizi · 2024-08-14T16:40:50Z

templates/parachain/zombienet/tests/smoke.rs

+//! `cargo build --release -features fast-runtime --bin polkadot --bin polkadot-execute-worker --bin
+//! polkadot-prepare-worker`


Suggested change

//! `cargo build --release -features fast-runtime --bin polkadot --bin polkadot-execute-worker --bin

//! polkadot-prepare-worker`

//! `cargo install --git https://github.com/paritytech/polkadot-sdk/ --tag polkadot-stable2407

//! polkadot polkadot-execute-worker polkadot-prepare-worker`

Ideally we could automatically replace the tag here as the template updates

shawntabrizi · 2024-08-14T16:41:04Z

templates/parachain/zombienet/tests/smoke.rs

+//! `cargo build --release -features fast-runtime --bin polkadot --bin polkadot-execute-worker --bin
+//! polkadot-prepare-worker`
+//! `cargo build --package parachain-template-node --release`
+//! `export PATH=<path-to-polkadot-sdk-repo>/target/release:$PATH


Suggested change

//! `export PATH=<path-to-polkadot-sdk-repo>/target/release:$PATH

You still need to have <path-to-polkadot-sdk-repo>/target/release in your PATH in order to run parachain-template-node, minimal-template-node, etc.

Co-authored-by: Bastian Köcher <git@kchr.de>

templates/zombienet/src/lib.rs

kianenigma · 2024-08-15T16:23:36Z

templates/zombienet/Cargo.toml

+zombienet-sdk = "0.2.6"
+
+[features]
+zombienet = []


Why does this have a separate feature?

The idea is to hide this integration test behind a feature, since require some extra artifacts and steps and we don't want to mess with people running cargo test.

But if you run cargo test in a template, for example ./templates/parachain, this crate is not part of your working tree and is not tested anyways??

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

paritytech-cicd-pr · 2024-08-20T12:03:21Z

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

pepoviola · 2024-08-21T13:49:26Z

ping @kianenigma / @shawntabrizi can we merge this one? NOTE: I need to fix the sdk in order to work with the solochain template.
Thx!

templates/parachain/zombienet.toml

kianenigma · 2024-08-23T13:20:40Z

templates/zombienet/Cargo.toml

+zombienet-sdk = "0.2.8"
+
+[features]
+zombienet = []


Again, this is not needed, as this crate is no longer part of templates/parachain. If you run cargo test in templates/parachain, this crate is not called.

Thanks for the feedback @kianenigma! The idea behind the feature flag is to prevent failures in these tests for users that run cargo test --workspace in the polkadot-sdk repo, since this tests need some extra steps (build the binaries) in order to run. So, I think we can keep the feature and set in our CI to ensure block production for the templates.

Thx!

kianenigma · 2024-08-23T13:20:52Z

templates/zombienet/tests/smoke.rs

+//! The you can run the test with
+//! `cargo test -p template-zombienet-tests`
+
+#[cfg(feature = "zombienet")]


Suggested change

#[cfg(feature = "zombienet")]

kianenigma

The feature needs to be removed, otherwise looks good.

pepoviola · 2024-09-12T17:33:49Z

Hi @kianenigma, just re-iterating on this. The feature flag is to prevent failures in these tests for users that run cargo test --workspace in the polkadot-sdk repo (without cding to the templates directory), since this tests need some extra steps (build the binaries) in order to run. So, I think we can keep the feature and set in our CI to ensure block production for the templates. wdyt?

Thx!

rzadp and others added 7 commits August 8, 2024 10:49

Update zombienet.toml

339455e

Merge branch 'master' into rzadp-patch-1

d30a6f3

update config and readme

a6c0b86

mdlint

263d088

add zombienet-sdk test to parachain-template

030c9a2

new line

5d74d5c

fmt

fe01455

pepoviola added R0-silent Changes should not be mentioned in any release notes T10-tests This PR/Issue is related to tests. labels Aug 13, 2024

pepoviola requested review from shawntabrizi and rzadp August 13, 2024 11:03

pepoviola requested a review from a team as a code owner August 13, 2024 11:03

pepoviola requested a review from kianenigma August 13, 2024 11:07

pepoviola added 3 commits August 13, 2024 08:16

include test in pipeline

4eebd13

include test in pipeline

654c763

typo in yml

a99310c

kianenigma approved these changes Aug 13, 2024

View reviewed changes

alvicsam approved these changes Aug 13, 2024

View reviewed changes

paritytech-review-bot bot requested a review from a team August 13, 2024 14:44

bkchr reviewed Aug 13, 2024

View reviewed changes

shawntabrizi reviewed Aug 14, 2024

View reviewed changes

rzadp and others added 4 commits August 14, 2024 19:06

Apply suggestions from code review

f8cf477

Co-authored-by: Bastian Köcher <git@kchr.de>

restore path example

403e4f4

finish readme

7be12e5

reorg tests and add feature

53bf1ab

pepoviola changed the title ~~add zombienet-sdk test to parachain-template~~ [DNM] add zombienet-sdk test to parachain-template Aug 14, 2024

rzadp and others added 2 commits August 15, 2024 17:34

instructions to install the node

2e3eb48

Merge branch 'master' into rzadp-patch-1

1dbddd9

kianenigma mentioned this pull request Aug 15, 2024

Change the chain to Rococo in the parachain template Zombienet config #5279

Merged

kianenigma reviewed Aug 15, 2024

View reviewed changes

templates/zombienet/src/lib.rs Outdated Show resolved Hide resolved

kianenigma reviewed Aug 15, 2024

View reviewed changes

rzadp and others added 7 commits August 16, 2024 11:21

Merge branch 'master' into rzadp-patch-1

406eb8f

Update templates/zombienet/src/lib.rs

3ac7360

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

clean code and update CI

6794739

merge base

37a1ac5

fix ci

446c1a2

fix CI

7023463

clippy

330a34c

pepoviola changed the title ~~[DNM] add zombienet-sdk test to parachain-template~~ add zombienet-sdk test to parachain-template Aug 20, 2024

Base automatically changed from rzadp-patch-1 to master August 23, 2024 11:26

kianenigma reviewed Aug 23, 2024

View reviewed changes

templates/parachain/zombienet.toml Outdated Show resolved Hide resolved

kianenigma reviewed Aug 23, 2024

View reviewed changes

kianenigma requested changes Aug 23, 2024

View reviewed changes

kianenigma approved these changes Sep 13, 2024

View reviewed changes

pepoviola added 2 commits September 16, 2024 20:03

merge master

b31910c

update lock file

df8f934

pepoviola enabled auto-merge September 16, 2024 18:27

pepoviola added this pull request to the merge queue Sep 16, 2024

Merged via the queue into master with commit 9307d99 Sep 16, 2024
206 of 207 checks passed

pepoviola deleted the rzadp-patch-1-1 branch September 16, 2024 19:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add zombienet-sdk test to parachain-template #5342

add zombienet-sdk test to parachain-template #5342

pepoviola commented Aug 13, 2024

rzadp commented Aug 13, 2024

rzadp commented Aug 13, 2024

kianenigma left a comment

kianenigma commented Aug 13, 2024

pepoviola commented Aug 13, 2024 •

edited

Loading

bkchr Aug 13, 2024

pepoviola Aug 13, 2024

shawntabrizi Aug 14, 2024 •

edited

Loading

shawntabrizi Aug 14, 2024

shawntabrizi Aug 14, 2024

pepoviola Aug 14, 2024

kianenigma Aug 15, 2024

pepoviola Aug 16, 2024

kianenigma Aug 23, 2024

paritytech-cicd-pr commented Aug 20, 2024

pepoviola commented Aug 21, 2024

kianenigma Aug 23, 2024

pepoviola Aug 24, 2024 •

edited

Loading

kianenigma Aug 23, 2024

kianenigma left a comment

pepoviola commented Sep 12, 2024

		//! `cargo build --release -features fast-runtime --bin polkadot --bin polkadot-execute-worker --bin
		//! polkadot-prepare-worker`

add zombienet-sdk test to parachain-template #5342

add zombienet-sdk test to parachain-template #5342

Conversation

pepoviola commented Aug 13, 2024

rzadp commented Aug 13, 2024

rzadp commented Aug 13, 2024

kianenigma left a comment

Choose a reason for hiding this comment

kianenigma commented Aug 13, 2024

pepoviola commented Aug 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shawntabrizi Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paritytech-cicd-pr commented Aug 20, 2024

pepoviola commented Aug 21, 2024

Choose a reason for hiding this comment

pepoviola Aug 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kianenigma left a comment

Choose a reason for hiding this comment

pepoviola commented Sep 12, 2024

pepoviola commented Aug 13, 2024 •

edited

Loading

shawntabrizi Aug 14, 2024 •

edited

Loading

pepoviola Aug 24, 2024 •

edited

Loading