-
Notifications
You must be signed in to change notification settings - Fork 43
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
ping/rust: introduce rust cross-version test #26
ping/rust: introduce rust cross-version test #26
Conversation
# CARGO_PATCH = """ | ||
# [patch.crates-io] | ||
# libp2pv0450 = { package = "libp2p", git = "{{ or .Env.GitTarget "https://github.com/libp2p/rust-libp2p" }}", branch = "master" } | ||
# """ |
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.
@mxinden fyi, that's how we could get master & current groups as far as I understand, but I get this for now:
Caused by:
The patch location `https://github.com/libp2p/rust-libp2p` does not appear to contain any packages matching the name `libp2p`.
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.
That is surprising to me. The below works fine in a project of mine:
+
+[patch.crates-io]
+libp2p = { package = "libp2p", git = "https://github.com/libp2p/rust-libp2p", branch = "master" }
Maybe related to the libp2pv0450
rename?
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.
Is this missing the .git
suffix, i.e.git = "https://github.com/libp2p/rust-libp2p.git
?
812d202
to
9375005
Compare
@laurentsenta just as a heads up, I am looking into cleaning up |
Pushed my latest work, mostly refactoring Running |
Update: I started working on running multiple iterations with different latencies, the correspondent Rust code to the Go code below. It works with 4 Rust instances. Still needs some clean-up. Lines 251 to 286 in eee18f0
@laurentsenta could you prepare a composition file with both a Rust and a Golang node? That would help testing interoperability of the |
I pushed the code needed to run multiple iterations of the ping test. It should now be interoperable with the Golang implementation, though thus far I have only tested it across different Rust versions. |
ping/rust/Cargo.toml
Outdated
# Note: if this version doesn't match EXACTLY the testground one, I got errors. | ||
# Is it possible we get similar issues with libp2p dependencies? |
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, that is expected behavior. In case we ever hit the same issue with libp2p we can differentiate by version via the passed in libp2p feature flag.
@mxinden I added the testground-related content, # from the test-plans folder, import the plans (once)
testground plan import --from ./ --name libp2p
# then run the simple composition
testground run composition -f ping/_compositions/go-rust-interop-latest.toml --wait I have a more complex test ready, BUT the simple one is not passing yet, This is the error I get:
I'm looking into it, one thing to note, it looks like the rust test publishes a raw string address ( |
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.
@laurentsenta thanks for merging the JSON changes and the iteration fix.
Should I test this locally or did you already get to it?
.parse() | ||
.unwrap(); | ||
|
||
for i in 1..iterations + 1 { |
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.
Good catch @laurentsenta.
- Showcase version selection through features Can be run with `cargo check --features libp2pv0450` or `cargo check --features libp2pv0440`. - Peer programming session
Potentially allows caching.
Building the binary in release mode is enough.
9d23dbc
to
139dc58
Compare
a6137e7
to
8e33cce
Compare
Marking this as ready for review, we had a call with @mxinden and decided we can merge, try rust - go interop on rust-libp2p, and improve performance. |
8e33cce
to
dcce2f5
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.
Cool. Let's go ahead here and iterate on it afterwards.
- Do once, from the test-plans root: import the test-plans with `testground plan import ./ --name libp2p` | ||
- Run the test with `testground run composition -f ping/_compositions/go-cross-versions.toml --wait` | ||
|
||
## How to add a new version to ping/rust |
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.
🚀
Runs:
go-libp2p v0.22 broke the test, I (we) need to update these. |
ping
test with a rust version.Rust ping test:
We add a
ping/rust
source folder. This test follows the workflow implemented byping/go
which means they may be combined.Cargo.toml
Dockerfile
.Testground compositions
Test outputs are shown on the testground dashboard (localhost:8042 for a default daemon that runs locally).
ping/_compositions/rust-cross-versions.toml
Goal: test different rust-libp2p versions together.
This composition tests v0.44.0, v0.45.1, v0.46.0, master, and an optional branch:
Usage:
ping/_compositions/go-rust-interop-latest.toml
Goal: test latest go and rust libp2p versions together.
This composition tests rust master, and go master and a custom branch either go or rust.
Usage:
ping/_compositions/go-rust-interop.toml
Goal: every known go and rust versions together.
This composition combines the version from both rust and go,
Usage:
CI
This action will build & run the test.
Todo: