Skip to content

Commit 2f7ec52

Browse files
isaacsisaacs
committed
test: Clean up some flakiness and inconsistency
* Get rid of a lot of usage of osenv.tmpdir in tests. * Remove unnecessary creation/deletion of common.pkg * Reduce rimraf.sync wherever possible (often collides with common-tap.js's rimraf on windows) * Use common test utilities wherever possible. * DRY tests with test templates where it makes sense.
1 parent 3e7ed30 commit 2f7ec52

File tree

109 files changed

+1114
-2814
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

109 files changed

+1114
-2814
lines changed

‎test/common-tap.js

+2-4
Original file line numberDiff line numberDiff line change
@@ -216,17 +216,15 @@ exports.readBinLink = function (path) {
216216

217217
exports.skipIfWindows = function (why) {
218218
if (!isWindows) return
219-
console.log('1..1')
220219
if (!why) why = 'this test not available on windows'
221-
console.log('ok 1 # skip ' + why)
220+
require('tap').plan(0, why)
222221
process.exit(0)
223222
}
224223

225224
exports.pendIfWindows = function (why) {
226225
if (!isWindows) return
227-
console.log('1..1')
228226
if (!why) why = 'this test is pending further changes on windows'
229-
console.log('not ok 1 # todo ' + why)
227+
require('tap').fail(' ', { todo: why, diagnostic: false })
230228
process.exit(0)
231229
}
232230

‎test/tap/404-parent.js

+6-12
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
var common = require('../common-tap.js')
22
var test = require('tap').test
33
var npm = require('../../')
4-
var osenv = require('osenv')
54
var path = require('path')
65
var fs = require('fs')
76
var rimraf = require('rimraf')
@@ -10,20 +9,15 @@ var mr = require('npm-registry-mock')
109

1110
test('404-parent: if parent exists, specify parent in error message', function (t) {
1211
setup()
13-
rimraf.sync(path.resolve(pkg, 'node_modules'))
14-
performInstall(function (err) {
15-
t.ok(err instanceof Error, 'error was returned')
16-
t.equal(err.parent, '404-parent', "error's parent set")
17-
t.end()
12+
rimraf(path.resolve(pkg, 'node_modules'), () => {
13+
performInstall(function (err) {
14+
t.ok(err instanceof Error, 'error was returned')
15+
t.equal(err.parent, '404-parent', "error's parent set")
16+
t.end()
17+
})
1818
})
1919
})
2020

21-
test('cleanup', function (t) {
22-
process.chdir(osenv.tmpdir())
23-
rimraf.sync(pkg)
24-
t.end()
25-
})
26-
2721
function setup () {
2822
fs.writeFileSync(path.resolve(pkg, 'package.json'), JSON.stringify({
2923
author: 'Evan Lucas',

‎test/tap/access.js

+3-7
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,7 @@ test('npm access public when no package passed and no package.json', function (t
7373
function (er, code, stdout, stderr) {
7474
t.ifError(er, 'npm access')
7575
t.match(stderr, /no package name passed to command and no package.json found/)
76-
rimraf.sync(missing)
77-
t.end()
76+
rimraf(missing, t.end)
7877
})
7978
})
8079

@@ -95,8 +94,7 @@ test('npm access public when no package passed and invalid package.json', functi
9594
function (er, code, stdout, stderr) {
9695
t.ifError(er, 'npm access')
9796
t.match(stderr, /Failed to parse json/)
98-
rimraf.sync(invalid)
99-
t.end()
97+
rimraf(invalid, t.end)
10098
})
10199
})
102100

@@ -405,8 +403,7 @@ test('npm access ls-packages with no package specified or package.json', functio
405403
function (er, code, stdout, stderr) {
406404
t.ifError(er, 'npm access ls-packages')
407405
t.same(JSON.parse(stdout), clientPackages)
408-
rimraf.sync(missing)
409-
t.end()
406+
rimraf(missing, t.end)
410407
}
411408
)
412409
})
@@ -557,7 +554,6 @@ test('npm access blerg', function (t) {
557554

558555
test('cleanup', function (t) {
559556
t.pass('cleaned up')
560-
rimraf.sync(pkg)
561557
server.done()
562558
server.close()
563559
t.end()

‎test/tap/add-remote-git-file.js

+3-21
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,16 @@ var fs = require('fs')
44
var resolve = require('path').resolve
55
var url = require('url')
66

7-
var osenv = require('osenv')
87
var mkdirp = require('mkdirp')
9-
var rimraf = require('rimraf')
108
var test = require('tap').test
119

1210
var npm = require('../../lib/npm.js')
1311
var fetchPackageMetadata = require('../../lib/fetch-package-metadata.js')
1412
var common = require('../common-tap.js')
1513

16-
var pkg = common.pkg
17-
var repo = common.pkg + '-repo'
14+
var pkg = resolve(common.pkg, 'package')
15+
var repo = resolve(common.pkg, 'repo')
16+
mkdirp.sync(pkg)
1817

1918
var git
2019
var cloneURL = 'git+file://' + resolve(pkg, 'child.git')
@@ -25,7 +24,6 @@ var pjChild = JSON.stringify({
2524
}, null, 2) + '\n'
2625

2726
test('setup', function (t) {
28-
bootstrap()
2927
setup(function (er, r) {
3028
t.ifError(er, 'git started up successfully')
3129

@@ -70,16 +68,6 @@ test('save install', function (t) {
7068
})
7169
})
7270

73-
test('clean', function (t) {
74-
cleanup()
75-
t.end()
76-
})
77-
78-
function bootstrap () {
79-
cleanup()
80-
mkdirp.sync(pkg)
81-
}
82-
8371
function setup (cb) {
8472
mkdirp.sync(repo)
8573
fs.writeFileSync(resolve(repo, 'package.json'), pjChild)
@@ -95,9 +83,3 @@ function setup (cb) {
9583
}, cb)
9684
})
9785
}
98-
99-
function cleanup () {
100-
process.chdir(osenv.tmpdir())
101-
rimraf.sync(repo)
102-
rimraf.sync(pkg)
103-
}

‎test/tap/add-remote-git-shrinkwrap.js

+5-21
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
11
var fs = require('fs')
22
var resolve = require('path').resolve
33

4-
var osenv = require('osenv')
54
var mkdirp = require('mkdirp')
6-
var rimraf = require('rimraf')
75
var test = require('tap').test
86

97
var npm = require('../../lib/npm.js')
108
var common = require('../common-tap.js')
119

12-
var pkg = common.pkg
13-
var repo = pkg + '-repo'
10+
var pkg = resolve(common.pkg, 'package')
11+
var repo = resolve(common.pkg, 'repo')
1412

1513
var daemon
1614
var daemonPID
@@ -30,7 +28,8 @@ var pjChild = JSON.stringify({
3028
}, null, 2) + '\n'
3129

3230
test('setup', function (t) {
33-
bootstrap()
31+
mkdirp.sync(pkg)
32+
fs.writeFileSync(resolve(pkg, 'package.json'), pjParent)
3433
setup(function (er, r) {
3534
t.ifError(er, 'git started up successfully')
3635

@@ -85,19 +84,10 @@ test('shrinkwrap gets correct _from and _resolved (#7121)', function (t) {
8584
})
8685

8786
test('clean', function (t) {
88-
daemon.on('close', function () {
89-
cleanup()
90-
t.end()
91-
})
87+
daemon.on('close', t.end)
9288
process.kill(daemonPID)
9389
})
9490

95-
function bootstrap () {
96-
cleanup()
97-
mkdirp.sync(pkg)
98-
fs.writeFileSync(resolve(pkg, 'package.json'), pjParent)
99-
}
100-
10191
function setup (cb) {
10292
mkdirp.sync(repo)
10393
fs.writeFileSync(resolve(repo, 'package.json'), pjChild)
@@ -145,9 +135,3 @@ function setup (cb) {
145135
}, cb)
146136
})
147137
}
148-
149-
function cleanup () {
150-
process.chdir(osenv.tmpdir())
151-
rimraf.sync(repo)
152-
rimraf.sync(pkg)
153-
}

‎test/tap/add-remote-git-submodule.js

+4-11
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
var fs = require('fs')
22
var resolve = require('path').resolve
33

4-
var osenv = require('osenv')
4+
var cwd = process.cwd()
55
var mkdirp = require('mkdirp')
66
var rimraf = require('rimraf')
77
var test = require('tap').test
88

99
var npm = require('../../lib/npm.js')
1010
var common = require('../common-tap.js')
1111

12-
var pkg = common.pkg
13-
var repos = pkg + '-repos'
12+
var pkg = resolve(common.pkg, 'package')
13+
var repos = resolve(common.pkg, 'repos')
1414
var subwt = resolve(repos, 'subwt')
1515
var topwt = resolve(repos, 'topwt')
1616
var suburl = 'git://localhost:' + common.gitPort + '/sub.git'
@@ -62,14 +62,13 @@ test('has file in submodule', function (t) {
6262

6363
test('clean', function (t) {
6464
daemon.on('close', function () {
65-
cleanup()
6665
t.end()
6766
})
6867
process.kill(daemonPID)
6968
})
7069

7170
function bootstrap (t) {
72-
process.chdir(osenv.tmpdir())
71+
process.chdir(cwd)
7372
rimraf.sync(pkg)
7473
mkdirp.sync(pkg)
7574
process.chdir(pkg)
@@ -141,9 +140,3 @@ function setup (cb) {
141140
}, cb)
142141
})
143142
}
144-
145-
function cleanup () {
146-
process.chdir(osenv.tmpdir())
147-
rimraf.sync(repos)
148-
rimraf.sync(pkg)
149-
}

‎test/tap/add-remote-git.js

+5-22
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
11
var fs = require('fs')
22
var resolve = require('path').resolve
33

4-
var osenv = require('osenv')
54
var mkdirp = require('mkdirp')
6-
var rimraf = require('rimraf')
75
var test = require('tap').test
86

97
var npm = require('../../lib/npm.js')
108
var common = require('../common-tap.js')
119

12-
var pkg = common.pkg
13-
var repo = pkg + '-repo'
10+
var pkg = resolve(common.pkg, 'package')
11+
var repo = resolve(pkg, 'repo')
1412

1513
var daemon
1614
var daemonPID
@@ -30,7 +28,8 @@ var pjChild = JSON.stringify({
3028
}, null, 2) + '\n'
3129

3230
test('setup', function (t) {
33-
bootstrap()
31+
mkdirp.sync(pkg)
32+
fs.writeFileSync(resolve(pkg, 'package.json'), pjParent)
3433
setup(function (er, r) {
3534
t.ifError(er, 'git started up successfully')
3635

@@ -47,25 +46,15 @@ test('install from repo', function (t) {
4746
process.chdir(pkg)
4847
npm.commands.install('.', [], function (er) {
4948
t.ifError(er, 'npm installed via git')
50-
5149
t.end()
5250
})
5351
})
5452

5553
test('clean', function (t) {
56-
daemon.on('close', function () {
57-
cleanup()
58-
t.end()
59-
})
54+
daemon.on('close', t.end)
6055
process.kill(daemonPID)
6156
})
6257

63-
function bootstrap () {
64-
cleanup()
65-
mkdirp.sync(pkg)
66-
fs.writeFileSync(resolve(pkg, 'package.json'), pjParent)
67-
}
68-
6958
function setup (cb) {
7059
mkdirp.sync(repo)
7160
fs.writeFileSync(resolve(repo, 'package.json'), pjChild)
@@ -113,9 +102,3 @@ function setup (cb) {
113102
}, cb)
114103
})
115104
}
116-
117-
function cleanup () {
118-
process.chdir(osenv.tmpdir())
119-
rimraf.sync(repo)
120-
rimraf.sync(pkg)
121-
}

‎test/tap/all-package-metadata.js

+7-13
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ function setup () {
2626
mkdirp.sync(cacheBase)
2727
}
2828

29-
function cleanup () {
30-
rimraf.sync(PKG_DIR)
29+
function cleanup (cb) {
30+
rimraf(PKG_DIR, cb)
3131
}
3232

3333
test('setup', function (t) {
@@ -88,8 +88,7 @@ test('allPackageMetadata full request', function (t) {
8888
}
8989
}, 'cache contents based on what was written')
9090
server.done()
91-
cleanup()
92-
t.end()
91+
cleanup(t.end)
9392
})
9493
})
9594

@@ -126,8 +125,7 @@ test('allPackageMetadata cache only', function (t) {
126125
t.ok(fileData, 'cache contents written to the right file')
127126
t.deepEquals(fileData, cacheContents, 'cacheContents written directly')
128127
server.done()
129-
cleanup()
130-
t.end()
128+
cleanup(t.end)
131129
})
132130
})
133131

@@ -188,8 +186,7 @@ test('createEntryStream merged stream', function (t) {
188186
t.ok(fileData, 'cache contents written to the right file')
189187
t.deepEquals(fileData, cacheContents, 'cache updated correctly')
190188
server.done()
191-
cleanup()
192-
t.end()
189+
cleanup(t.end)
193190
})
194191
})
195192

@@ -205,14 +202,11 @@ test('allPackageMetadata no sources', function (t) {
205202
t.ok(err, 'no sources, got an error')
206203
t.match(err.message, /No search sources available/, 'useful error message')
207204
server.done()
208-
cleanup()
209-
t.end()
205+
cleanup(t.end)
210206
})
211207
})
212208

213209
test('cleanup', function (t) {
214-
cleanup()
215210
server.close()
216-
t.pass('all done')
217-
t.done()
211+
cleanup(t.end)
218212
})

0 commit comments

Comments
 (0)