Просмотр исходного кода

Decaf cleanup for FSPersistorManager

Simon Detheridge 6 лет назад
Родитель
Сommit
ce90292394

+ 156 - 193
services/filestore/app/js/FSPersistorManager.js

@@ -1,206 +1,169 @@
-/* eslint-disable
-    handle-callback-err,
-    no-unreachable,
-    node/no-deprecated-api,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-const logger = require('logger-sharelatex')
 const fs = require('fs')
+const logger = require('logger-sharelatex')
 const path = require('path')
-const LocalFileWriter = require('./LocalFileWriter')
-const Errors = require('./Errors')
 const rimraf = require('rimraf')
-const _ = require('underscore')
+const Stream = require('stream')
+const { promisify, callbackify } = require('util')
+
+const LocalFileWriter = require('./LocalFileWriter').promises
+const { NotFoundError, ReadError } = require('./Errors')
+
+const pipeline = promisify(Stream.pipeline)
+const fsUnlink = promisify(fs.unlink)
+const fsOpen = promisify(fs.open)
+const fsStat = promisify(fs.stat)
+const fsReaddir = promisify(fs.readdir)
+const rmrf = promisify(rimraf)
 
 const filterName = key => key.replace(/\//g, '_')
 
-module.exports = {
-  sendFile(location, target, source, callback) {
-    if (callback == null) {
-      callback = function(err) {}
-    }
-    const filteredTarget = filterName(target)
-    logger.log({ location, target: filteredTarget, source }, 'sending file')
-    const done = _.once(function(err) {
-      if (err != null) {
-        logger.err(
-          { err, location, target: filteredTarget, source },
-          'Error on put of file'
-        )
-      }
-      return callback(err)
-    })
-    // actually copy the file (instead of moving it) to maintain consistent behaviour
-    // between the different implementations
-    const sourceStream = fs.createReadStream(source)
-    sourceStream.on('error', done)
-    const targetStream = fs.createWriteStream(`${location}/${filteredTarget}`)
-    targetStream.on('error', done)
-    targetStream.on('finish', () => done())
-    return sourceStream.pipe(targetStream)
-  },
-
-  sendStream(location, target, sourceStream, callback) {
-    if (callback == null) {
-      callback = function(err) {}
-    }
-    logger.log({ location, target }, 'sending file stream')
-    sourceStream.on('error', err =>
-      logger.err({ location, target, err: err('error on stream to send') })
-    )
-    return LocalFileWriter.writeStream(sourceStream, null, (err, fsPath) => {
-      if (err != null) {
-        logger.err(
-          { location, target, fsPath, err },
-          'something went wrong writing stream to disk'
-        )
-        return callback(err)
-      }
-      return this.sendFile(location, target, fsPath, (
-        err // delete the temporary file created above and return the original error
-      ) => LocalFileWriter.deleteFile(fsPath, () => callback(err)))
-    })
-  },
-
-  // opts may be {start: Number, end: Number}
-  getFileStream(location, name, opts, callback) {
-    if (callback == null) {
-      callback = function(err, res) {}
-    }
-    const filteredName = filterName(name)
-    logger.log({ location, filteredName }, 'getting file')
-    return fs.open(`${location}/${filteredName}`, 'r', function(err, fd) {
-      if (err != null) {
-        logger.err(
-          { err, location, filteredName: name },
-          'Error reading from file'
-        )
-        if (err.code === 'ENOENT') {
-          return callback(new Errors.NotFoundError(err.message), null)
-        } else {
-          return callback(err, null)
+async function sendFile(location, target, source) {
+  const filteredTarget = filterName(target)
+  logger.log({ location, target: filteredTarget, source }, 'sending file')
+
+  // actually copy the file (instead of moving it) to maintain consistent behaviour
+  // between the different implementations
+  const sourceStream = fs.createReadStream(source)
+  const targetStream = fs.createWriteStream(`${location}/${filteredTarget}`)
+  await pipeline(sourceStream, targetStream)
+}
+
+async function sendStream(location, target, sourceStream) {
+  logger.log({ location, target }, 'sending file stream')
+
+  let fsPath
+  try {
+    fsPath = await LocalFileWriter.writeStream(sourceStream)
+    await sendFile(location, target, fsPath)
+  } finally {
+    await LocalFileWriter.deleteFile(fsPath)
+  }
+}
+
+// opts may be {start: Number, end: Number}
+async function getFileStream(location, name, opts) {
+  const filteredName = filterName(name)
+  logger.log({ location, filteredName }, 'getting file')
+
+  try {
+    opts.fd = await fsOpen(`${location}/${filteredName}`, 'r')
+  } catch (err) {
+    logger.err({ err, location, filteredName: name }, 'Error reading from file')
+
+    if (err.code === 'ENOENT') {
+      throw new NotFoundError({
+        message: 'file not found',
+        info: {
+          location,
+          filteredName
         }
-      }
-      opts.fd = fd
-      const sourceStream = fs.createReadStream(null, opts)
-      return callback(null, sourceStream)
-    })
-  },
-
-  getFileSize(location, filename, callback) {
-    const fullPath = path.join(location, filterName(filename))
-    return fs.stat(fullPath, function(err, stats) {
-      if (err != null) {
-        if (err.code === 'ENOENT') {
-          logger.log({ location, filename }, 'file not found')
-          callback(new Errors.NotFoundError(err.message))
-        } else {
-          logger.err({ err, location, filename }, 'failed to stat file')
-          callback(err)
+      }).withCause(err)
+    }
+    throw new ReadError('failed to open file for streaming').withCause(err)
+  }
+
+  return fs.createReadStream(null, opts)
+}
+
+async function getFileSize(location, filename) {
+  const fullPath = path.join(location, filterName(filename))
+
+  try {
+    const stat = await fsStat(fullPath)
+    return stat.size
+  } catch (err) {
+    logger.err({ err, location, filename }, 'failed to stat file')
+
+    if (err.code === 'ENOENT') {
+      throw new NotFoundError({
+        message: 'file not found',
+        info: {
+          location,
+          fullPath
         }
-        return
-      }
-      return callback(null, stats.size)
-    })
-  },
-
-  copyFile(location, fromName, toName, callback) {
-    if (callback == null) {
-      callback = function(err) {}
+      }).withCause(err)
     }
-    const filteredFromName = filterName(fromName)
-    const filteredToName = filterName(toName)
-    logger.log(
-      { location, fromName: filteredFromName, toName: filteredToName },
-      'copying file'
-    )
-    const sourceStream = fs.createReadStream(`${location}/${filteredFromName}`)
-    sourceStream.on('error', function(err) {
-      logger.err(
-        { err, location, key: filteredFromName },
-        'Error reading from file'
-      )
-      return callback(err)
-    })
-    const targetStream = fs.createWriteStream(`${location}/${filteredToName}`)
-    targetStream.on('error', function(err) {
-      logger.err(
-        { err, location, key: filteredToName },
-        'Error writing to file'
-      )
-      return callback(err)
-    })
-    targetStream.on('finish', () => callback(null))
-    return sourceStream.pipe(targetStream)
-  },
-
-  deleteFile(location, name, callback) {
-    const filteredName = filterName(name)
-    logger.log({ location, filteredName }, 'delete file')
-    return fs.unlink(`${location}/${filteredName}`, function(err) {
-      if (err != null) {
-        logger.err({ err, location, filteredName }, 'Error on delete.')
-        return callback(err)
-      } else {
-        return callback()
-      }
-    })
-  },
-
-  deleteDirectory(location, name, callback) {
-    if (callback == null) {
-      callback = function(err) {}
+    throw new ReadError('failed to stat file').withCause(err)
+  }
+}
+
+async function copyFile(location, fromName, toName) {
+  const filteredFromName = filterName(fromName)
+  const filteredToName = filterName(toName)
+  logger.log({ location, filteredFromName, filteredToName }, 'copying file')
+
+  const sourceStream = fs.createReadStream(`${location}/${filteredFromName}`)
+  const targetStream = fs.createWriteStream(`${location}/${filteredToName}`)
+  await pipeline(sourceStream, targetStream)
+}
+
+async function deleteFile(location, name) {
+  const filteredName = filterName(name)
+  logger.log({ location, filteredName }, 'delete file')
+  await fsUnlink(`${location}/${filteredName}`)
+}
+
+async function deleteDirectory(location, name) {
+  const filteredName = filterName(name.replace(/\/$/, ''))
+
+  logger.log({ location, filteredName }, 'deleting directory')
+
+  await rmrf(`${location}/${filteredName}`)
+}
+
+async function checkIfFileExists(location, name) {
+  const filteredName = filterName(name)
+  try {
+    const stat = await fsStat(`${location}/${filteredName}`)
+    return !!stat
+  } catch (err) {
+    if (err.code === 'ENOENT') {
+      return false
     }
-    const filteredName = filterName(name.replace(/\/$/, ''))
-    return rimraf(`${location}/${filteredName}`, function(err) {
-      if (err != null) {
-        logger.err({ err, location, filteredName }, 'Error on rimraf rmdir.')
-        return callback(err)
-      } else {
-        return callback()
-      }
-    })
-  },
-
-  checkIfFileExists(location, name, callback) {
-    if (callback == null) {
-      callback = function(err, exists) {}
+    throw new ReadError('failed to stat file').withCause(err)
+  }
+}
+
+// note, does not recurse into subdirectories
+async function directorySize(location, name) {
+  const filteredName = filterName(name.replace(/\/$/, ''))
+  let size = 0
+
+  try {
+    const files = await fsReaddir(`${location}/${filteredName}`)
+    for (const file of files) {
+      const stat = await fsStat(`${location}/${filteredName}/${file}`)
+      size += stat.size
     }
-    const filteredName = filterName(name)
-    logger.log({ location, filteredName }, 'checking if file exists')
-    return fs.exists(`${location}/${filteredName}`, function(exists) {
-      logger.log({ location, filteredName, exists }, 'checked if file exists')
-      return callback(null, exists)
-    })
-  },
-
-  directorySize(location, name, callback) {
-    const filteredName = filterName(name.replace(/\/$/, ''))
-    logger.log({ location, filteredName }, 'get project size in file system')
-    return fs.readdir(`${location}/${filteredName}`, function(err, files) {
-      if (err != null) {
-        logger.err(
-          { err, location, filteredName },
-          'something went wrong listing prefix in aws'
-        )
-        return callback(err)
-      }
-      let totalSize = 0
-      _.each(files, function(entry) {
-        const fd = fs.openSync(`${location}/${filteredName}/${entry}`, 'r')
-        const fileStats = fs.fstatSync(fd)
-        totalSize += fileStats.size
-        return fs.closeSync(fd)
-      })
-      logger.log({ totalSize }, 'total size', { files })
-      return callback(null, totalSize)
-    })
+  } catch (err) {
+    throw new ReadError({
+      message: 'failed to get directory size',
+      info: { location, name }
+    }).withCause(err)
+  }
+
+  return size
+}
+
+module.exports = {
+  sendFile: callbackify(sendFile),
+  sendStream: callbackify(sendStream),
+  getFileStream: callbackify(getFileStream),
+  getFileSize: callbackify(getFileSize),
+  copyFile: callbackify(copyFile),
+  deleteFile: callbackify(deleteFile),
+  deleteDirectory: callbackify(deleteDirectory),
+  checkIfFileExists: callbackify(checkIfFileExists),
+  directorySize: callbackify(directorySize),
+  promises: {
+    sendFile,
+    sendStream,
+    getFileStream,
+    getFileSize,
+    copyFile,
+    deleteFile,
+    deleteDirectory,
+    checkIfFileExists,
+    directorySize
   }
 }

+ 214 - 409
services/filestore/test/unit/js/FSPersistorManagerTests.js

@@ -1,502 +1,307 @@
-/* eslint-disable
-    handle-callback-err,
-    no-return-assign,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-const { assert } = require('chai')
 const sinon = require('sinon')
 const chai = require('chai')
-const { should } = chai
 const { expect } = chai
-const modulePath = '../../../app/js/FSPersistorManager.js'
 const SandboxedModule = require('sandboxed-module')
-const fs = require('fs')
-const response = require('response')
+const Errors = require('../../../app/js/Errors')
+
+chai.use(require('sinon-chai'))
+chai.use(require('chai-as-promised'))
+
+const modulePath = '../../../app/js/FSPersistorManager.js'
 
 describe('FSPersistorManagerTests', function() {
+  const stat = { size: 4 }
+  const fd = 1234
+  const readStream = 'readStream'
+  const writeStream = 'writeStream'
+  const remoteStream = 'remoteStream'
+  const tempFile = '/tmp/potato.txt'
+  const location = '/foo'
+  const error = new Error('guru meditation error')
+
+  const files = ['wombat.txt', 'potato.tex']
+  let fs, rimraf, stream, LocalFileWriter, FSPersistorManager
+
   beforeEach(function() {
-    this.Fs = {
-      rename: sinon.stub(),
-      createReadStream: sinon.stub(),
-      createWriteStream: sinon.stub(),
-      unlink: sinon.stub(),
-      rmdir: sinon.stub(),
-      exists: sinon.stub(),
-      readdir: sinon.stub(),
-      open: sinon.stub(),
-      openSync: sinon.stub(),
-      fstatSync: sinon.stub(),
-      closeSync: sinon.stub(),
-      stat: sinon.stub()
+    fs = {
+      createReadStream: sinon.stub().returns(readStream),
+      createWriteStream: sinon.stub().returns(writeStream),
+      unlink: sinon.stub().yields(),
+      open: sinon.stub().yields(null, fd),
+      readdir: sinon.stub().yields(null, files),
+      stat: sinon.stub().yields(null, stat)
     }
-    this.Rimraf = sinon.stub()
-    this.LocalFileWriter = {
-      writeStream: sinon.stub(),
-      deleteFile: sinon.stub()
+    rimraf = sinon.stub().yields()
+    stream = { pipeline: sinon.stub().yields() }
+    LocalFileWriter = {
+      promises: {
+        writeStream: sinon.stub().resolves(tempFile),
+        deleteFile: sinon.stub().resolves()
+      }
     }
-    this.requires = {
-      './LocalFileWriter': this.LocalFileWriter,
-      fs: this.Fs,
-      'logger-sharelatex': {
-        log() {},
-        err() {}
+    FSPersistorManager = SandboxedModule.require(modulePath, {
+      requires: {
+        './LocalFileWriter': LocalFileWriter,
+        fs: fs,
+        'logger-sharelatex': {
+          log() {},
+          err() {}
+        },
+        rimraf: rimraf,
+        stream: stream,
+        './Errors': Errors
       },
-      response: response,
-      rimraf: this.Rimraf,
-      './Errors': (this.Errors = { NotFoundError: sinon.stub() })
-    }
-    this.location = '/tmp'
-    this.name1 = '530f2407e7ef165704000007/530f838b46d9a9e859000008'
-    this.name1Filtered = '530f2407e7ef165704000007_530f838b46d9a9e859000008'
-    this.name2 = 'second_file'
-    this.error = 'error_message'
-    return (this.FSPersistorManager = SandboxedModule.require(modulePath, {
-      requires: this.requires
-    }))
+      globals: { console }
+    })
   })
 
   describe('sendFile', function() {
-    beforeEach(function() {
-      return (this.Fs.createReadStream = sinon.stub().returns({
-        on() {},
-        pipe() {}
-      }))
-    })
-
-    it('should copy the file', function(done) {
-      this.Fs.createWriteStream = sinon.stub().returns({
-        on(event, handler) {
-          if (event === 'finish') {
-            return process.nextTick(handler)
-          }
-        }
-      })
-      return this.FSPersistorManager.sendFile(
-        this.location,
-        this.name1,
-        this.name2,
-        err => {
-          this.Fs.createReadStream.calledWith(this.name2).should.equal(true)
-          this.Fs.createWriteStream
-            .calledWith(`${this.location}/${this.name1Filtered}`)
-            .should.equal(true)
-          return done()
-        }
+    it('should copy the file', async function() {
+      await FSPersistorManager.promises.sendFile(location, files[0], files[1])
+      expect(fs.createReadStream).to.have.been.calledWith(files[1])
+      expect(fs.createWriteStream).to.have.been.calledWith(
+        `${location}/${files[0]}`
       )
+      expect(stream.pipeline).to.have.been.calledWith(readStream, writeStream)
     })
 
-    return it('should return an error if the file cannot be stored', function(done) {
-      this.Fs.createWriteStream = sinon.stub().returns({
-        on: (event, handler) => {
-          if (event === 'error') {
-            return process.nextTick(() => {
-              return handler(this.error)
-            })
-          }
-        }
-      })
-      return this.FSPersistorManager.sendFile(
-        this.location,
-        this.name1,
-        this.name2,
-        err => {
-          this.Fs.createReadStream.calledWith(this.name2).should.equal(true)
-          this.Fs.createWriteStream
-            .calledWith(`${this.location}/${this.name1Filtered}`)
-            .should.equal(true)
-          err.should.equal(this.error)
-          return done()
-        }
-      )
+    it('should return an error if the file cannot be stored', async function() {
+      stream.pipeline.yields(error)
+      await expect(
+        FSPersistorManager.promises.sendFile(location, files[0], files[1])
+      ).to.eventually.be.rejectedWith(error)
     })
   })
 
   describe('sendStream', function() {
-    beforeEach(function() {
-      this.FSPersistorManager.sendFile = sinon.stub().callsArgWith(3)
-      this.LocalFileWriter.writeStream.callsArgWith(2, null, this.name1)
-      this.LocalFileWriter.deleteFile.callsArg(1)
-      return (this.SourceStream = { on() {} })
+    it('should send the stream to LocalFileWriter', async function() {
+      await FSPersistorManager.promises.sendStream(
+        location,
+        files[0],
+        remoteStream
+      )
+      expect(LocalFileWriter.promises.writeStream).to.have.been.calledWith(
+        remoteStream
+      )
     })
 
-    it('should sent stream to LocalFileWriter', function(done) {
-      return this.FSPersistorManager.sendStream(
-        this.location,
-        this.name1,
-        this.SourceStream,
-        () => {
-          this.LocalFileWriter.writeStream
-            .calledWith(this.SourceStream)
-            .should.equal(true)
-          return done()
-        }
+    it('should delete the temporary file', async function() {
+      await FSPersistorManager.promises.sendStream(
+        location,
+        files[0],
+        remoteStream
+      )
+      expect(LocalFileWriter.promises.deleteFile).to.have.been.calledWith(
+        tempFile
       )
     })
 
-    it('should return the error from LocalFileWriter', function(done) {
-      this.LocalFileWriter.writeStream.callsArgWith(2, this.error)
-      return this.FSPersistorManager.sendStream(
-        this.location,
-        this.name1,
-        this.SourceStream,
-        err => {
-          err.should.equal(this.error)
-          return done()
-        }
-      )
+    it('should return the error from LocalFileWriter', async function() {
+      LocalFileWriter.promises.writeStream.rejects(error)
+      await expect(
+        FSPersistorManager.promises.sendStream(location, files[0], remoteStream)
+      ).to.eventually.be.rejectedWith(error)
     })
 
-    return it('should send the file to the filestore', function(done) {
-      this.LocalFileWriter.writeStream.callsArgWith(2)
-      return this.FSPersistorManager.sendStream(
-        this.location,
-        this.name1,
-        this.SourceStream,
-        err => {
-          this.FSPersistorManager.sendFile.called.should.equal(true)
-          return done()
-        }
+    it('should send the temporary file to the filestore', async function() {
+      await FSPersistorManager.promises.sendStream(
+        location,
+        files[0],
+        remoteStream
       )
+      expect(fs.createReadStream).to.have.been.calledWith(tempFile)
     })
   })
 
   describe('getFileStream', function() {
-    beforeEach(function() {
-      return (this.opts = {})
-    })
+    const filename = 'wombat/potato'
+    const filteredFilename = 'wombat_potato'
 
-    it('should use correct file location', function(done) {
-      this.FSPersistorManager.getFileStream(
-        this.location,
-        this.name1,
-        this.opts,
-        (err, res) => {}
-      )
-      this.Fs.open
-        .calledWith(`${this.location}/${this.name1Filtered}`)
-        .should.equal(true)
-      return done()
+    it('should use correct file location', async function() {
+      await FSPersistorManager.promises.getFileStream(location, filename, {})
+      expect(fs.open).to.have.been.calledWith(`${location}/${filteredFilename}`)
     })
 
-    describe('with start and end options', function() {
-      beforeEach(function() {
-        this.fd = 2019
-        this.opts_in = { start: 0, end: 8 }
-        this.opts = { start: 0, end: 8, fd: this.fd }
-        return this.Fs.open.callsArgWith(2, null, this.fd)
+    it('should pass the options to createReadStream', async function() {
+      await FSPersistorManager.promises.getFileStream(location, filename, {
+        start: 0,
+        end: 8
       })
-
-      return it('should pass the options to createReadStream', function(done) {
-        this.FSPersistorManager.getFileStream(
-          this.location,
-          this.name1,
-          this.opts_in,
-          (err, res) => {}
-        )
-        this.Fs.createReadStream.calledWith(null, this.opts).should.equal(true)
-        return done()
+      expect(fs.createReadStream).to.have.been.calledWith(null, {
+        start: 0,
+        end: 8,
+        fd
       })
     })
 
-    return describe('error conditions', function() {
-      describe('when the file does not exist', function() {
-        beforeEach(function() {
-          this.fakeCode = 'ENOENT'
-          const err = new Error()
-          err.code = this.fakeCode
-          return this.Fs.open.callsArgWith(2, err, null)
-        })
-
-        return it('should give a NotFoundError', function(done) {
-          return this.FSPersistorManager.getFileStream(
-            this.location,
-            this.name1,
-            this.opts,
-            (err, res) => {
-              expect(res).to.equal(null)
-              expect(err).to.not.equal(null)
-              expect(err instanceof this.Errors.NotFoundError).to.equal(true)
-              return done()
-            }
-          )
-        })
-      })
+    it('should give a NotFoundError if the file does not exist', async function() {
+      const err = new Error()
+      err.code = 'ENOENT'
+      fs.open.yields(err)
 
-      return describe('when some other error happens', function() {
-        beforeEach(function() {
-          this.fakeCode = 'SOMETHINGHORRIBLE'
-          const err = new Error()
-          err.code = this.fakeCode
-          return this.Fs.open.callsArgWith(2, err, null)
-        })
-
-        return it('should give an Error', function(done) {
-          return this.FSPersistorManager.getFileStream(
-            this.location,
-            this.name1,
-            this.opts,
-            (err, res) => {
-              expect(res).to.equal(null)
-              expect(err).to.not.equal(null)
-              expect(err instanceof Error).to.equal(true)
-              return done()
-            }
-          )
-        })
-      })
+      await expect(
+        FSPersistorManager.promises.getFileStream(location, filename, {})
+      )
+        .to.eventually.be.rejectedWith('file not found')
+        .and.be.an.instanceOf(Errors.NotFoundError)
+    })
+
+    it('should wrap any other error', async function() {
+      fs.open.yields(error)
+      await expect(
+        FSPersistorManager.promises.getFileStream(location, filename, {})
+      )
+        .to.eventually.be.rejectedWith('failed to open file for streaming')
+        .and.be.an.instanceOf(Errors.ReadError)
+        .and.have.property('cause', error)
     })
   })
 
   describe('getFileSize', function() {
-    it('should return the file size', function(done) {
-      const expectedFileSize = 75382
-      this.Fs.stat.yields(new Error('fs.stat got unexpected arguments'))
-      this.Fs.stat
-        .withArgs(`${this.location}/${this.name1Filtered}`)
-        .yields(null, { size: expectedFileSize })
-
-      return this.FSPersistorManager.getFileSize(
-        this.location,
-        this.name1,
-        (err, fileSize) => {
-          if (err != null) {
-            return done(err)
-          }
-          expect(fileSize).to.equal(expectedFileSize)
-          return done()
-        }
-      )
+    const filename = 'wombat/potato'
+    const badFilename = 'neenaw.tex'
+    const filteredFilename = 'wombat_potato'
+    const size = 65536
+    const noentError = new Error('not found')
+    noentError.code = 'ENOENT'
+
+    beforeEach(function() {
+      fs.stat
+        .yields(error)
+        .withArgs(`${location}/${filteredFilename}`)
+        .yields(null, { size })
+        .withArgs(`${location}/${badFilename}`)
+        .yields(noentError)
     })
 
-    it('should throw a NotFoundError if the file does not exist', function(done) {
-      const error = new Error()
-      error.code = 'ENOENT'
-      this.Fs.stat.yields(error)
-
-      return this.FSPersistorManager.getFileSize(
-        this.location,
-        this.name1,
-        (err, fileSize) => {
-          expect(err).to.be.instanceof(this.Errors.NotFoundError)
-          return done()
-        }
-      )
+    it('should return the file size', async function() {
+      expect(
+        await FSPersistorManager.promises.getFileSize(location, filename)
+      ).to.equal(size)
     })
 
-    return it('should rethrow any other error', function(done) {
-      const error = new Error()
-      this.Fs.stat.yields(error)
-
-      return this.FSPersistorManager.getFileSize(
-        this.location,
-        this.name1,
-        (err, fileSize) => {
-          expect(err).to.equal(error)
-          return done()
-        }
-      )
+    it('should throw a NotFoundError if the file does not exist', async function() {
+      await expect(
+        FSPersistorManager.promises.getFileSize(location, badFilename)
+      ).to.eventually.be.rejected.and.be.an.instanceOf(Errors.NotFoundError)
     })
-  })
 
-  describe('copyFile', function() {
-    beforeEach(function() {
-      this.ReadStream = {
-        on() {},
-        pipe: sinon.stub()
-      }
-      this.WriteStream = { on() {} }
-      this.Fs.createReadStream.returns(this.ReadStream)
-      return this.Fs.createWriteStream.returns(this.WriteStream)
+    it('should wrap any other error', async function() {
+      await expect(FSPersistorManager.promises.getFileSize(location, 'raccoon'))
+        .to.eventually.be.rejected.and.be.an.instanceOf(Errors.ReadError)
+        .and.have.property('cause', error)
     })
+  })
 
-    it('Should open the source for reading', function(done) {
-      this.FSPersistorManager.copyFile(
-        this.location,
-        this.name1,
-        this.name2,
-        function() {}
+  describe('copyFile', function() {
+    it('Should open the source for reading', async function() {
+      await FSPersistorManager.promises.copyFile(location, files[0], files[1])
+      expect(fs.createReadStream).to.have.been.calledWith(
+        `${location}/${files[0]}`
       )
-      this.Fs.createReadStream
-        .calledWith(`${this.location}/${this.name1Filtered}`)
-        .should.equal(true)
-      return done()
     })
 
-    it('Should open the target for writing', function(done) {
-      this.FSPersistorManager.copyFile(
-        this.location,
-        this.name1,
-        this.name2,
-        function() {}
+    it('Should open the target for writing', async function() {
+      await FSPersistorManager.promises.copyFile(location, files[0], files[1])
+      expect(fs.createWriteStream).to.have.been.calledWith(
+        `${location}/${files[1]}`
       )
-      this.Fs.createWriteStream
-        .calledWith(`${this.location}/${this.name2}`)
-        .should.equal(true)
-      return done()
     })
 
-    return it('Should pipe the source to the target', function(done) {
-      this.FSPersistorManager.copyFile(
-        this.location,
-        this.name1,
-        this.name2,
-        function() {}
-      )
-      this.ReadStream.pipe.calledWith(this.WriteStream).should.equal(true)
-      return done()
+    it('Should pipe the source to the target', async function() {
+      await FSPersistorManager.promises.copyFile(location, files[0], files[1])
+      expect(stream.pipeline).to.have.been.calledWith(readStream, writeStream)
     })
   })
 
   describe('deleteFile', function() {
-    beforeEach(function() {
-      return this.Fs.unlink.callsArgWith(1, this.error)
+    it('Should call unlink with correct options', async function() {
+      await FSPersistorManager.promises.deleteFile(location, files[0])
+      expect(fs.unlink).to.have.been.calledWith(`${location}/${files[0]}`)
     })
 
-    it('Should call unlink with correct options', function(done) {
-      return this.FSPersistorManager.deleteFile(
-        this.location,
-        this.name1,
-        err => {
-          this.Fs.unlink
-            .calledWith(`${this.location}/${this.name1Filtered}`)
-            .should.equal(true)
-          return done()
-        }
-      )
-    })
-
-    return it('Should propogate the error', function(done) {
-      return this.FSPersistorManager.deleteFile(
-        this.location,
-        this.name1,
-        err => {
-          err.should.equal(this.error)
-          return done()
-        }
-      )
+    it('Should propagate the error', async function() {
+      fs.unlink.yields(error)
+      await expect(
+        FSPersistorManager.promises.deleteFile(location, files[0])
+      ).to.eventually.be.rejectedWith(error)
     })
   })
 
   describe('deleteDirectory', function() {
-    beforeEach(function() {
-      return this.Rimraf.callsArgWith(1, this.error)
-    })
-
-    it('Should call rmdir(rimraf) with correct options', function(done) {
-      return this.FSPersistorManager.deleteDirectory(
-        this.location,
-        this.name1,
-        err => {
-          this.Rimraf.calledWith(
-            `${this.location}/${this.name1Filtered}`
-          ).should.equal(true)
-          return done()
-        }
-      )
+    it('Should call rmdir(rimraf) with correct options', async function() {
+      await FSPersistorManager.promises.deleteDirectory(location, files[0])
+      expect(rimraf).to.have.been.calledWith(`${location}/${files[0]}`)
     })
 
-    return it('Should propogate the error', function(done) {
-      return this.FSPersistorManager.deleteDirectory(
-        this.location,
-        this.name1,
-        err => {
-          err.should.equal(this.error)
-          return done()
-        }
-      )
+    it('Should propagate the error', async function() {
+      rimraf.yields(error)
+      await expect(
+        FSPersistorManager.promises.deleteDirectory(location, files[0])
+      ).to.eventually.be.rejectedWith(error)
     })
   })
 
   describe('checkIfFileExists', function() {
+    const filename = 'wombat'
+    const badFilename = 'potato'
+    const noentError = new Error('not found')
+    noentError.code = 'ENOENT'
+
     beforeEach(function() {
-      return this.Fs.exists.callsArgWith(1, true)
+      fs.stat
+        .yields(error)
+        .withArgs(`${location}/${filename}`)
+        .yields(null, {})
+        .withArgs(`${location}/${badFilename}`)
+        .yields(noentError)
     })
 
-    it('Should call exists with correct options', function(done) {
-      return this.FSPersistorManager.checkIfFileExists(
-        this.location,
-        this.name1,
-        exists => {
-          this.Fs.exists
-            .calledWith(`${this.location}/${this.name1Filtered}`)
-            .should.equal(true)
-          return done()
-        }
-      )
+    it('Should call stat with correct options', async function() {
+      await FSPersistorManager.promises.checkIfFileExists(location, filename)
+      expect(fs.stat).to.have.been.calledWith(`${location}/${filename}`)
     })
 
-    // fs.exists simply returns false on any error, so...
-    it('should not return an error', function(done) {
-      return this.FSPersistorManager.checkIfFileExists(
-        this.location,
-        this.name1,
-        (err, exists) => {
-          expect(err).to.be.null
-          return done()
-        }
-      )
+    it('Should return true for existing files', async function() {
+      expect(
+        await FSPersistorManager.promises.checkIfFileExists(location, filename)
+      ).to.equal(true)
     })
 
-    it('Should return true for existing files', function(done) {
-      this.Fs.exists.callsArgWith(1, true)
-      return this.FSPersistorManager.checkIfFileExists(
-        this.location,
-        this.name1,
-        (err, exists) => {
-          exists.should.be.true
-          return done()
-        }
-      )
+    it('Should return false for non-existing files', async function() {
+      expect(
+        await FSPersistorManager.promises.checkIfFileExists(
+          location,
+          badFilename
+        )
+      ).to.equal(false)
     })
 
-    return it('Should return false for non-existing files', function(done) {
-      this.Fs.exists.callsArgWith(1, false)
-      return this.FSPersistorManager.checkIfFileExists(
-        this.location,
-        this.name1,
-        (err, exists) => {
-          exists.should.be.false
-          return done()
-        }
+    it('should wrap the error if there is a problem', async function() {
+      await expect(
+        FSPersistorManager.promises.checkIfFileExists(location, 'llama')
       )
+        .to.eventually.be.rejected.and.be.an.instanceOf(Errors.ReadError)
+        .and.have.property('cause', error)
     })
   })
 
-  return describe('directorySize', function() {
-    it('should propogate the error', function(done) {
-      this.Fs.readdir.callsArgWith(1, this.error)
-      return this.FSPersistorManager.directorySize(
-        this.location,
-        this.name1,
-        (err, totalsize) => {
-          err.should.equal(this.error)
-          return done()
-        }
+  describe('directorySize', function() {
+    it('should wrap the error', async function() {
+      fs.readdir.yields(error)
+      await expect(
+        FSPersistorManager.promises.directorySize(location, 'wombat')
       )
+        .to.eventually.be.rejected.and.be.an.instanceOf(Errors.ReadError)
+        .and.include({ cause: error })
+        .and.have.property('info')
+        .which.includes({ location, name: 'wombat' })
     })
 
-    return it('should sum directory files size', function(done) {
-      this.Fs.readdir.callsArgWith(1, null, [
-        { file1: 'file1' },
-        { file2: 'file2' }
-      ])
-      this.Fs.fstatSync.returns({ size: 1024 })
-      return this.FSPersistorManager.directorySize(
-        this.location,
-        this.name1,
-        (err, totalsize) => {
-          expect(totalsize).to.equal(2048)
-          return done()
-        }
-      )
+    it('should sum directory files size', async function() {
+      expect(
+        await FSPersistorManager.promises.directorySize(location, 'wombat')
+      ).to.equal(stat.size * files.length)
     })
   })
 })