Skip to content
This repository was archived by the owner on Feb 12, 2024. It is now read-only.

Commit 5cc3072

Browse files
achingbrainachingbrain
committed
chore: remove try-catch from tests where functions are async
We had a few instances in the tests where assertions may be missed due to functions not throwing where we thought they would so this PR refactors that code to use `.then(onResult, onReject)` instead. Follows on from #2514 (comment)
1 parent 50f3667 commit 5cc3072

File tree

11 files changed

+127
-163
lines changed

11 files changed

+127
-163
lines changed

‎package.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -173,12 +173,12 @@
173173
"pull-defer": "~0.2.3",
174174
"pull-file": "^1.1.0",
175175
"pull-mplex": "~0.1.1",
176-
"pull-ndjson": "~0.1.1",
176+
"pull-ndjson": "achingbrain/pull-ndjson#upgrade-deps",
177177
"pull-pushable": "^2.2.0",
178178
"pull-sort": "^1.0.1",
179179
"pull-stream": "^3.6.14",
180180
"pull-stream-to-async-iterator": "^1.0.2",
181-
"pull-stream-to-stream": "^1.3.4",
181+
"pull-stream-to-stream": "^2.0.0",
182182
"pull-traverse": "^1.0.3",
183183
"readable-stream": "^3.4.0",
184184
"receptacle": "^1.3.2",

‎src/core/components/pubsub.js

+10-2
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,11 @@ module.exports = function pubsub (self) {
3333
return
3434
}
3535

36-
checkOnlineAndEnabled()
36+
try {
37+
checkOnlineAndEnabled()
38+
} catch (err) {
39+
return Promise.reject(err)
40+
}
3741

3842
return self.libp2p.pubsub.subscribe(topic, handler, options)
3943
},
@@ -50,7 +54,11 @@ module.exports = function pubsub (self) {
5054
return
5155
}
5256

53-
checkOnlineAndEnabled()
57+
try {
58+
checkOnlineAndEnabled()
59+
} catch (err) {
60+
return Promise.reject(err)
61+
}
5462

5563
return self.libp2p.pubsub.unsubscribe(topic, handler)
5664
},

‎test/cli/daemon.js

+41-62
Original file line numberDiff line numberDiff line change
@@ -104,14 +104,13 @@ describe('daemon', () => {
104104
}
105105
})
106106

107-
try {
108-
await daemon
109-
throw new Error('Did not kill process')
110-
} catch (err) {
111-
expect(err.killed).to.be.true()
112-
113-
expect(stdout).to.include('Daemon is ready')
114-
}
107+
await daemon.then(
108+
() => expect.fail('Did not kill process'),
109+
(err) => {
110+
expect(err.killed).to.be.true()
111+
expect(stdout).to.include('Daemon is ready')
112+
}
113+
)
115114
})
116115

117116
it('should allow bind to multiple addresses for API and Gateway', async function () {
@@ -142,15 +141,15 @@ describe('daemon', () => {
142141
}
143142
})
144143

145-
try {
146-
await daemon
147-
throw new Error('Did not kill process')
148-
} catch (err) {
149-
expect(err.killed).to.be.true()
144+
await daemon.then(
145+
() => expect.fail('Did not kill process'),
146+
(err) => {
147+
expect(err.killed).to.be.true()
150148

151-
apiAddrs.forEach(addr => expect(err.stdout).to.include(`API listening on ${addr.slice(0, -2)}`))
152-
gatewayAddrs.forEach(addr => expect(err.stdout).to.include(`Gateway (read only) listening on ${addr.slice(0, -2)}`))
153-
}
149+
apiAddrs.forEach(addr => expect(err.stdout).to.include(`API listening on ${addr.slice(0, -2)}`))
150+
gatewayAddrs.forEach(addr => expect(err.stdout).to.include(`Gateway (read only) listening on ${addr.slice(0, -2)}`))
151+
}
152+
)
154153
})
155154

156155
it('should allow no bind addresses for API and Gateway', async function () {
@@ -171,15 +170,15 @@ describe('daemon', () => {
171170
}
172171
})
173172

174-
try {
175-
await daemon
176-
throw new Error('Did not kill process')
177-
} catch (err) {
178-
expect(err.killed).to.be.true()
173+
await daemon.then(
174+
() => expect.fail('Did not kill process'),
175+
(err) => {
176+
expect(err.killed).to.be.true()
179177

180-
expect(err.stdout).to.not.include('API listening on')
181-
expect(err.stdout).to.not.include('Gateway (read only) listening on')
182-
}
178+
expect(err.stdout).to.not.include('API listening on')
179+
expect(err.stdout).to.not.include('Gateway (read only) listening on')
180+
}
181+
)
183182
})
184183

