From 56be88fd65e35eaaa4f9a88a8f3a1cda9addf3d8 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Sat, 1 May 2021 12:14:57 -0700 Subject: [PATCH 1/6] move: do not create parent directory if it is root --- lib/move-sync/__tests__/move-sync.test.js | 21 +++++++++++++- lib/move-sync/move-sync.js | 8 +++++- lib/move/__tests__/move.test.js | 35 +++++++++++++++++------ lib/move/move.js | 7 +++++ 4 files changed, 61 insertions(+), 10 deletions(-) diff --git a/lib/move-sync/__tests__/move-sync.test.js b/lib/move-sync/__tests__/move-sync.test.js index dca83aaba..59fe82f06 100644 --- a/lib/move-sync/__tests__/move-sync.test.js +++ b/lib/move-sync/__tests__/move-sync.test.js @@ -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() @@ -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', () => { @@ -268,6 +270,23 @@ describe('moveSync()', () => { }) }) + describeIfWindows('> when dest parent is root', () => { + it('should not create parent directory', done => { + const src = path.join(TEST_DIR, 'a-file') + const 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('> when actually trying to move a folder across devices', () => { const differentDevice = '/mnt' let __skipTests = false diff --git a/lib/move-sync/move-sync.js b/lib/move-sync/move-sync.js index 7e13587af..1c36799a0 100644 --- a/lib/move-sync/move-sync.js +++ b/lib/move-sync/move-sync.js @@ -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) { diff --git a/lib/move/__tests__/move.test.js b/lib/move/__tests__/move.test.js index af41b93f8..169749625 100644 --- a/lib/move/__tests__/move.test.js +++ b/lib/move/__tests__/move.test.js @@ -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++ @@ -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() }) }) @@ -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() }) @@ -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() }) }) @@ -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() }) }) @@ -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() }) @@ -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() }) }) @@ -263,7 +265,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() }) @@ -271,6 +273,23 @@ describe('+ move()', () => { }) }) + describeIfWindows('> when dest parent is root', () => { + it('should not create parent directory', done => { + const src = path.join(TEST_DIR, 'a-file') + const 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') @@ -284,7 +303,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() }) }) diff --git a/lib/move/move.js b/lib/move/move.js index 23840e027..e9523e65a 100644 --- a/lib/move/move.js +++ b/lib/move/move.js @@ -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) @@ -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) { From 58f4021f851cbf9e2941dbd235beafe577a4b96f Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Sat, 1 May 2021 12:26:01 -0700 Subject: [PATCH 2/6] fix move-sync test --- lib/move-sync/__tests__/move-sync.test.js | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/lib/move-sync/__tests__/move-sync.test.js b/lib/move-sync/__tests__/move-sync.test.js index 59fe82f06..4b0eaf254 100644 --- a/lib/move-sync/__tests__/move-sync.test.js +++ b/lib/move-sync/__tests__/move-sync.test.js @@ -271,19 +271,15 @@ describe('moveSync()', () => { }) describeIfWindows('> when dest parent is root', () => { - it('should not create parent directory', done => { + it('should not create parent directory', () => { const src = path.join(TEST_DIR, 'a-file') const 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() - }) - }) + fse.moveSync(src, dest) + + const contents = fs.readFileSync(dest, 'utf8') + const expected = /^sonic the hedgehog\r?\n$/ + assert(contents.match(expected)) }) }) From 78d2bfe41098a5afafbe8cab3a351f4ad210a5e8 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Sat, 1 May 2021 12:35:39 -0700 Subject: [PATCH 3/6] move-sync test: remove dest after test is done --- lib/move-sync/__tests__/move-sync.test.js | 6 ++++++ lib/move/__tests__/move.test.js | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/move-sync/__tests__/move-sync.test.js b/lib/move-sync/__tests__/move-sync.test.js index 4b0eaf254..fb3c7bde5 100644 --- a/lib/move-sync/__tests__/move-sync.test.js +++ b/lib/move-sync/__tests__/move-sync.test.js @@ -275,11 +275,17 @@ describe('moveSync()', () => { const src = path.join(TEST_DIR, 'a-file') const dest = path.join(path.parse(TEST_DIR).root, 'another-file') + console.log('debug: dest ==>', dest) + fse.moveSync(src, dest) const contents = fs.readFileSync(dest, 'utf8') const expected = /^sonic the hedgehog\r?\n$/ assert(contents.match(expected)) + // this test is a bit special since + // we're moving to root, so we need to + // manually remove dest after we're done. + fse.removeSync(dest) }) }) diff --git a/lib/move/__tests__/move.test.js b/lib/move/__tests__/move.test.js index 169749625..c8d6a46fc 100644 --- a/lib/move/__tests__/move.test.js +++ b/lib/move/__tests__/move.test.js @@ -284,7 +284,10 @@ describe('+ move()', () => { const expected = /^sonic the hedgehog\r?\n$/ assert.ifError(err) assert.ok(contents.match(expected)) - done() + // this test is a bit special since + // we're moving to root, so we need to + // manually remove dest after we're done. + fse.remove(dest, done) }) }) }) From 30502109a2f6ff2d87227987307b191f473eabbf Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Sat, 1 May 2021 12:41:50 -0700 Subject: [PATCH 4/6] remove debug log --- lib/move-sync/__tests__/move-sync.test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/move-sync/__tests__/move-sync.test.js b/lib/move-sync/__tests__/move-sync.test.js index fb3c7bde5..91d9e418e 100644 --- a/lib/move-sync/__tests__/move-sync.test.js +++ b/lib/move-sync/__tests__/move-sync.test.js @@ -275,8 +275,6 @@ describe('moveSync()', () => { const src = path.join(TEST_DIR, 'a-file') const dest = path.join(path.parse(TEST_DIR).root, 'another-file') - console.log('debug: dest ==>', dest) - fse.moveSync(src, dest) const contents = fs.readFileSync(dest, 'utf8') From d42f3507f18948fbe286872e1556e72e7bb54609 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Sat, 1 May 2021 16:43:51 -0700 Subject: [PATCH 5/6] remove dest in afterEach --- lib/move-sync/__tests__/move-sync.test.js | 10 +++++----- lib/move/__tests__/move.test.js | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/move-sync/__tests__/move-sync.test.js b/lib/move-sync/__tests__/move-sync.test.js index 91d9e418e..506b62ddb 100644 --- a/lib/move-sync/__tests__/move-sync.test.js +++ b/lib/move-sync/__tests__/move-sync.test.js @@ -271,19 +271,19 @@ 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') - const dest = path.join(path.parse(TEST_DIR).root, 'another-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)) - // this test is a bit special since - // we're moving to root, so we need to - // manually remove dest after we're done. - fse.removeSync(dest) }) }) diff --git a/lib/move/__tests__/move.test.js b/lib/move/__tests__/move.test.js index c8d6a46fc..6de530155 100644 --- a/lib/move/__tests__/move.test.js +++ b/lib/move/__tests__/move.test.js @@ -274,9 +274,13 @@ describe('+ move()', () => { }) 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') - const dest = path.join(path.parse(TEST_DIR).root, 'another-file') + dest = path.join(path.parse(TEST_DIR).root, 'another-file') fse.move(src, dest, err => { assert.ifError(err) @@ -284,10 +288,6 @@ describe('+ move()', () => { const expected = /^sonic the hedgehog\r?\n$/ assert.ifError(err) assert.ok(contents.match(expected)) - // this test is a bit special since - // we're moving to root, so we need to - // manually remove dest after we're done. - fse.remove(dest, done) }) }) }) From 060cd87c0a3eefb20017950fa0d9d32be077b571 Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Sat, 1 May 2021 16:48:13 -0700 Subject: [PATCH 6/6] call done() after test is done --- lib/move/__tests__/move.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/move/__tests__/move.test.js b/lib/move/__tests__/move.test.js index 6de530155..d20f530d4 100644 --- a/lib/move/__tests__/move.test.js +++ b/lib/move/__tests__/move.test.js @@ -288,6 +288,7 @@ describe('+ move()', () => { const expected = /^sonic the hedgehog\r?\n$/ assert.ifError(err) assert.ok(contents.match(expected)) + done() }) }) })