p2p/simulations, swarm/network: Custom services in snapshot #17991
Conversation
p2p/simulations/network_test.go
Outdated
| network.Connect(ids[0], ids[2]) | ||
| time.Sleep(time.Second) | ||
| network.Disconnect(ids[0], ids[2]) | ||
| time.Sleep(time.Second) |
There was a problem hiding this comment.
Have you tried shorter Sleeps? The time.Second are adding up....Without wanting to add more complexity, what about listening to the Connection and Disconnection events?
| rawlog = flag.Bool("rawlog", false, "remove terminal formatting from logs") | ||
| nodeCount = flag.Int("nodes", 10, "number of nodes to create (default 10)") | ||
| initCount = flag.Int("conns", 1, "number of originally connected peers (default 1)") | ||
| snapshotFile = flag.String("snapshot", "", "create snapshot") |
There was a problem hiding this comment.
isn't this better to document as "path of snapshot file created" or something like that?
…ulation connect/disconnect
acud
force-pushed
the
discovery-snapshot-customservice
branch
from
20dab68 to
e59205c
Compare
p2p/simulations/network_test.go
Outdated
| } | ||
| } | ||
|
|
||
| snap, err = network.SnapshotWithServices([]string{"bzz"}, []string{"test"}) |
There was a problem hiding this comment.
I guess it is best to put dummy2 in the services to be removed not the non-existing test and then check it it is removed on line 129
p2p/simulations/network_test.go
Outdated
| adapter := adapters.NewSimAdapter(adapters.Services{ | ||
| "dummy": newDummyService, | ||
| "dummy2": newDummy2Service, | ||
| "dummy": node.NewNoopServiceA, |
There was a problem hiding this comment.
p2p/simulations/network_test.go
Outdated
| adapter := adapters.NewSimAdapter(adapters.Services{ | ||
| "dummy": newDummyService, | ||
| "dummy2": newDummy2Service, | ||
| "dummy": node.NewNoopServiceA, |
There was a problem hiding this comment.
acud
force-pushed
the
discovery-snapshot-customservice
branch
from
020fb88 to
5ec709a
Compare
p2p/simulations/http_test.go
Outdated
| @@ -676,6 +722,9 @@ func TestHTTPSnapshot(t *testing.T) { | |||
| if conn.Other.String() != nodes[1].ID { | |||
| t.Fatalf("expected connection to have other=%q, got other=%q", nodes[1].ID, conn.Other) | |||
| } | |||
| if conn.Up == false { | |||
acud
force-pushed
the
discovery-snapshot-customservice
branch
from
e55901b to
a07d55b
Compare
There was a problem hiding this comment.
LGTM
Only one general suggestion. It would be good to signal done channels (in this case eventsDone) with closing them, rather then sending an empty struct to it, even if there is only one receiver. Adding new receivers would not require changing the sender part and code semantics would be right if someone uses it as an example or copies it.
|
thanks @janos i'll keep it in mind for future PRs 🙏 |
This PR makes it possible to selectively add and remove services from snapshot generated from simulations discovery test.
It also prunes connection objects that have status down from the generated snapshots.
Note; this uses sleeps for the
TestSnapshotExplicittest because the timing of peer events and actual changes in state in the simulated nodes seems to be imprecise (and the test in itself is very simple). Fixing this will be a separate PR.