Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

p2p/simulation: move connection methods from swarm/network/simulation #1057

Closed
wants to merge 7 commits into from

Conversation

frncmx
Copy link
Contributor

@frncmx frncmx commented Dec 13, 2018

This refactor was done as the second part of our current network
testing efforts; as outlined in #1038.

Co-authored-by: Elad Nachmias theman@elad.im


This PR is part two of four PRs that adds test and benchmark to ensure that the snapshot connection content is correct upon creation, and that the correct connections are made after snapshot is loaded. All actual connections should be contained in snapshot, and all connections and only those connections should be in the network after load.

  1. Part one creates the correctness test and adds a vanilla benchmark function.

  2. Part two will move connection methods from swarm/network/simulation to p2p/simulations

  3. Part three will add a snapshot generation binary leveraging the different configurations provided by part two.

  4. Part four will add exhaustive benchmarks with different snapshots generated from part three.


This refactor was done as the second part of our current network
testing efforts; as outlined in #1038.

Co-authored-by: Elad Nachmias <theman@elad.im>
Copy link
Member

@janos janos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I have only one small suggestion.

Copy link
Member

@janos janos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ups, I missed a few problems.

swarm/network/stream/visualized_snapshot_sync_sim_test.go Outdated Show resolved Hide resolved
swarm/network/stream/snapshot_sync_test.go Outdated Show resolved Hide resolved
swarm/network/stream/snapshot_sync_test.go Outdated Show resolved Hide resolved
Copy link
Member

@janos janos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Contributor

@nolash nolash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't do a comparison of the NoopService, as I assumed it's unchanged from the version introduced in #1038

p2p/simulations/connect.go Outdated Show resolved Hide resolved
p2p/simulations/connect.go Show resolved Hide resolved
p2p/simulations/connect.go Outdated Show resolved Hide resolved
p2p/simulations/connect_test.go Show resolved Hide resolved
p2p/simulations/connect_test.go Show resolved Hide resolved
p2p/simulations/connect_test.go Outdated Show resolved Hide resolved
p2p/simulations/connect_test.go Show resolved Hide resolved
p2p/simulations/connect_test.go Show resolved Hide resolved
p2p/simulations/test.go Outdated Show resolved Hide resolved
swarm/network/simulation/node_test.go Show resolved Hide resolved
As they are only used in the same package.
Ferenc Szabo added 3 commits December 14, 2018 14:18
Functions on Network struct return *Node and not enode.ID in general.
Let's keep the interface consistent.
@frncmx
Copy link
Contributor Author

frncmx commented Dec 17, 2018

as merged upstream

@frncmx frncmx closed this Dec 17, 2018
@frncmx frncmx deleted the sim-move-connection-methods branch December 17, 2018 11:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants