Add fast multidimensional interop tests #97
Conversation
for multidim-interop with redis
multidim-interop/go/v0.22/main.go
Outdated
| transport = os.Getenv("transport") | ||
| secureChannel = os.Getenv("security") | ||
| muxer = os.Getenv("muxer") | ||
| is_dialer_str = os.Getenv("is_dialer") |
There was a problem hiding this comment.
Nit: No underscores in Go variable names.
|
Added JS support for this branch in https://github.com/libp2p/test-plans/pull/98/files. Ready for review @MarcoPolo . |
There was a problem hiding this comment.
Summary: I suggest we continue without Testground, that is continue on this pull request.
What does Testground offer that WO-Testground doesn't?
- Testground provides network traffic shaping out of the box, but we may be able to achieve the same using Docker and the Compose specification. Additionally, our current Ping tests only ensure that the Ping still makes it across the wire, without thoroughly testing the impact of network shaping.
- Testground offers orchestration across multiple machines via Kubernetes, but the Compose specification may be able to run a single Compose YAML across multiple machines as well.
Highlighting my main arguments for WO-Testground
- It is significantly easier to use, as described in the main Pull Request description.
- It is well-specified and supported, as it builds on top of Compose.
Miscellaneous
- As proposed by @thomaseizinger, I would like the testing logic to be located in the same repository as the code it is testing in the long term. For example, the testplan_0500.rs code should be in the github.com/libp2p/rust-libp2p repository on the v0.50 branch. I don't see any problems with either Testground or WO-Testground in this regard.
Wish for next steps
- I would like to minimize the timeframe in which we use both Testground and WO-Testground. In other words, I would prefer a quick transition from Testground to WO-Testground. I expect maintaining both over a longer period of time to be a high maintenance burden. If I understand WO-Testground correctly, it already includes all the features that we currently use in Testground (excluding network shaping, which I am fine with dropping). See also the discussion on the multidimensional Testground pull request at feat: libp2p implementation interoperability dashboard on various dimensions. Uses testplans-dsl. #85 (comment).
- Proceeding in multiple pull requests is fine by me. Great to see a JS pull request already open add working ping test (js) or multidim-interop #98.
- @MarcoPolo just a wish. You know best how to proceed.
@MarcoPolo let me know how I can be helpful here. If I understand correctly @jxs will help with the Rust side for now? Is that fine @jxs and @MarcoPolo?
Thanks @MarcoPolo for pushing this forward!
There was a problem hiding this comment.
My preference would be for this pull request to atomically replace all other tests, e.g. the existing Ping tests. That said, this is not a strong preference. I suggest to go with whatever strategy @MarcoPolo proposes.
I am fine proceeding here, i.e. merging here and doing the remaining work (e.g. porting all rust-libp2p versions, updating the CI scripts) in a follow-up pull request. (As discussed, please remove v0.49.0 before merging as it is still using Testground.)
Do I understand correctly that we have consensus to proceed without Testground? @MarcoPolo in case you merge here today, have you talked to everyone involved to give folks a chance to include their arguments in this discussion?
Co-authored-by: João Oliveira <hello@jxs.pt>
Co-authored-by: João Oliveira <hello@jxs.pt>
Co-authored-by: João Oliveira <hello@jxs.pt>
Yes. I think most of the libp2p team is aligned here. I don't know of anyone who is against this. I've talked with Laurent and he agrees this makes sense for this test. @p-shahi suggested to make a small writeup to summarize the reasoning here for historical purposes. I'm happy to do that in a standalone issue after merging this PR. |
* add working ping test (js) or multidim-interop * Add JS-libp2p to interop tests * ping libp2p-js (wo): resolve PR comments * Add yamux js-libp2p test Co-authored-by: Marco Munizaga <git@marcopolo.io>
This is an alternative version to #85 . This uses https://compose-spec.io to define the tests and
docker composeto run the test.On my machine this is 10x faster than testground. On CI, this is about 3.5x faster (I've done no optimizations on either local or CI, but we could probably parallelize the tests more if the CI instances were beefier).
The basic idea here:
versions.tsdocker composeto run this.The entry point for everything is
testplans.tsand you can run this locally by doingnpm run test(assuming you have docker up).This is not a replacement of testground. Instead this is using common tools to run these interop tests faster. I make no claim here that this replaces testground. That being said, I found working completely outside of testground a nicer and faster experience. Compose is a well trodden tool with lots of documentation available. I found
compose.yamlspec good and simple enough for what I need here.Compared to #85
compose.yamland inspect it or manually copy it and tweak it. No custom sync service, just a plain redis store and redis client for synchronization.Don't let the 13k diff scare you. Most of it is lock files
go.sumorpackage-lock.json. The important bits to review here are the Typescript files inmultidim-interopand the Go implementation of ping.