Skip to content

Commit

Permalink
Rewrite tests to not use /tmp and not require root
Browse files Browse the repository at this point in the history
Also improve test coverage for nearly full branch coverage.
  • Loading branch information
iarna committed Jan 6, 2017
1 parent e16442a commit 629265c
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 84 deletions.
14 changes: 5 additions & 9 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,9 @@ var chain = require('slide').chain
var MurmurHash3 = require('imurmurhash')
var extend = Object.assign || require('util')._extend

function murmurhex () {
var hash = new MurmurHash3()
for (var ii = 0; ii < arguments.length; ++ii) hash.hash('' + arguments[ii])
return hash.result()
}
var invocations = 0
var getTmpname = function (filename) {
return filename + '.' + murmurhex(__filename, process.pid, ++invocations)
return filename + '.' + MurmurHash3(__filename).hash(process.pid).hash(++invocations).result()
}

module.exports = function writeFile (filename, data, options, callback) {
Expand All @@ -28,11 +23,13 @@ module.exports = function writeFile (filename, data, options, callback) {
// Either mode or chown is not explicitly set
// Default behavior is to copy it from original file
return fs.stat(filename, function (err, stats) {
if (err || !stats) return thenWriteFile()

options = extend({}, options)
if (!err && stats && !options.mode) {
if (!options.mode) {
options.mode = stats.mode
}
if (!err && stats && !options.chown && process.getuid) {
if (!options.chown && process.getuid) {
options.chown = { uid: stats.uid, gid: stats.gid }
}
return thenWriteFile()
Expand Down Expand Up @@ -62,7 +59,6 @@ module.exports.sync = function writeFileSync (filename, data, options) {
// Default behavior is to copy it from original file
try {
var stats = fs.statSync(filename)

options = extend({}, options)
if (!options.mode) {
options.mode = stats.mode
Expand Down
9 changes: 5 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,16 @@
},
"homepage": "https://github.com/iarna/write-file-atomic",
"dependencies": {
"graceful-fs": "^4.1.2",
"graceful-fs": "^4.1.11",

This comment has been minimized.

Copy link
@valery-barysok

valery-barysok Jan 14, 2017

it leads to issue on win 10

npm/npm#12059 (comment)
valery-barysok/session-file-store#26 (my project)
and so on.

Can you lock version to 4.1.10? In other case i and others need to lock version of write-file-atomic to version before this change and lock it in my project to workaround issue.

"imurmurhash": "^0.1.4",
"slide": "^1.1.5"
},
"devDependencies": {
"require-inject": "^1.1.0",
"mkdirp": "^0.5.1",
"require-inject": "^1.4.0",
"rimraf": "^2.5.4",
"standard": "^5.4.1",
"tap": "^2.3.1",
"tmp": "0.0.28"
"tap": "^2.3.1"
},
"files": [
"index.js"
Expand Down
225 changes: 154 additions & 71 deletions test/integration.js
Original file line number Diff line number Diff line change
@@ -1,118 +1,201 @@
'use strict'
var fs = require('graceful-fs')
var path = require('path')
var test = require('tap').test
var writeFileAtomic = require('../index')
var fs = require('fs')
var tmp = require('tmp')

function tmpPath () {
// We need to manually set os temp dir because of:
// https://github.com/npm/npm/issues/4531#issuecomment-226294103
var dir = tmp.dirSync({ dir: '/tmp' }).name
return tmp.tmpNameSync({ dir: dir })
var mkdirp = require('mkdirp')
var rimraf = require('rimraf')
var requireInject = require('require-inject')

var workdir = path.join(__dirname, path.basename(__filename, '.js'))
var testfiles = 0
function tmpFile () {
return path.join(workdir, 'test-' + (++testfiles))
}

function readFile (path) {
return fs.readFileSync(path).toString()
}

function didWriteFileAtomic (t, expected, filename, data, options, callback) {
if (options instanceof Function) {
callback = options
options = null
}
if (!options) options = {}
var actual = {}
var writeFileAtomic = requireInject('../index.js', {
'graceful-fs': Object.assign({}, fs, {
chown: function mockChown (filename, uid, gid, cb) {
actual.uid = uid
actual.gid = gid
process.nextTick(cb)
},
stat: function mockStat (filename, cb) {
fs.stat(filename, function (err, stats) {
if (err) return cb(err)
cb(null, Object.assign(stats, expected || {}))
})
}
})
})
return writeFileAtomic(filename, data, options, function (err) {
t.isDeeply(actual, expected, 'ownership is as expected')
callback(err)
})
}

function didWriteFileAtomicSync (t, expected, filename, data, options) {
var actual = {}
var writeFileAtomic = requireInject('../index.js', {
'graceful-fs': Object.assign({}, fs, {
chownSync: function mockChownSync (filename, uid, gid) {
actual.uid = uid
actual.gid = gid
},
statSync: function mockStatSync (filename) {
var stats = fs.statSync(filename)
return Object.assign(stats, expected || {})
}
})
})
writeFileAtomic.sync(filename, data, options)
t.isDeeply(actual, expected)
}

function currentUser () {
return {
uid: process.getuid(),
gid: process.getgid()
}
}

test('setup', function (t) {
rimraf.sync(workdir)
mkdirp.sync(workdir)
t.done()
})

test('writes simple file (async)', function (t) {
t.plan(1)
var path = tmpPath()
writeFileAtomic(path, '42', function (err) {
if (err) throw err
t.is(readFile(path), '42')
t.plan(3)
var file = tmpFile()
didWriteFileAtomic(t, {}, file, '42', function (err) {
t.ifError(err, 'no error')
t.is(readFile(file), '42', 'content ok')
})
})

test('runs chown on given file (async)', function (t) {
t.plan(2)
var path = tmpPath()
writeFileAtomic(path, '42', { chown: { uid: 42, gid: 43 } }, function (err) {
if (err) throw err
var stat = fs.statSync(path)
t.is(stat.uid, 42)
t.is(stat.gid, 43)
var file = tmpFile()
didWriteFileAtomic(t, {uid: 42, gid: 43}, file, '42', { chown: { uid: 42, gid: 43 } }, function (err) {
t.ifError(err, 'no error')
t.is(readFile(file), '42', 'content ok')
t.done()
})
})

test('runs chmod on given file (async)', function (t) {
t.plan(1)
var path = tmpPath()
writeFileAtomic(path, '42', { mode: parseInt('741', 8) }, function (err) {
if (err) throw err
var stat = fs.statSync(path)
t.plan(3)
var file = tmpFile()
didWriteFileAtomic(t, {}, file, '42', { mode: parseInt('741', 8) }, function (err) {
t.ifError(err, 'no error')
var stat = fs.statSync(file)
t.is(stat.mode, parseInt('100741', 8))
})
})

test('run chmod AND chown (async)', function (t) {
t.plan(3)
var file = tmpFile()
didWriteFileAtomic(t, {uid: 42, gid: 43}, file, '42', { mode: parseInt('741', 8), chown: {uid: 42, gid: 43} }, function (err) {
t.ifError(err, 'no error')
var stat = fs.statSync(file)
t.is(stat.mode, parseInt('100741', 8))
})
})

test('does not change chmod by default (async)', function (t) {
t.plan(1)
var path = tmpPath()
writeFileAtomic(path, '42', { mode: parseInt('741', 8) }, function (err) {
if (err) throw err
t.plan(5)
var file = tmpFile()
didWriteFileAtomic(t, {}, file, '42', { mode: parseInt('741', 8) }, function (err) {
t.ifError(err, 'no error')

writeFileAtomic(path, '43', function (err) {
if (err) throw err
var stat = fs.statSync(path)
didWriteFileAtomic(t, currentUser(), file, '43', function (err) {
t.ifError(err, 'no error')
var stat = fs.statSync(file)
t.is(stat.mode, parseInt('100741', 8))
})
})
})

test('does not change chown by default (async)', function (t) {
t.plan(2)
var path = tmpPath()
writeFileAtomic(path, '42', { chown: { uid: 42, gid: 43 } }, function (err) {
if (err) throw err

writeFileAtomic(path, '43', function (err) {
if (err) throw err
var stat = fs.statSync(path)
t.is(stat.uid, 42)
t.is(stat.gid, 43)
})
})
t.plan(6)
var file = tmpFile()
didWriteFileAtomic(t, {uid: 42, gid: 43}, file, '42', { chown: { uid: 42, gid: 43 } }, _setModeOnly)

function _setModeOnly (err) {
t.ifError(err, 'no error')

didWriteFileAtomic(t, {uid: 42, gid: 43}, file, '43', { mode: parseInt('741', 8) }, _allDefault)
}

function _allDefault (err) {
t.ifError(err, 'no error')

didWriteFileAtomic(t, {uid: 42, gid: 43}, file, '43', _noError)
}

function _noError (err) {
t.ifError(err, 'no error')
}
})

test('writes simple file (sync)', function (t) {
t.plan(1)
var path = tmpPath()
writeFileAtomic.sync(path, '42')
t.is(readFile(path), '42')
t.plan(2)
var file = tmpFile()
didWriteFileAtomicSync(t, {}, file, '42')
t.is(readFile(file), '42')
})

test('runs chown on given file (sync)', function (t) {
t.plan(2)
var path = tmpPath()
writeFileAtomic.sync(path, '42', { chown: { uid: 42, gid: 43 } })
var stat = fs.statSync(path)
t.is(stat.uid, 42)
t.is(stat.gid, 43)
t.plan(1)
var file = tmpFile()
didWriteFileAtomicSync(t, {uid: 42, gid: 43}, file, '42', { chown: { uid: 42, gid: 43 } })
})

test('runs chmod on given file (sync)', function (t) {
t.plan(1)
var path = tmpPath()
writeFileAtomic.sync(path, '42', { mode: parseInt('741', 8) })
var stat = fs.statSync(path)
t.plan(2)
var file = tmpFile()
didWriteFileAtomicSync(t, {}, file, '42', { mode: parseInt('741', 8) })
var stat = fs.statSync(file)
t.is(stat.mode, parseInt('100741', 8))
})

test('runs chown and chmod (sync)', function (t) {
t.plan(2)
var file = tmpFile()
didWriteFileAtomicSync(t, {uid: 42, gid: 43}, file, '42', { mode: parseInt('741', 8), chown: { uid: 42, gid: 43 } })
var stat = fs.statSync(file)
t.is(stat.mode, parseInt('100741', 8))
})

test('does not change chmod by default (sync)', function (t) {
t.plan(1)
var path = tmpPath()
writeFileAtomic.sync(path, '42', { mode: parseInt('741', 8) })
writeFileAtomic.sync(path, '43')
var stat = fs.statSync(path)
t.plan(3)
var file = tmpFile()
didWriteFileAtomicSync(t, {}, file, '42', { mode: parseInt('741', 8) })
didWriteFileAtomicSync(t, currentUser(), file, '43')
var stat = fs.statSync(file)
t.is(stat.mode, parseInt('100741', 8))
})

test('does not change chown by default (sync)', function (t) {
t.plan(2)
var path = tmpPath()
writeFileAtomic.sync(path, '42', { chown: { uid: 42, gid: 43 } })
writeFileAtomic.sync(path, '43')
var stat = fs.statSync(path)
t.is(stat.uid, 42)
t.is(stat.gid, 43)
t.plan(3)
var file = tmpFile()
didWriteFileAtomicSync(t, {uid: 42, gid: 43}, file, '42', { chown: { uid: 42, gid: 43 } })
didWriteFileAtomicSync(t, {uid: 42, gid: 43}, file, '43', { mode: parseInt('741', 8) })
didWriteFileAtomicSync(t, {uid: 42, gid: 43}, file, '44')
})

test('cleanup', function (t) {
rimraf.sync(workdir)
t.done()
})

0 comments on commit 629265c

Please sign in to comment.