From d12f3df8ec7355af03535d55db2ddba61d094565 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Tue, 27 Jul 2021 09:28:33 +0100 Subject: [PATCH] fix: fix flaky pubsub test In the http client, when we subscribe to a topic we open a HTTP connection which we keep open for the duration of the subscription. When we unsubscribe we abort the connection but it can remain open for a little while after the abort, even if we try to wait on the `fetch` command ending before continuting, which leads to the topic still being present in the subs list, so retry asserting that the subs list is empty in the tests within a certain time window. Fixes this sort of error: ``` ipfs: 1) interface-ipfs-core over ipfs-http-client tests against go-ipfs ipfs: .pubsub.unsubscribe ipfs: should subscribe 5 handlers and unsubscribe once with no reference to the handlers: ipfs: AssertionError: expected [ Array(1) ] to deeply equal [] ipfs: + expected - actual ipfs: -[ ipfs: - "pubsub-tests-SVOFzpM5DtbcI7jBETrmm" ipfs: -] ipfs: +[] ``` --- .../src/pubsub/unsubscribe.js | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/packages/interface-ipfs-core/src/pubsub/unsubscribe.js b/packages/interface-ipfs-core/src/pubsub/unsubscribe.js index 0caa42760a..55bf5813a4 100644 --- a/packages/interface-ipfs-core/src/pubsub/unsubscribe.js +++ b/packages/interface-ipfs-core/src/pubsub/unsubscribe.js @@ -3,7 +3,8 @@ const { isBrowser, isWebWorker, isElectronRenderer } = require('ipfs-utils/src/env') const { getTopic } = require('./utils') -const { getDescribe, getIt, expect } = require('../utils/mocha') +const { getDescribe, getIt } = require('../utils/mocha') +const waitFor = require('../utils/wait-for') /** @typedef { import("ipfsd-ctl/src/factory") } Factory */ /** @@ -40,7 +41,18 @@ module.exports = (common, options) => { await ipfs.pubsub.unsubscribe(someTopic, handlers[i]) } - return expect(ipfs.pubsub.ls()).to.eventually.eql([]) + // Unsubscribing in the http client aborts the connection we hold open + // but does not wait for it to close so the subscription list sometimes + // takes a little time to empty + await waitFor(async () => { + const subs = await ipfs.pubsub.ls() + + return subs.length === 0 + }, { + interval: 1000, + timeout: 30000, + name: 'subscriptions to be empty' + }) }) it(`should subscribe ${count} handlers and unsubscribe once with no reference to the handlers`, async () => { @@ -50,7 +62,18 @@ module.exports = (common, options) => { } await ipfs.pubsub.unsubscribe(someTopic) - return expect(ipfs.pubsub.ls()).to.eventually.eql([]) + // Unsubscribing in the http client aborts the connection we hold open + // but does not wait for it to close so the subscription list sometimes + // takes a little time to empty + await waitFor(async () => { + const subs = await ipfs.pubsub.ls() + + return subs.length === 0 + }, { + interval: 1000, + timeout: 30000, + name: 'subscriptions to be empty' + }) }) }) }