Skip to content

Commit

Permalink
chore: fix flaky local peer device info test
Browse files Browse the repository at this point in the history
`waitForPeers()` has a `waitForDeviceInfo` option. When enabled, this
helper should resolve only after device info is populated.

It had a tricky bug that only manifested some of the time (see
[example CI failure 1][1], [2]). In some cases, the function could
_think_ device info was populated when it wasn't.[^CI]

To fix this, I reworked the helper. It now does redundant work but it's
now stateless-ish, which should avoid this problem.

[^CI]: Well, that's what I could reproduce locally. The errors on CI are
       unhelpful ([which I hope to fix in Brittle][brittle PR]). It's
       possible a different problem exists on CI that I couldn't repro
       locally.

[1]: https://github.com/digidem/mapeo-core-next/actions/runs/7925562783/job/21731451900
[2]: https://github.com/digidem/mapeo-core-next/actions/runs/7965492933/job/21745121763
[brittle PR]: holepunchto/brittle#44
  • Loading branch information
EvanHahn committed Feb 20, 2024
1 parent 2ec1154 commit 936a642
Showing 1 changed file with 26 additions and 39 deletions.
65 changes: 26 additions & 39 deletions test-e2e/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,50 +118,37 @@ export async function invite({
*
* @param {readonly MapeoManager[]} managers
* @param {{ waitForDeviceInfo?: boolean }} [opts] Optionally wait for device names to be set
* @returns {Promise<void>}
*/
export async function waitForPeers(
managers,
{ waitForDeviceInfo = false } = {}
) {
const peerCounts = managers.map((manager) => {
return manager[kRPC].peers.filter(({ status }) => status === 'connected')
.length
})
const peersHaveNames = managers.map((manager) => {
return manager[kRPC].peers.every(({ name }) => !!name)
})
const expectedCount = managers.length - 1
return new Promise((res) => {
if (
peerCounts.every((v) => v === expectedCount) &&
(!waitForDeviceInfo || peersHaveNames.every((v) => v))
) {
return res(null)
}
for (const [idx, manager] of managers.entries()) {
manager.on('local-peers', function onPeers(peers) {
const connectedPeerCount = peers.filter(
export const waitForPeers = (managers, { waitForDeviceInfo = false } = {}) =>
new Promise((res) => {
const expectedCount = managers.length - 1
const isDone = () =>
managers.every((manager) => {
const { peers } = manager[kRPC]
const connectedPeers = peers.filter(
({ status }) => status === 'connected'
).length
const allHaveNames = peers.every(({ name }) => !!name)
peerCounts[idx] = connectedPeerCount
peersHaveNames[idx] = allHaveNames
if (
connectedPeerCount === expectedCount &&
(!waitForDeviceInfo || allHaveNames)
) {
manager.off('local-peers', onPeers)
}
if (
peerCounts.every((v) => v === expectedCount) &&
(!waitForDeviceInfo || peersHaveNames.every((v) => v))
) {
res(null)
}
)
return (
connectedPeers.length === expectedCount &&
(!waitForDeviceInfo || connectedPeers.every(({ name }) => !!name))
)
})

if (isDone()) {
res()
return
}

const onLocalPeers = () => {
if (isDone()) {
for (const manager of managers) manager.off('local-peers', onLocalPeers)
res()
}
}

for (const manager of managers) manager.on('local-peers', onLocalPeers)
})
}

/**
* Create `count` manager instances. Each instance has a deterministic identity
Expand Down

0 comments on commit 936a642

Please sign in to comment.