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

pss: Port tests to network/simulation #1682

Merged
merged 3 commits into from
Aug 30, 2019

Conversation

aperonam
Copy link
Contributor

Because handshake_test.go and protocol_test.go both use a function from pss_test.go that has been modified, both files have some minor changes

There is still a dependency with p2p/simulations/adapters in pss_test.go through the use of the type adapters.serviceContext , this is only used as the type of a function parameter, maybe we could extract this type into network/simulations and refactor its uses

Also, there are more dependencies with p2p/simulations in pss/client and pss/notify, this could be refactored in a future PR

References issue #1652

…hereum's p2p/simulations. protocol_test and handshake_test modified to work with change in setupNetwork function
return adapters.Services{
protocolName: func(ctx *adapters.ServiceContext) (node.Service, error) {
return map[string]simulation.ServiceFunc{
"bzz": func(ctx *adapters.ServiceContext, bucket *sync.Map) (s node.Service, cleanup func(), err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't move blocks of code when making changes in the future. It's easier to compare if you leave them where they are, then we can always reorganize the blocks separately.

pss/pss_test.go Show resolved Hide resolved
pss/pss_test.go Show resolved Hide resolved
pss/pss_test.go Outdated Show resolved Hide resolved
@@ -43,9 +44,11 @@ func testHandshake(t *testing.T) {
addrsizestring := strings.Split(t.Name(), "/")
addrsize, _ = strconv.ParseInt(addrsizestring[1], 10, 0)

sim := simulation.NewInProc(newServices(false))
defer sim.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

The simulation constructor seems to belong inside the setupNetwork

@aperonam
Copy link
Contributor Author

aperonam commented Aug 23, 2019

Pushed commit f607f62 to fix PR review.
Simulation is created again inside setupNetwork.
AddNodesAndconnectRing returns error if 2 nodes, added check in setupNetwork to decide if connectRing or connectChain

@aperonam aperonam requested a review from nolash August 26, 2019 06:51
@nolash nolash requested a review from janos August 26, 2019 07:48
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.

@nolash nolash merged commit e712b70 into ethersphere:master Aug 30, 2019
@skylenet skylenet added this to the 0.5.0 milestone Sep 17, 2019
chadsr added a commit to chadsr/swarm that referenced this pull request Sep 23, 2019
* 'master' of github.com:ethersphere/swarm:
  pss: Modularize crypto and remove Whisper. Step 1 - isolate whisper code (ethersphere#1698)
  pss: Improve pressure backstop queue handling - no mutex (ethersphere#1695)
  cmd/swarm-snapshot: if 2 nodes to create snapshot use connectChain (ethersphere#1709)
  network: Add API for Capabilities (ethersphere#1675)
  pss: fixed flaky test that was using a global variable instead of a local one (ethersphere#1702)
  pss: Port tests to `network/simulation` (ethersphere#1682)
  storage: fix hasherstore seen check to happen when error is nil (ethersphere#1700)
  vendor: upgrade go-ethereum to 1.9.2 (ethersphere#1689)
  bzzeth: initial support for bzz-eth protocol (ethersphere#1571)
  network/stream: terminate runUpdateSyncing on peer quit (ethersphere#1696)
  all: first working SWAP version (ethersphere#1554)
  version: update to v0.5.0 unstable (ethersphere#1694)
  chunk, storage: storage with multi chunk Set method (ethersphere#1684)
  chunk, storage: add HasMulti to chunk.Store (ethersphere#1686)
  chunk, shed, storage: chunk.Store GetMulti method (ethersphere#1691)
  api, chunk: progress bar support (ethersphere#1649)
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.

4 participants