185184
skipOnWindows('should handle SIGINT gracefully', async function () {
@@ -211,32 +210,14 @@ describe('daemon', () => {
211210
await ipfs('init')
212211

213212
const daemon = ipfs('daemon --silent')
214-
const stop = async (err) => {
215-
daemon.kill()
216-
217-
if (err) {
218-
throw err
219-
}
220213

221-
try {
222-
await daemon
223-
} catch (err) {
224-
if (!err.killed) {
225-
throw err
226-
}
227-
}
228-
}
214+
setTimeout(() => {
215+
daemon.kill()
216+
}, 5 * 1000)
229217

230-
return new Promise((resolve, reject) => {
231-
daemon.stdout.on('data', (data) => {
232-
reject(new Error('Output was received ' + data.toString('utf8')))
233-
})
218+
const output = await daemon
234219

235-
setTimeout(() => {
236-
resolve()
237-
}, 5 * 1000)
238-
})
239-
.then(stop, stop)
220+
expect(output).to.be.empty()
240221
})
241222

242223
it('should present ipfs path help when option help is received', async function () {
@@ -262,16 +243,16 @@ describe('daemon', () => {
262243
}
263244
})
264245

265-
try {
266-
await daemon
267-
throw new Error('Did not kill process')
268-
} catch (err) {
269-
expect(err.killed).to.be.true()
246+
await daemon.then(
247+
() => expect.fail('Did not kill process'),
248+
(err) => {
249+
expect(err.killed).to.be.true()
270250

271-
expect(err.stdout).to.include(`js-ipfs version: ${pkg.version}`)
272-
expect(err.stdout).to.include(`System version: ${os.arch()}/${os.platform()}`)
273-
expect(err.stdout).to.include(`Node.js version: ${process.versions.node}`)
274-
}
251+
expect(err.stdout).to.include(`js-ipfs version: ${pkg.version}`)
252+
expect(err.stdout).to.include(`System version: ${os.arch()}/${os.platform()}`)
253+
expect(err.stdout).to.include(`Node.js version: ${process.versions.node}`)
254+
}
255+
)
275256
})
276257

277258
it('should init by default', async function () {
@@ -290,12 +271,10 @@ describe('daemon', () => {
290271
}
291272
})
292273

293-
try {
294-
await daemon
295-
throw new Error('Did not kill process')
296-
} catch (err) {
297-
expect(err.killed).to.be.true()
298-
}
274+
await daemon.then(
275+
() => expect.fail('Did not kill process'),
276+
(err) => expect(err.killed).to.be.true()
277+
)
299278

300279
expect(fs.existsSync(repoPath)).to.be.true()
301280
})

‎test/cli/init.js

