Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

fix: swarm addrs test #454

Merged
merged 2 commits into from
Apr 4, 2019
Merged

fix: swarm addrs test #454

merged 2 commits into from
Apr 4, 2019

Conversation

alanshaw
Copy link
Contributor

@alanshaw alanshaw commented Apr 4, 2019

If the common factory creates a node with no bootstrap nodes then getting a list of swarm addresses could be empty (or could also be empty anyway if the connection to the bootstrap nodes takes a long time). This PR spawns and connects another IPFS node to ensure that there should be swarm addresses available.

If the common factory creates a node with no bootstrap nodes then getting a list of swarm addresses could be empty (or could also be empty anyway if the connection to the bootstrap nodes takes a long time). This PR spawns and connects another IPFS node to ensure that there should be swarm addresses available.

License: MIT
Signed-off-by: Alan Shaw <[email protected]>
@ghost ghost assigned alanshaw Apr 4, 2019
@ghost ghost added the in progress label Apr 4, 2019
Copy link
Contributor

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

Looks good! Just pointed out a minor detail!

})
})
})

after((done) => common.teardown(done))

it('should get a list of node addresses', (done) => {
ipfs.swarm.addrs((err, multiaddrs) => {
ipfsA.swarm.addrs((err, multiaddrs) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we change multiaddrs to peers or peersInfo?

I was not understanding the forEach validation, but after getting into the code, I noticed that in fact ipfs.swarm.addrs() returns an array of PeerInfo. This way, calling this multiaddrs seems confusing. I think we should also update in the SPEC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed to peerInfos as there's already precedent for that name in swarm.peers docs.

Copy link
Contributor

@jacobheun jacobheun left a comment

Choose a reason for hiding this comment

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

Looks good!

@alanshaw alanshaw merged commit 16ad830 into master Apr 4, 2019
@alanshaw alanshaw deleted the fix/swarm-addrs-test branch April 4, 2019 10:49
@ghost ghost removed the in progress label Apr 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants