-
Notifications
You must be signed in to change notification settings - Fork 1.2k
integration: add circuit v2 transport integration test #3392
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
base: master
Are you sure you want to change the base?
integration: add circuit v2 transport integration test #3392
Conversation
Did you use an LLM to help generate this PR? We kindly ask that you must disclose that:
The goal is to add circuitv2 to the existing transport integration tests, not to create a new one. See https://github.com/libp2p/go-libp2p/pull/3162/files for some initial work. |
No there is not any LLM used here just use it to understand the thing how the curcuitv2 works. Yes, I will mention in pr if any AI is used. I understand what you mean for the issue I think I misunderstood this issue. |
Thank you! |
@MarcoPolo Please have a look. |
p2p/test/transport/transport_test.go
Outdated
finalLibp2pOpts := transformOpts(opts) | ||
finalLibp2pOpts = append(finalLibp2pOpts, | ||
libp2p.Identity(listenerHost.Peerstore().PrivKey(listenerHost.ID())), | ||
libp2p.EnableRelay(), | ||
libp2p.AddrsFactory(addrFactory), | ||
libp2p.ListenAddrStrings("/ip4/127.0.0.1/udp/0/quic-v1"), | ||
) | ||
|
||
finalHost, err := libp2p.New(finalLibp2pOpts...) | ||
require.NoError(t, err) | ||
|
||
err = finalHost.Connect(ctx, peer.AddrInfo{ | ||
ID: relayHost.ID(), | ||
Addrs: relayHost.Addrs(), | ||
}) | ||
require.NoError(t, err) | ||
|
||
_, err = client.Reserve(ctx, finalHost, peer.AddrInfo{ | ||
ID: relayHost.ID(), | ||
Addrs: relayHost.Addrs(), | ||
}) | ||
require.NoError(t, err) |
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.
Why is there a 3rd host? You have the listener, relay, and "final."
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 I think i overcomplicate this
I am making some changes
Next time, please run the tests before submitting a PR.
|
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.
This is a good start, but there are failing tests. There might be an issue in the circuitV2 transport itself, but I haven't dived deeper yet.
@MarcoPolo I changed some part of code. |
The purpose of these tests is to test all aspects of a transport, not just the ability to send ping a small message. The failures are likely related to something about the circuitv2 transport. While we could skip every test that fails in circuitv2, I'd rather we attempt to fix the issues. |
For testing :
go test ./p2p/test/transport -v -run TestPing
Fixes: #3095