+4-5
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,10 @@ describe('init', function () {
8383
it('profile non-existent', async function () {
8484
this.timeout(40 * 1000)
8585

86-
try {
87-
await ipfs('init --profile doesnt-exist')
88-
} catch (err) {
89-
expect(err.stdout).includes('Could not find profile')
90-
}
86+
await ipfs('init --profile doesnt-exist').then(
87+
() => expect.fail('Should have thrown'),
88+
(err) => expect(err.stdout).includes('Could not find profile')
89+
)
9190
})
9291

9392
it('should present ipfs path help when option help is received', async function () {

‎test/core/create-node.spec.js

+11-13
Original file line numberDiff line numberDiff line change
@@ -171,20 +171,18 @@ describe('create node', function () {
171171

172172
expect(ipfs.isOnline()).to.be.false()
173173

174-
try {
175-
await ipfs.ready
176-
} catch (err) {
177-
expect(ipfs.isOnline()).to.be.false()
178-
179-
// After the error has occurred, it should still reject
180-
try {
181-
await ipfs.ready
182-
} catch (_) {
183-
return
184-
}
185-
}
174+
await ipfs.ready.then(
175+
() => expect.fail('Should have thrown'),
176+
(err) => expect(err.message).to.contain('Expected modulus bit count >= 512')
177+
)
178+
179+
expect(ipfs.isOnline()).to.be.false()
186180

187-
throw new Error('ready promise did not reject')
181+
// After the error has occurred, it should still reject
182+
await ipfs.ready.then(
183+
() => expect.fail('Should have thrown'),
184+
(err) => expect(err.message).to.contain('Expected modulus bit count >= 512')
185+
)
188186
})
189187

190188
it('should create a ready node with IPFS.create', async () => {

‎test/core/dht.spec.js

+5-10
Original file line numberDiff line numberDiff line change
@@ -74,16 +74,11 @@ describe.skip('dht', () => {
7474
})
7575

7676
describe('put', () => {
77-
it('should callback with error for DHT not available', async () => {
78-
let res
79-
try {
80-
res = await ipfs.dht.put(Buffer.from('a'), Buffer.from('b'))
81-
} catch (err) {
82-
expect(err).to.exist()
83-
expect(err.code).to.equal('ERR_DHT_DISABLED')
84-
}
85-
86-
expect(res).to.not.exist()
77+
it('should error when DHT not available', async () => {
78+
await ipfs.dht.put(Buffer.from('a'), Buffer.from('b')).then(
79+
() => expect.fail('Should have thrown'),
80+
(err) => expect(err.code).to.equal('ERR_DHT_DISABLED')
81+
)
8782
})
8883
})
8984
})

‎test/core/gc.spec.js

+5-6
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ const IPFSFactory = require('ipfsd-ctl')
77
const pEvent = require('p-event')
88
const env = require('ipfs-utils/src/env')
99
const IPFS = require('../../src/core')
10-
const { Errors } = require('interface-datastore')
1110

1211
// We need to detect when a readLock or writeLock is requested for the tests
1312
// so we override the Mutex class to emit an event
@@ -189,11 +188,11 @@ describe('gc', function () {
189188
await rm1
190189

191190
// Second rm should fail because GC has already removed that block
192-
try {
193-
await rm2
194-
} catch (err) {
195-
expect(err.code).eql(Errors.dbDeleteFailedError().code)
196-
}
191+
const results = await rm2
192+
const result = results
193+
.filter(result => result.hash === cid2.toString())
194+
.pop()
195+
expect(result).to.have.property('error').that.contains('block not found')
197196

198197
// Confirm second block has been removed
199198
const localRefs = (await ipfs.refs.local()).map(r => r.ref)

‎test/core/libp2p.spec.js

+11-14
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ describe('libp2p customization', function () {
316316
})
317317
})
318318

319-
it('select floodsub as pubsub router if node', (done) => {
319+
it('select floodsub as pubsub router if node', async () => {
320320
const ipfs = {
321321
_repo: {
322322
datastore
@@ -334,21 +334,18 @@ describe('libp2p customization', function () {
334334
}
335335
}
336336

337-
try {
338-
_libp2p = libp2pComponent(ipfs, customConfig)
339-
} catch (err) {
340-
if (!isNode) {
341-
expect(err).to.exist()
342-
expect(err.code).to.eql('ERR_NOT_SUPPORTED')
343-
done()
344-
}
337+
if (!isNode) {
338+
await libp2pComponent(ipfs, customConfig).then(
339+
() => expect.fail('Should have thrown'),
340+
(err) => expect(err.code).to.eql('ERR_NOT_SUPPORTED')
341+
)
345342
}
346343

347-
_libp2p.start((err) => {
348-
expect(err).to.not.exist()
349-
expect(_libp2p._modules.pubsub).to.eql(require('libp2p-floodsub'))
350-
done()
351-
})
344+
_libp2p = libp2pComponent(ipfs, customConfig)
345+
346+
await _libp2p.start()
347+
348+
expect(_libp2p._modules.pubsub).to.eql(require('libp2p-floodsub'))
352349
})
353350
})
354351
})

‎test/core/name-pubsub.js

+4-5
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,10 @@ describe('name-pubsub', function () {
128128
const resolvesEmpty = await nodeB.name.resolve(idB.id)
129129
expect(resolvesEmpty).to.be.eq(emptyDirCid)
130130

131-
try {
132-
await nodeA.name.resolve(idB.id)
133-
} catch (error) {
134-
expect(error).to.exist()
135-
}
131+
await nodeA.name.resolve(idB.id).then(
132+
() => expect.fail('should have thrown'),
133+
(err) => expect(err.code).to.equal('ERR_NO_RECORD_FOUND')
134+
)
136135

137136
const publish = await nodeB.name.publish(path)
138137
expect(publish).to.be.eql({

0 commit comments

Comments
 (0)