Full tests, handle errors properly in many cases

chownr.js

@@ -7,6 +7,7 @@ const LCHOWN = fs.lchown ? 'lchown' : 'chown'
 /* istanbul ignore next */
 const LCHOWNSYNC = fs.lchownSync ? 'lchownSync' : 'chownSync'
 
+/* istanbul ignore next */
 const needEISDIRHandled = fs.lchown &&
  !process.version.match(/v1[1-9]+\./) &&
  !process.version.match(/v10\.[6-9]/)
@@ -20,6 +21,7 @@ const lchownSync = (path, uid, gid) => {
  }
 }
 
+/* istanbul ignore next */
 const chownSync = (path, uid, gid) => {
  try {
  return fs.chownSync(path, uid, gid)
@@ -66,18 +68,16 @@ if (/^v4\./.test(nodeVersion))
 const chown = (cpath, uid, gid, cb) => {
  fs[LCHOWN](cpath, uid, gid, handleEISDIR(cpath, uid, gid, er => {
  // Skip ENOENT error
- if (er && er.code === 'ENOENT') return cb()
- cb(er)
+ cb(er && er.code !== 'ENOENT' ? er : null)
  }))
 }
 
 const chownrKid = (p, child, uid, gid, cb) => {
  if (typeof child === 'string')
  return fs.lstat(path.resolve(p, child), (er, stats) => {
  // Skip ENOENT error
- if (er && er.code === 'ENOENT') return cb()
  if (er)
- return cb(er)
+ return cb(er.code !== 'ENOENT' ? er : null)
  stats.name = child
  chownrKid(p, stats, uid, gid, cb)
  })
@@ -100,10 +100,12 @@ const chownr = (p, uid, gid, cb) => {
  readdir(p, { withFileTypes: true }, (er, children) => {
  // any error other than ENOTDIR or ENOTSUP means it's not readable,
  // or doesn't exist. give up.
- if (er && er.code !== 'ENOTDIR' && er.code !== 'ENOTSUP')
- return cb(er)
- if (er && er.code === 'ENOENT')
- return cb()
+ if (er) {
+ if (er.code === 'ENOENT')
+ return cb()
+ else if (er.code !== 'ENOTDIR' && er.code !== 'ENOTSUP')
+ return cb(er)
+ }
  if (er || !children.length)
  return chown(p, uid, gid, cb)
 
@@ -129,8 +131,10 @@ const chownrKidSync = (p, child, uid, gid) => {
  stats.name = child
  child = stats
  } catch (er) {
- if (er.code === 'ENOENT') return
- throw er;
+ if (er.code === 'ENOENT')
+ return
+ else
+ throw er
  }
  }
 
@@ -145,13 +149,15 @@ const chownrSync = (p, uid, gid) => {
  try {
  children = readdirSync(p, { withFileTypes: true })
  } catch (er) {
- if (er && er.code === 'ENOTDIR' && er.code !== 'ENOTSUP')
+ if (er.code === 'ENOENT')
+ return
+ else if (er.code === 'ENOTDIR' || er.code === 'ENOTSUP')
  return handleEISDirSync(p, uid, gid)
- if (er && er.code === 'ENOENT') return
- throw er
+ else
+  throw er
  }
 
- if (children.length)
+ if (children && children.length)
  children.forEach(child => chownrKidSync(p, child, uid, gid))
 
  return handleEISDirSync(p, uid, gid)

package.json

@@ -16,8 +16,11 @@
  "rimraf": "^2.7.1",
  "tap": "^14.10.6"
  },
+ "tap": {
+ "check-coverage": true
+ },
  "scripts": {
- "test": "tap test/*.js --cov",
+ "test": "tap",
  "preversion": "npm test",
  "postversion": "npm publish",
  "postpublish": "git push origin --follow-tags"

test/basic.js

@@ -2,84 +2,72 @@ if (!process.getuid || !process.getgid) {
  throw new Error("Tests require getuid/getgid support")
 }
 
-var curUid = +process.getuid()
-, curGid = +process.getgid()
-, chownr = require("../")
-, test = require("tap").test
-, mkdirp = require("mkdirp")
-, rimraf = require("rimraf")
-, fs = require("fs")
+const curUid = +process.getuid()
+const curGid = +process.getgid()
+const chownr = require("../")
+const t = require("tap")
+const mkdirp = require("mkdirp")
+const rimraf = require("rimraf")
+const fs = require("fs")
 
 // sniff the 'id' command for other groups that i can legally assign to
-var exec = require("child_process").exec
-, groups
-, dirs = []
+const {exec} = require("child_process")
+let groups
+let dirs = []
 
-exec("id", function (code, output) {
- if (code) throw new Error("failed to run 'id' command")
- groups = output.trim().split("=")[3].split(",").map(function (s) {
- return parseInt(s, 10)
- }).filter(function (g) {
- return g !== curGid
- })
-
- // console.error([curUid, groups[0]], "uid, gid")
-
- rimraf("/tmp/chownr", function (er) {
- if (er) throw er
- var cnt = 5
- for (var i = 0; i < 5; i ++) {
- mkdirp(getDir(), then)
- }
- function then (er) {
- if (er) throw er
- if (-- cnt === 0) {
- runTest()
- }
- }
+t.test('get the ids to use', { bail: true }, t => {
+ exec("id", function (code, output) {
+ if (code) throw new Error("failed to run 'id' command")
+ groups = output.trim().split("=")[3].split(",")
+ .map(s => parseInt(s, 10))
+ .filter(g => g !== curGid)
+ t.end()
  })
 })
 
-function getDir () {
- var dir = "/tmp/chownr"
-
- dir += "/" + Math.floor(Math.random() * Math.pow(16,4)).toString(16)
- dirs.push(dir)
- dir += "/" + Math.floor(Math.random() * Math.pow(16,4)).toString(16)
- dirs.push(dir)
- dir += "/" + Math.floor(Math.random() * Math.pow(16,4)).toString(16)
- dirs.push(dir)
- return dir
-}
+t.test('run test', t => {
+ const dir = t.testdir({
+ a: { b: { c: {}}},
+ d: { e: { f: {}}},
+ g: { h: { i: {}}},
+ j: { k: { l: {}}},
+ m: { n: { o: {}}},
+ })
 
-function runTest () {
- test("should complete successfully", function (t) {
+ t.test("should complete successfully", t => {
  // console.error("calling chownr", curUid, groups[0], typeof curUid, typeof groups[0])
- chownr("/tmp/chownr", curUid, groups[0], function (er) {
- t.ifError(er)
+ chownr(dir, curUid, groups[0], er => {
+ if (er)
+ throw er
  t.end()
  })
  })
 
- dirs.forEach(function (dir) {
- test("verify "+dir, function (t) {
- fs.stat(dir, function (er, st) {
- if (er) {
- t.ifError(er)
- return t.end()
- }
- t.equal(st.uid, curUid, "uid should be " + curUid)
- t.equal(st.gid, groups[0], "gid should be "+groups[0])
- t.end()
- })
- })
- })
+ const dirs = [
+ '',
+ 'a',
+ 'a/b',
+ 'a/b/c',
+ 'd',
+ 'd/e',
+ 'd/e/f',
+ 'g',
+ 'g/h',
+ 'g/h/i',
+ 'j',
+ 'j/k',
+ 'j/k/l',
+ 'm',
+ 'm/n',
+ 'm/n/o',
+ ]
 
- test("cleanup", function (t) {
- rimraf("/tmp/chownr/", function (er) {
- t.ifError(er)
- t.end()
+ dirs.forEach(d => t.test(`verify ${d}`, t => {
+ t.match(fs.statSync(`${dir}/${d}`), {
+ uid: curUid,
+ gid: groups[0],
  })
- })
-}
-
+ t.end()
+ }))
+ t.end()
+})