Merge branch 'develop'

Closes #523

docs/copy-sync.md

@@ -9,7 +9,7 @@ Copy a file or directory. The directory can have contents. Like `cp -r`.
   - `errorOnExist` `<boolean>`: when `overwrite` is `false` and the destination exists, throw an error. Default is `false`.
   - `dereference` `<boolean>`: dereference symlinks, default is `false`.
   - `preserveTimestamps` `<boolean>`: will set last modification and access times to the ones of the original source files, default is `false`.
-  - `filter` `<Function>`: Function to filter copied files. Return `true` to include, `false` to exclude. This can also be a RegExp, however this is deprecated (See [issue #239](https://github.com/jprichardson/node-fs-extra/issues/239) for background).
+  - `filter` `<Function>`: Function to filter copied files. Return `true` to include, `false` to exclude.
 
 ## Example:
 

docs/copy.md

@@ -9,7 +9,7 @@ Copy a file or directory. The directory can have contents. Like `cp -r`.
   - `errorOnExist` `<boolean>`: when `overwrite` is `false` and the destination exists, throw an error. Default is `false`.
   - `dereference` `<boolean>`: dereference symlinks, default is `false`.
   - `preserveTimestamps` `<boolean>`: will set last modification and access times to the ones of the original source files, default is `false`.
-  - `filter` `<Function>`: Function to filter copied files. Return `true` to include, `false` to exclude. This can also be a RegExp, however this is deprecated (See [issue #239](https://github.com/jprichardson/node-fs-extra/issues/239) for background).
+  - `filter` `<Function>`: Function to filter copied files. Return `true` to include, `false` to exclude. Can also return a `Promise` that resolves to `true` or `false` (or pass in an `async` function).
 - `callback` `<Function>`
 
 ## Example:

lib/copy-sync/__tests__/copy-sync-dir.test.js

@@ -21,6 +21,21 @@ describe('+ copySync()', () => {
   })
 
   describe('> when the source is a directory', () => {
+    describe('> when dest exists and is a file', () => {
+      it('should throw error', () => {
+        const src = path.join(TEST_DIR, 'src')
+        const dest = path.join(TEST_DIR, 'file.txt')
+        fs.mkdirSync(src)
+        fs.ensureFileSync(dest)
+
+        try {
+          fs.copySync(src, dest)
+        } catch (err) {
+          assert.strictEqual(err.message, `Cannot overwrite non-directory '${dest}' with directory '${src}'.`)
+        }
+      })
+    })
+
     it('should copy the directory synchronously', () => {
       const FILES = 2
 
@@ -89,6 +104,18 @@ describe('+ copySync()', () => {
   })
 
   describe('> when filter is used', () => {
+    it('should do nothing if filter fails', () => {
+      const srcDir = path.join(TEST_DIR, 'src')
+      const srcFile = path.join(srcDir, 'srcfile.css')
+      fs.outputFileSync(srcFile, 'src contents')
+      const destDir = path.join(TEST_DIR, 'dest')
+      const destFile = path.join(destDir, 'destfile.css')
+      const filter = s => path.extname(s) !== '.css' && !fs.statSync(s).isDirectory()
+
+      fs.copySync(srcFile, destFile, filter)
+      assert(!fs.existsSync(destDir))
+    })
+
     it('should should apply filter recursively', () => {
       const FILES = 2
       // Don't match anything that ends with a digit higher than 0:

lib/copy-sync/__tests__/copy-sync-preserve-time.test.js

@@ -1,66 +1,76 @@
 'use strict'
 
-const fs = require('fs')
+const fs = require(process.cwd())
 const os = require('os')
 const path = require('path')
 const utimes = require('../../util/utimes')
 const assert = require('assert')
-const copySync = require('../copy-sync')
+const nodeVersion = process.versions.node
+const nodeVersionMajor = parseInt(nodeVersion.split('.')[0], 10)
 
-/* global beforeEach, describe, it */
+/* global beforeEach, afterEach, describe, it */
 
 if (process.arch === 'ia32') console.warn('32 bit arch; skipping copySync timestamp tests')
+if (nodeVersionMajor < 8) console.warn(`old node version (v${nodeVersion}); skipping copySync timestamp tests`)
 
-const describeIf64 = process.arch === 'ia32' ? describe.skip : describe
+const describeIfPractical = (process.arch === 'ia32' || nodeVersionMajor < 8) ? describe.skip : describe
 
-describeIf64('copySync', () => {
-  let TEST_DIR
+describeIfPractical('copySync() - preserveTimestamps option', () => {
+  let TEST_DIR, src, dest
 
   beforeEach(done => {
     TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-preserve-time')
-    require(process.cwd()).emptyDir(TEST_DIR, done)
+    fs.emptyDir(TEST_DIR, done)
   })
 
-  describe('> modification option', () => {
-    const SRC_FIXTURES_DIR = path.join(__dirname, './fixtures')
-    const FILES = ['a-file', path.join('a-folder', 'another-file'), path.join('a-folder', 'another-folder', 'file3')]
+  afterEach(done => fs.remove(TEST_DIR, done))
 
-    describe('> when modified option is turned off', () => {
-      it('should have different timestamps on copy', () => {
-        const from = path.join(SRC_FIXTURES_DIR)
-        copySync(from, TEST_DIR, {preserveTimestamps: false})
+  const FILES = ['a-file', path.join('a-folder', 'another-file'), path.join('a-folder', 'another-folder', 'file3')]
+
+  describe('> when preserveTimestamps option is false', () => {
+    it('should have different timestamps on copy', done => {
+      src = path.join(TEST_DIR, 'src')
+      dest = path.join(TEST_DIR, 'dest')
+      FILES.forEach(f => fs.ensureFileSync(path.join(src, f)))
+
+      setTimeout(() => {
+        fs.copySync(src, dest, {preserveTimestamps: false})
         FILES.forEach(testFile({preserveTimestamps: false}))
-      })
+        done()
+      }, 100)
     })
+  })
 
-    describe('> when modified option is turned on', () => {
-      it('should have the same timestamps on copy', () => {
-        const from = path.join(SRC_FIXTURES_DIR)
-        copySync(from, TEST_DIR, {preserveTimestamps: true})
-        FILES.forEach(testFile({preserveTimestamps: true}))
-      })
+  describe('> when preserveTimestamps option is true', () => {
+    it('should have the same timestamps on copy', () => {
+      src = path.join(TEST_DIR, 'src')
+      dest = path.join(TEST_DIR, 'dest')
+      FILES.forEach(f => fs.ensureFileSync(path.join(src, f)))
+
+      fs.copySync(src, dest, {preserveTimestamps: true})
+      FILES.forEach(testFile({preserveTimestamps: true}))
     })
+  })
 
-    function testFile (options) {
-      return function (file) {
-        const a = path.join(SRC_FIXTURES_DIR, file)
-        const b = path.join(TEST_DIR, file)
-        const fromStat = fs.statSync(a)
-        const toStat = fs.statSync(b)
-        if (options.preserveTimestamps) {
-          // https://github.com/nodejs/io.js/issues/2069
-          if (process.platform !== 'win32') {
-            assert.strictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime())
-            assert.strictEqual(toStat.atime.getTime(), fromStat.atime.getTime())
-          } else {
-            assert.strictEqual(toStat.mtime.getTime(), utimes.timeRemoveMillis(fromStat.mtime.getTime()))
-            assert.strictEqual(toStat.atime.getTime(), utimes.timeRemoveMillis(fromStat.atime.getTime()))
-          }
+  function testFile (options) {
+    return function (file) {
+      const a = path.join(src, file)
+      const b = path.join(dest, file)
+      const fromStat = fs.statSync(a)
+      const toStat = fs.statSync(b)
+      if (options.preserveTimestamps) {
+        // https://github.com/nodejs/io.js/issues/2069
+        if (process.platform !== 'win32') {
+          assert.strictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime())
+          assert.strictEqual(toStat.atime.getTime(), fromStat.atime.getTime())
         } else {
-          assert.notEqual(toStat.mtime.getTime(), fromStat.mtime.getTime())
-          // the access time might actually be the same, so check only modification time
+          assert.strictEqual(toStat.mtime.getTime(), utimes.timeRemoveMillis(fromStat.mtime.getTime()))
+          assert.strictEqual(toStat.atime.getTime(), utimes.timeRemoveMillis(fromStat.atime.getTime()))
         }
+      } else {
+        assert.notEqual(toStat.mtime.getTime(), fromStat.mtime.getTime())
+        // the access time might actually be the same, so check only modification time
       }
     }
-  })
+  }
 })

lib/copy-sync/__tests__/copy-sync-prevent-copying-identical.test.js

@@ -0,0 +1,181 @@
+'use strict'
+
+const assert = require('assert')
+const os = require('os')
+const path = require('path')
+const fs = require(process.cwd())
+const klawSync = require('klaw-sync')
+
+/* global beforeEach, afterEach, describe, it */
+
+describe('+ copySync() - prevent copying identical files and dirs', () => {
+  let TEST_DIR = ''
+  let src = ''
+  let dest = ''
+
+  beforeEach(done => {
+    TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-prevent-copying-identical')
+    fs.emptyDir(TEST_DIR, done)
+  })
+
+  afterEach(done => fs.remove(TEST_DIR, done))
+
+  it('should return an error if src and dest are the same', () => {
+    const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync')
+    const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync')
+    try {
+      fs.copySync(fileSrc, fileDest)
+    } catch (err) {
+      assert.equal(err.message, 'Source and destination must not be the same.')
+    }
+  })
+
+  // src is directory:
+  //  src is regular, dest is symlink
+  //  src is symlink, dest is regular
+  //  src is symlink, dest is symlink
+
+  describe('> when the source is a directory', () => {
+    describe(`>> when src is regular and dest is a symlink that points to src`, () => {
+      it('should not copy and return', () => {
+        src = path.join(TEST_DIR, 'src')
+        fs.mkdirsSync(src)
+        const subdir = path.join(TEST_DIR, 'src', 'subdir')
+        fs.mkdirsSync(subdir)
+        fs.writeFileSync(path.join(subdir, 'file.txt'), 'some data')
+
+        const destLink = path.join(TEST_DIR, 'dest-symlink')
+        fs.symlinkSync(src, destLink, 'dir')
+
+        const oldlen = klawSync(src).length
+
+        fs.copySync(src, destLink)
+
+        const newlen = klawSync(src).length
+        assert.strictEqual(newlen, oldlen)
+        const link = fs.readlinkSync(destLink)
+        assert.strictEqual(link, src)
+      })
+    })
+
+    describe(`>> when src is a symlink that points to a regular dest`, () => {
+      it('should throw error', () => {
+        dest = path.join(TEST_DIR, 'dest')
+        fs.mkdirsSync(dest)
+        const subdir = path.join(TEST_DIR, 'dest', 'subdir')
+        fs.mkdirsSync(subdir)
+        fs.writeFileSync(path.join(subdir, 'file.txt'), 'some data')
+
+        const srcLink = path.join(TEST_DIR, 'src-symlink')
+        fs.symlinkSync(dest, srcLink, 'dir')
+
+        const oldlen = klawSync(dest).length
+
+        try {
+          fs.copySync(srcLink, dest)
+        } catch (err) {
+          assert(err)
+        }
+
+        // assert nothing copied
+        const newlen = klawSync(dest).length
+        assert.strictEqual(newlen, oldlen)
+        const link = fs.readlinkSync(srcLink)
+        assert.strictEqual(link, dest)
+      })
+    })
+
+    describe('>> when src and dest are symlinks that point to the exact same path', () => {
+      it('should not copy and return', () => {
+        src = path.join(TEST_DIR, 'src')
+        fs.mkdirsSync(src)
+        const srcLink = path.join(TEST_DIR, 'src_symlink')
+        fs.symlinkSync(src, srcLink, 'dir')
+        const destLink = path.join(TEST_DIR, 'dest_symlink')
+        fs.symlinkSync(src, destLink, 'dir')
+
+        const srclenBefore = klawSync(srcLink).length
+        const destlenBefore = klawSync(destLink).length
+
+        fs.copySync(srcLink, destLink)
+
+        const srclenAfter = klawSync(srcLink).length
+        assert.strictEqual(srclenAfter, srclenBefore, 'src length should not change')
+        const destlenAfter = klawSync(destLink).length
+        assert.strictEqual(destlenAfter, destlenBefore, 'dest length should not change')
+
+        const srcln = fs.readlinkSync(srcLink)
+        assert.strictEqual(srcln, src)
+        const destln = fs.readlinkSync(destLink)
+        assert.strictEqual(destln, src)
+      })
+    })
+  })
+
+  // src is file:
+  //  src is regular, dest is symlink
+  //  src is symlink, dest is regular
+  //  src is symlink, dest is symlink
+
+  describe('> when the source is a file', () => {
+    describe(`>> when src is regular and dest is a symlink that points to src`, () => {
+      it('should not copy and return', () => {
+        src = path.join(TEST_DIR, 'src', 'somefile.txt')
+        fs.ensureFileSync(src)
+        fs.writeFileSync(src, 'some data')
+
+        const destLink = path.join(TEST_DIR, 'dest-symlink')
+        fs.symlinkSync(src, destLink, 'file')
+
+        fs.copySync(src, destLink)
+
+        const link = fs.readlinkSync(destLink)
+        assert.strictEqual(link, src)
+        assert(fs.readFileSync(link, 'utf8'), 'some data')
+      })
+    })
+
+    describe(`>> when src is a symlink that points to a regular dest`, () => {
+      it('should throw error', () => {
+        dest = path.join(TEST_DIR, 'dest', 'somefile.txt')
+        fs.ensureFileSync(dest)
+        fs.writeFileSync(dest, 'some data')
+
+        const srcLink = path.join(TEST_DIR, 'src-symlink')
+        fs.symlinkSync(dest, srcLink, 'file')
+
+        try {
+          fs.copySync(srcLink, dest)
+        } catch (err) {
+          assert.ok(err)
+        }
+        const link = fs.readlinkSync(srcLink)
+        assert.strictEqual(link, dest)
+        assert(fs.readFileSync(link, 'utf8'), 'some data')
+      })
+    })
+
+    describe('>> when src and dest are symlinks that point to the exact same path', () => {
+      it('should not copy and return', () => {
+        src = path.join(TEST_DIR, 'src', 'srcfile.txt')
+        fs.ensureFileSync(src)
+        fs.writeFileSync(src, 'src data')
+
+        const srcLink = path.join(TEST_DIR, 'src_symlink')
+        fs.symlinkSync(src, srcLink, 'file')
+
+        const destLink = path.join(TEST_DIR, 'dest_symlink')
+        fs.symlinkSync(src, destLink, 'file')
+
+        fs.copySync(srcLink, destLink)
+
+        const srcln = fs.readlinkSync(srcLink)
+        assert.strictEqual(srcln, src)
+        const destln = fs.readlinkSync(destLink)
+        assert.strictEqual(destln, src)
+        assert(fs.readFileSync(srcln, 'utf8'), 'src data')
+        assert(fs.readFileSync(destln, 'utf8'), 'src data')
+      })
+    })
+  })
+})