From adf9985ec9062fcd518660493b00afc904e7927c Mon Sep 17 00:00:00 2001 From: achingbrain Date: Fri, 21 Jan 2022 07:08:29 +0000 Subject: [PATCH 1/3] chore: fix flaky tests These tests are flaky in CI, probably due to differences in timing introduced by #1058 Fixes #1134 --- test/relay/auto-relay.node.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/relay/auto-relay.node.js b/test/relay/auto-relay.node.js index 5ec2643e06..ddb31c535f 100644 --- a/test/relay/auto-relay.node.js +++ b/test/relay/auto-relay.node.js @@ -370,9 +370,9 @@ describe('auto-relay', () => { // Wait for other peer connected to be added as listen addr await pWaitFor(() => relayLibp2p1.transportManager.listen.callCount === 2) - expect(autoRelay1._tryToListenOnRelay.callCount).to.equal(1) - expect(autoRelay1._listenRelays.size).to.equal(1) - expect(relayLibp2p1.connectionManager.size).to.eql(1) + await pWaitFor(() => autoRelay1._tryToListenOnRelay.callCount === 1) + await pWaitFor(() => autoRelay1._listenRelays.size === 1) + await pWaitFor(() => relayLibp2p1.connectionManager.size === 1) }) it('should not fail when trying to dial unreachable peers to add as hop relay and replaced removed ones', async () => { @@ -492,7 +492,7 @@ describe('auto-relay', () => { // Wait for peer added as listen relay await pWaitFor(() => autoRelay2._addListenRelay.callCount === 1) - expect(autoRelay2._listenRelays.size).to.equal(1) + await pWaitFor(() => autoRelay2._listenRelays.size === 1) // Relay 1 discovers Relay 2 relayed multiaddr via Relay 3 const ma2RelayedBy3 = relayLibp2p2.multiaddrs[relayLibp2p2.multiaddrs.length - 1] From 13b2d05e8203dd1cafe763e5bb180b26748c8f21 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Fri, 21 Jan 2022 07:45:00 +0000 Subject: [PATCH 2/3] chore: await async things --- test/connection-manager/index.node.js | 14 +++++++------- test/core/ping.node.js | 4 ++-- test/dialing/direct.node.js | 8 ++++---- test/dialing/direct.spec.js | 4 ++-- test/identify/index.spec.js | 6 +++--- test/peer-discovery/index.node.js | 4 ++-- test/peer-discovery/index.spec.js | 2 +- test/relay/auto-relay.node.js | 12 ++++++------ test/relay/relay.node.js | 2 +- test/utils/creators/peer.js | 6 +++--- 10 files changed, 31 insertions(+), 31 deletions(-) diff --git a/test/connection-manager/index.node.js b/test/connection-manager/index.node.js index bcd381b23c..d187895c6a 100644 --- a/test/connection-manager/index.node.js +++ b/test/connection-manager/index.node.js @@ -118,7 +118,7 @@ describe('libp2p.connections', () => { } }) - libp2p.peerStore.addressBook.set(remoteLibp2p.peerId, remoteLibp2p.multiaddrs) + await libp2p.peerStore.addressBook.set(remoteLibp2p.peerId, remoteLibp2p.multiaddrs) await libp2p.dial(remoteLibp2p.peerId) expect(libp2p.connections.size).to.eql(1) @@ -161,8 +161,8 @@ describe('libp2p.connections', () => { }) // Populate PeerStore before starting - libp2p.peerStore.addressBook.set(nodes[0].peerId, nodes[0].multiaddrs) - libp2p.peerStore.addressBook.set(nodes[1].peerId, nodes[1].multiaddrs) + await libp2p.peerStore.addressBook.set(nodes[0].peerId, nodes[0].multiaddrs) + await libp2p.peerStore.addressBook.set(nodes[1].peerId, nodes[1].multiaddrs) await libp2p.start() @@ -188,8 +188,8 @@ describe('libp2p.connections', () => { }) // Populate PeerStore before starting - libp2p.peerStore.addressBook.set(nodes[0].peerId, nodes[0].multiaddrs) - libp2p.peerStore.addressBook.set(nodes[1].peerId, nodes[1].multiaddrs) + await libp2p.peerStore.addressBook.set(nodes[0].peerId, nodes[0].multiaddrs) + await libp2p.peerStore.addressBook.set(nodes[1].peerId, nodes[1].multiaddrs) await libp2p.start() @@ -253,7 +253,7 @@ describe('libp2p.connections', () => { }) // Populate PeerStore after starting (discovery) - libp2p.peerStore.addressBook.set(nodes[0].peerId, nodes[0].multiaddrs) + await libp2p.peerStore.addressBook.set(nodes[0].peerId, nodes[0].multiaddrs) // Wait for peer to connect const conn = await libp2p.dial(nodes[0].peerId) @@ -290,7 +290,7 @@ describe('libp2p.connections', () => { } }) - libp2p.peerStore.addressBook.set(remoteLibp2p.peerId, remoteLibp2p.multiaddrs) + await libp2p.peerStore.addressBook.set(remoteLibp2p.peerId, remoteLibp2p.multiaddrs) await libp2p.dial(remoteLibp2p.peerId) const totalConns = Array.from(libp2p.connections.values()) diff --git a/test/core/ping.node.js b/test/core/ping.node.js index 4bbd4f76cc..63ebec0160 100644 --- a/test/core/ping.node.js +++ b/test/core/ping.node.js @@ -19,8 +19,8 @@ describe('ping', () => { config: baseOptions }) - nodes[0].peerStore.addressBook.set(nodes[1].peerId, nodes[1].multiaddrs) - nodes[1].peerStore.addressBook.set(nodes[0].peerId, nodes[0].multiaddrs) + await nodes[0].peerStore.addressBook.set(nodes[1].peerId, nodes[1].multiaddrs) + await nodes[1].peerStore.addressBook.set(nodes[0].peerId, nodes[0].multiaddrs) }) afterEach(() => Promise.all(nodes.map(n => n.stop()))) diff --git a/test/dialing/direct.node.js b/test/dialing/direct.node.js index 0d88a397f0..b321fc2da7 100644 --- a/test/dialing/direct.node.js +++ b/test/dialing/direct.node.js @@ -128,7 +128,7 @@ describe('Dialing (direct, TCP)', () => { peerStore }) - peerStore.addressBook.set(peerId, remoteTM.getAddrs()) + await peerStore.addressBook.set(peerId, remoteTM.getAddrs()) const connection = await dialer.connectToPeer(peerId) expect(connection).to.exist() @@ -326,7 +326,7 @@ describe('Dialing (direct, TCP)', () => { }) sinon.spy(libp2p.dialer, 'connectToPeer') - libp2p.peerStore.addressBook.set(remotePeerId, remoteLibp2p.multiaddrs) + await libp2p.peerStore.addressBook.set(remotePeerId, remoteLibp2p.multiaddrs) const connection = await libp2p.dial(remotePeerId) expect(connection).to.exist() @@ -471,7 +471,7 @@ describe('Dialing (direct, TCP)', () => { const fullAddress = remoteAddr.encapsulate(`/p2p/${remoteLibp2p.peerId.toB58String()}`) - libp2p.peerStore.addressBook.set(remotePeerId, remoteLibp2p.multiaddrs) + await libp2p.peerStore.addressBook.set(remotePeerId, remoteLibp2p.multiaddrs) const dialResults = await Promise.all([...new Array(dials)].map((_, index) => { if (index % 2 === 0) return libp2p.dial(remoteLibp2p.peerId) return libp2p.dial(fullAddress) @@ -501,7 +501,7 @@ describe('Dialing (direct, TCP)', () => { const error = new Error('Boom') sinon.stub(libp2p.transportManager, 'dial').callsFake(() => Promise.reject(error)) - libp2p.peerStore.addressBook.set(remotePeerId, remoteLibp2p.multiaddrs) + await libp2p.peerStore.addressBook.set(remotePeerId, remoteLibp2p.multiaddrs) const dialResults = await pSettle([...new Array(dials)].map((_, index) => { if (index % 2 === 0) return libp2p.dial(remoteLibp2p.peerId) return libp2p.dial(remoteAddr) diff --git a/test/dialing/direct.spec.js b/test/dialing/direct.spec.js index 14beacb4f1..ba97352668 100644 --- a/test/dialing/direct.spec.js +++ b/test/dialing/direct.spec.js @@ -48,8 +48,8 @@ describe('Dialing (direct, WebSockets)', () => { localTM.add(Transport.prototype[Symbol.toStringTag], Transport, { filter: filters.all }) }) - afterEach(() => { - peerStore.delete(peerId) + afterEach(async () => { + await peerStore.delete(peerId) sinon.restore() }) diff --git a/test/identify/index.spec.js b/test/identify/index.spec.js index 88a61eeeda..7e960dfe1b 100644 --- a/test/identify/index.spec.js +++ b/test/identify/index.spec.js @@ -41,13 +41,13 @@ describe('Identify', () => { peerId: localPeer, datastore: new MemoryDatastore() }) - localPeerStore.protoBook.set(localPeer, protocols) + await localPeerStore.protoBook.set(localPeer, protocols) remotePeerStore = new PeerStore({ peerId: remotePeer, datastore: new MemoryDatastore() }) - remotePeerStore.protoBook.set(remotePeer, protocols) + await remotePeerStore.protoBook.set(remotePeer, protocols) localAddressManager = new AddressManager(localPeer) remoteAddressManager = new AddressManager(remotePeer) @@ -372,7 +372,7 @@ describe('Identify', () => { peerId: remotePeer, datastore: new MemoryDatastore() }) - remotePeerStore.protoBook.set(remotePeer, storedProtocols) + await remotePeerStore.protoBook.set(remotePeer, storedProtocols) const remoteIdentify = new IdentifyService({ libp2p: { diff --git a/test/peer-discovery/index.node.js b/test/peer-discovery/index.node.js index bdf4587960..cc5618bb4e 100644 --- a/test/peer-discovery/index.node.js +++ b/test/peer-discovery/index.node.js @@ -186,8 +186,8 @@ describe('peer discovery scenarios', () => { remoteLibp2p2.start() ]) - libp2p.peerStore.addressBook.set(remotePeerId1, remoteLibp2p1.multiaddrs) - remoteLibp2p2.peerStore.addressBook.set(remotePeerId1, remoteLibp2p1.multiaddrs) + await libp2p.peerStore.addressBook.set(remotePeerId1, remoteLibp2p1.multiaddrs) + await remoteLibp2p2.peerStore.addressBook.set(remotePeerId1, remoteLibp2p1.multiaddrs) // Topology: // A -> B diff --git a/test/peer-discovery/index.spec.js b/test/peer-discovery/index.spec.js index 72248621b0..1a43d9f7e8 100644 --- a/test/peer-discovery/index.spec.js +++ b/test/peer-discovery/index.spec.js @@ -38,7 +38,7 @@ describe('peer discovery', () => { } }) - libp2p.peerStore.addressBook.set(remotePeerId, [new Multiaddr('/ip4/165.1.1.1/tcp/80')]) + await libp2p.peerStore.addressBook.set(remotePeerId, [new Multiaddr('/ip4/165.1.1.1/tcp/80')]) const deferred = defer() sinon.stub(libp2p.dialer, 'connectToPeer').callsFake((remotePeerId) => { diff --git a/test/relay/auto-relay.node.js b/test/relay/auto-relay.node.js index ddb31c535f..d2028cc4b2 100644 --- a/test/relay/auto-relay.node.js +++ b/test/relay/auto-relay.node.js @@ -310,7 +310,7 @@ describe('auto-relay', () => { await pWaitFor(() => relayLibp2p1.multiaddrs.length === originalMultiaddrs1Length + 1) expect(relayLibp2p1.multiaddrs[originalMultiaddrs1Length].getPeerId()).to.eql(relayLibp2p2.peerId.toB58String()) - // Only one will be used for listeninng + // Only one will be used for listening expect(relayLibp2p1.transportManager.listen.callCount).to.equal(1) // Spy if relay from listen map was removed @@ -349,7 +349,7 @@ describe('auto-relay', () => { expect(autoRelay1._listenRelays.size).to.equal(1) expect(relayLibp2p1.connectionManager.size).to.equal(2) - // Only one will be used for listeninng + // Only one will be used for listening expect(relayLibp2p1.transportManager.listen.callCount).to.equal(1) // Disconnect not used listen relay @@ -363,7 +363,7 @@ describe('auto-relay', () => { sinon.spy(relayLibp2p1, 'dial') // Remove peer used as relay from peerStore and disconnect it - relayLibp2p1.peerStore.delete(relayLibp2p2.peerId) + await relayLibp2p1.peerStore.delete(relayLibp2p2.peerId) await relayLibp2p1.hangUp(relayLibp2p2.peerId) expect(autoRelay1._listenRelays.size).to.equal(0) expect(relayLibp2p1.connectionManager.size).to.equal(0) @@ -394,7 +394,7 @@ describe('auto-relay', () => { expect(autoRelay1._listenRelays.size).to.equal(1) expect(relayLibp2p1.connectionManager.size).to.equal(2) - // Only one will be used for listeninng + // Only one will be used for listening expect(relayLibp2p1.transportManager.listen.callCount).to.equal(1) // Disconnect not used listen relay @@ -410,7 +410,7 @@ describe('auto-relay', () => { }) // Remove peer used as relay from peerStore and disconnect it - relayLibp2p1.peerStore.delete(relayLibp2p2.peerId) + await relayLibp2p1.peerStore.delete(relayLibp2p2.peerId) await relayLibp2p1.hangUp(relayLibp2p2.peerId) expect(autoRelay1._listenRelays.size).to.equal(0) expect(relayLibp2p1.connectionManager.size).to.equal(0) @@ -619,7 +619,7 @@ describe('auto-relay', () => { await pWaitFor(() => local.multiaddrs.length === originalMultiaddrsLength + 1) const relayedAddr = local.multiaddrs[local.multiaddrs.length - 1] - remote.peerStore.addressBook.set(local.peerId, [relayedAddr]) + await remote.peerStore.addressBook.set(local.peerId, [relayedAddr]) // Dial from remote through the relayed address const conn = await remote.dial(local.peerId) diff --git a/test/relay/relay.node.js b/test/relay/relay.node.js index 803a3849ae..4510b8629d 100644 --- a/test/relay/relay.node.js +++ b/test/relay/relay.node.js @@ -51,7 +51,7 @@ describe('Dialing (via relay, TCP)', () => { await libp2p.stop() // Clear the peer stores for await (const peer of libp2p.peerStore.getPeers()) { - libp2p.peerStore.delete(peer.id) + await libp2p.peerStore.delete(peer.id) } })) }) diff --git a/test/utils/creators/peer.js b/test/utils/creators/peer.js index ef93d237dd..63b943f76b 100644 --- a/test/utils/creators/peer.js +++ b/test/utils/creators/peer.js @@ -36,17 +36,17 @@ async function createPeer ({ number = 1, fixture = true, started = true, populat if (started) { await Promise.all(peers.map((p) => p.start())) - populateAddressBooks && _populateAddressBooks(peers) + populateAddressBooks && await _populateAddressBooks(peers) } return peers } -function _populateAddressBooks (peers) { +async function _populateAddressBooks (peers) { for (let i = 0; i < peers.length; i++) { for (let j = 0; j < peers.length; j++) { if (i !== j) { - peers[i].peerStore.addressBook.set(peers[j].peerId, peers[j].multiaddrs) + await peers[i].peerStore.addressBook.set(peers[j].peerId, peers[j].multiaddrs) } } } From 22822a4d989d8f1a1295677dfb66bee42612aead Mon Sep 17 00:00:00 2001 From: achingbrain Date: Fri, 21 Jan 2022 07:55:46 +0000 Subject: [PATCH 3/3] chore: no need to delete all the peers, they are stored in memory --- test/relay/relay.node.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/test/relay/relay.node.js b/test/relay/relay.node.js index 4510b8629d..e9c1207875 100644 --- a/test/relay/relay.node.js +++ b/test/relay/relay.node.js @@ -47,13 +47,7 @@ describe('Dialing (via relay, TCP)', () => { afterEach(async () => { // Stop each node - return Promise.all([srcLibp2p, relayLibp2p, dstLibp2p].map(async libp2p => { - await libp2p.stop() - // Clear the peer stores - for await (const peer of libp2p.peerStore.getPeers()) { - await libp2p.peerStore.delete(peer.id) - } - })) + return Promise.all([srcLibp2p, relayLibp2p, dstLibp2p].map(libp2p => libp2p.stop())) }) it('should be able to connect to a peer over a relay with active connections', async () => {