Run fixture cleanup asynchronously, after teardown

This makes the fixture cleanup _always_ run after any user-supplied
teardown functions (which was ambiguous before), and removes using the
async form of `rimraf` so that it can retry with backoff if the
directories are locked.

Fixes: #677

PR-URL: #704
Close: #704

docs/src/content/docs/api/fixtures/index.md

@@ -92,3 +92,34 @@ The name is determined by the filename and path of the `main` script.  If
 no `main` script is available (for example, if running tap in a node repl),
 then it defaults to the folder name `TAP` in the current working
 directory.
+
+## Timing Caveat
+
+While the fixture directory is _created_ synchronously, it is _removed_
+asynchronously, because that is the only way to get around `ENOTEMPTY`
+errors on Windows.
+
+This means that the next test after one that uses `t.testdir()` will be
+deferred until after the end of the current run to completion.  So, for
+example:
+
+```js
+t.test('first test', t => {
+  console.error('one')
+  t.testdir()
+  t.end()
+})
+console.error('two')
+t.test('second test', t => {
+  console.error('three')
+  t.end()
+})
+console.error('four')
+```
+
+This will print `one two four three` instead of `one two three four`,
+because the second test is deferred while waiting for the first test's
+fixture dir to be removed.
+
+The fixture directory cleanup will always happen _after_ any user-scheduled
+`t.teardown()` functions, as of tap v14.11.0.

lib/test.js

@@ -58,6 +58,7 @@ const _beforeEnd = Symbol('_beforeEnd')
 const _emits = Symbol('_emits')
 const _nextChildId = Symbol('_nextChildId')
 const _expectUncaught = Symbol('_expectUncaught')
+const _createdFixture = Symbol('_createdFixture')
 const Snapshot = require('./snapshot.js')
 
 const hasOwn = (obj, key) =>
@@ -122,6 +123,7 @@ class Test extends Base {
 
     this.doingStdinOnly = false
     this.onTeardown = []
+    this[_createdFixture] = false
     this.subtests = []
     this.pool = new Pool()
     this.queue = ['TAP version 13\n']
@@ -516,6 +518,7 @@ class Test extends Base {
   }
 
   emitSubTeardown (p) {
+    // if it's not a thing that CAN have teardowns, nothing to do here
     if (!p.onTeardown)
       return
 
@@ -547,7 +550,17 @@ class Test extends Base {
       }
     }
 
-    p.emit('teardown')
+    // ok we're done, just delete the fixture if it created one.
+    // do this AFTER all user-generated teardowns, and asynchronously so
+    // that we can do the fancy backoff dance for Win32's weirdo fs.
+    if (p[_createdFixture])
+      return new Promise((res, rej) => {
+        rimraf(p[_createdFixture], er =>
+          er ? /* istanbul ignore next - rimraf never fails lol */ rej(er)
+          : res())
+      }).then(() => p.emit('teardown'))
+    else
+      p.emit('teardown')
   }
 
   writeSubComment (p, cb) {
@@ -1272,7 +1285,7 @@ class Test extends Base {
     const dir = this.testdirName
     rimraf.sync(dir)
     if (!this.saveFixture)
-      this.teardown(() => rimraf.sync(dir))
+      this[_createdFixture] = dir
     Fixture.make(dir, fixture || {})
     return dir
   }

test/test.js

@@ -1198,10 +1198,12 @@ t.test('test dir name does not throw when no main module is present', t => {
   })
 })
 
-t.test('save a fixture', t => {
+t.test('fixture dir stuff', t => {
   const tdn = t.testdirName
   t.throws(() => fs.statSync(tdn), 'doesnt exist yet')
+  t.teardown(() => fs.statSync(tdn), 'exists in teardown')
   const dir = t.testdir()
+  t.teardown(() => fs.statSync(tdn), 'exists in teardown scheduled after testdir')
   t.equal(dir, tdn)
   t.ok(fs.statSync(dir).isDirectory(), 'made directory')
   t.testdir({ file: 'contents' })
@@ -1213,12 +1215,21 @@ t.test('save a fixture', t => {
   t.throws(() => fs.statSync(`${dir}/file`), 'old dir cleared out')
   t.equal(fs.readFileSync(`${dir}/file2`, 'utf8'), 'contents', 'made file')
   t.equal(fs.readlinkSync(`${dir}/link`), 'file2', 'made symlink')
+  let removeDir
+  t.test('remove the dir when its done', t => {
+    removeDir = t.testdir()
+    t.end()
+  })
   let leaveDir
   t.test('leave the dir behind', { saveFixture: true }, t => {
+    t.throws(() => fs.statSync(removeDir), 'previous dir was removed')
     leaveDir = t.testdir()
     t.parent.teardown(() => rimraf.sync(leaveDir))
     t.end()
   })
-  t.ok(fs.statSync(leaveDir).isDirectory(), 'left dir behind')
+  t.test('check leaveDir is still there', t => {
+    t.ok(fs.statSync(leaveDir).isDirectory(), 'left dir behind')
+    t.end()
+  })
   t.end()
 })