v1 tests

[davidmarkclements]

• tests refactored to same approach as used in guts testing

• 100% coverage

• at some point it will make sense to split test utils out (some code with guts is duped), but would like to do that later on

• maxPeers has been replaced with maxClientSockets and maxServerSockets (maxServerSockets defaults to infinity for parity with server.maxConnections, maxClientSockets default is 16)

• various small bug fixes

• side note: @hyperswarm/discovery readme seems to suggest that passing lookup: false option to discovery.announce method will prevent the topic stream from emitting peers. This is not the case, whether lookup is true/false it will always emit peers - (I don't think dht-rpc has the functionality to control this)

• another side note: clarification needed here, if an announcing-only peer joins a topic after a lookup-only peer has joined that topic, should a connection be established between the peers?Currently a connection isn't established in this scenario.

[mafintosh]

@davidmarkclements anyway to still support a general max sockets? in most cases you don't care much if you're acting as a server or client (having the individual is cool too).

re, the lookup: false still emitting peers. that's a typo/misunderstanding. any dht operation will emit peers.

on your another side note: no a connection should not be established in this case, until the lookup peer does another lookup op against the dht after the announce

[davidmarkclements]

@davidmarkclements anyway to still support a general max sockets? in most cases you don't care much if you're acting as a server or client (having the individual is cool too).

so the reason I split them was that it became apparent that treating them the same could become a footgun. I need to regather my thoughts, it might make sense to have a face to face convo about this one

re, the lookup: false still emitting peers. that's a typo/misunderstanding. any dht operation will emit peers.

ok.. I think that means that the implicit ergonomics of this can lead to accidental double operations (again may be better to explain at higher res)

on your another side note: no a connection should not be established in this case, until the lookup peer does another lookup op against the dht after the announce

at a high level this feels unintuitive - although it completely makes sense in a server/client paradigm (this is a similar tension to the separating server/client connections discussion)

[mafintosh]

@davidmarkclements if you disable lookup and only do announce you are basically opting in to behaving as a server.

happy to discuss the maxServer/Client stuff face to face. After reading the impl I'd worry a bit that support both maxPeers and this might be complicated, and maxPeers is more important than maxServer here.

The way I initially considered implementing it was kinda like utpServer.maxConnections = tcpServer.maxConnections = Infinity and then setting both of those to 0 if connections.length >= maxPeers. If a connection drops reset them to Infinity.

[davidmarkclements]

this PR is dependent on guts PR: https://github.com/hyperswarm/guts/pull/9

[davidmarkclements]

re peer removal #26 - which is based on this branch

[mafintosh]

@davidmarkclements guts 1.0.0 is out here with your PR, you wanna bump it here?

[mafintosh]

@davidmarkclements i'm still not sure i follow the maxPeers logic. when reading the code it seems to me like the client connections will max out at 16 (default) although maxPeers is set to 24?

Maybe let's talk it through on our call

[davidmarkclements]

discussion result was to set max clients default to infinity - done in https://github.com/hyperswarm/network/pull/25/commits/ed858f191688f86e0f24e85f3f0dc3410cc0d610