Skip to content

Commit

Permalink
move: do not create parent directory if it is root (#897)
Browse files Browse the repository at this point in the history 
* move: do not create parent directory if it is root

* fix move-sync test

* move-sync test: remove dest after test is done

* remove debug log

* remove dest in afterEach

* call done() after test is done
  • Loading branch information
@manidlou
manidlou committed May 3, 2021
1 parent e6f8cb4 commit 289d9aa
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 10 deletions.
21 changes: 20 additions & 1 deletion lib/move-sync/__tests__/move-sync.test.js
View file
Expand Up @@ -10,6 +10,8 @@ const assert = require('assert')

/* global afterEach, beforeEach, describe, it */

const describeIfWindows = process.platform === 'win32' ? describe : describe.skip

function createSyncErrFn (errCode) {
const fn = function () {
const err = new Error()
Expand Down Expand Up @@ -120,7 +122,7 @@ describe('moveSync()', () => {

const contents = fs.readFileSync(dest, 'utf8')
const expected = /^sonic the hedgehog\r?\n$/
assert.ok(contents.match(expected), `${contents} match ${expected}`)
assert.ok(contents.match(expected))
})

it('should overwrite the destination directory if overwrite = true', () => {
Expand Down Expand Up @@ -268,6 +270,23 @@ describe('moveSync()', () => {
})
})

describeIfWindows('> when dest parent is root', () => {
let dest

afterEach(() => fse.removeSync(dest))

it('should not create parent directory', () => {
const src = path.join(TEST_DIR, 'a-file')
dest = path.join(path.parse(TEST_DIR).root, 'another-file')

fse.moveSync(src, dest)

const contents = fs.readFileSync(dest, 'utf8')
const expected = /^sonic the hedgehog\r?\n$/
assert(contents.match(expected))
})
})

describe('> when actually trying to move a folder across devices', () => {
const differentDevice = '/mnt'
let __skipTests = false
Expand Down
8 changes: 7 additions & 1 deletion lib/move-sync/move-sync.js
View file
Expand Up @@ -13,10 +13,16 @@ function moveSync (src, dest, opts) {

const { srcStat, isChangingCase = false } = stat.checkPathsSync(src, dest, 'move', opts)
stat.checkParentPathsSync(src, srcStat, dest, 'move')
mkdirpSync(path.dirname(dest))
if (!isParentRoot(dest)) mkdirpSync(path.dirname(dest))
return doRename(src, dest, overwrite, isChangingCase)
}

function isParentRoot (dest) {
const parent = path.dirname(dest)
const parsedPath = path.parse(parent)
return parsedPath.root === parent
}

function doRename (src, dest, overwrite, isChangingCase) {
if (isChangingCase) return rename(src, dest, overwrite)
if (overwrite) {
Expand Down
39 changes: 31 additions & 8 deletions lib/move/__tests__/move.test.js
View file
Expand Up @@ -8,6 +8,8 @@ const assert = require('assert')

/* global afterEach, beforeEach, describe, it */

const describeIfWindows = process.platform === 'win32' ? describe : describe.skip

function createAsyncErrFn (errCode) {
const fn = function (...args) {
fn.callCount++
Expand Down Expand Up @@ -63,7 +65,7 @@ describe('+ move()', () => {
fs.readFile(dest, 'utf8', (err, contents) => {
const expected = /^sonic the hedgehog\r?\n$/
assert.ifError(err)
assert.ok(contents.match(expected), `${contents} match ${expected}`)
assert.ok(contents.match(expected))
done()
})
})
Expand Down Expand Up @@ -114,7 +116,7 @@ describe('+ move()', () => {
fs.readFile(path.join(dest, 'another-folder', 'file3'), 'utf8', (err, contents) => {
const expected = /^knuckles\r?\n$/
assert.ifError(err)
assert.ok(contents.match(expected), `${contents} match ${expected}`)
assert.ok(contents.match(expected))
tearDownMockFs()
done()
})
Expand All @@ -132,7 +134,7 @@ describe('+ move()', () => {
fs.readFile(dest, 'utf8', (err, contents) => {
const expected = /^sonic the hedgehog\r?\n$/
assert.ifError(err)
assert.ok(contents.match(expected), `${contents} match ${expected}`)
assert.ok(contents.match(expected))
done()
})
})
Expand Down Expand Up @@ -206,7 +208,7 @@ describe('+ move()', () => {
fs.readFile(dest, 'utf8', (err, contents) => {
const expected = /^sonic the hedgehog\r?\n$/
assert.ifError(err)
assert.ok(contents.match(expected), `${contents} match ${expected}`)
assert.ok(contents.match(expected))
done()
})
})
Expand All @@ -225,7 +227,7 @@ describe('+ move()', () => {
fs.readFile(dest, 'utf8', (err, contents) => {
const expected = /^sonic the hedgehog\r?\n$/
assert.ifError(err)
assert.ok(contents.match(expected), `${contents} match ${expected}`)
assert.ok(contents.match(expected))
tearDownMockFs()
done()
})
Expand All @@ -244,7 +246,7 @@ describe('+ move()', () => {
fs.readFile(path.join(dest, 'another-file'), 'utf8', (err, contents) => {
const expected = /^tails\r?\n$/
assert.ifError(err)
assert.ok(contents.match(expected), `${contents} match ${expected}`)
assert.ok(contents.match(expected))
done()
})
})
Expand All @@ -263,14 +265,35 @@ describe('+ move()', () => {
fs.readFile(path.join(dest, 'another-folder', 'file3'), 'utf8', (err, contents) => {
const expected = /^knuckles\r?\n$/
assert.ifError(err)
assert.ok(contents.match(expected), `${contents} match ${expected}`)
assert.ok(contents.match(expected))
tearDownMockFs()
done()
})
})
})
})

describeIfWindows('> when dest parent is root', () => {
let dest

afterEach(done => fse.remove(dest, done))

it('should not create parent directory', done => {
const src = path.join(TEST_DIR, 'a-file')
dest = path.join(path.parse(TEST_DIR).root, 'another-file')

fse.move(src, dest, err => {
assert.ifError(err)
fs.readFile(dest, 'utf8', (err, contents) => {
const expected = /^sonic the hedgehog\r?\n$/
assert.ifError(err)
assert.ok(contents.match(expected))
done()
})
})
})
})

describe('> clobber', () => {
it('should be an alias for overwrite', done => {
const src = path.join(TEST_DIR, 'a-file')
Expand All @@ -284,7 +307,7 @@ describe('+ move()', () => {
fs.readFile(dest, 'utf8', (err, contents) => {
const expected = /^sonic the hedgehog\r?\n$/
assert.ifError(err)
assert.ok(contents.match(expected), `${contents} match ${expected}`)
assert.ok(contents.match(expected))
done()
})
})
Expand Down
7 changes: 7 additions & 0 deletions lib/move/move.js
View file
Expand Up @@ -21,6 +21,7 @@ function move (src, dest, opts, cb) {
const { srcStat, isChangingCase = false } = stats
stat.checkParentPaths(src, srcStat, dest, 'move', err => {
if (err) return cb(err)
if (isParentRoot(dest)) return doRename(src, dest, overwrite, isChangingCase, cb)
mkdirp(path.dirname(dest), err => {
if (err) return cb(err)
return doRename(src, dest, overwrite, isChangingCase, cb)
Expand All @@ -29,6 +30,12 @@ function move (src, dest, opts, cb) {
})
}

function isParentRoot (dest) {
const parent = path.dirname(dest)
const parsedPath = path.parse(parent)
return parsedPath.root === parent
}

function doRename (src, dest, overwrite, isChangingCase, cb) {
if (isChangingCase) return rename(src, dest, overwrite, cb)
if (overwrite) {
Expand Down

0 comments on commit 289d9aa

Please sign in to comment.