Full tests, handle errors properly in many cases

Author: isaacs

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()
+})