deps: Stop overzealously replacing subdeps when updating

This fixes a regresion introduced by 971fd47

PR-URL: #10338
Credit/Blame: @iarna

lib/install/deps.js

@@ -28,8 +28,6 @@ var isInstallable = require('./validate-args.js').isInstallable
 var packageId = require('../utils/package-id.js')
 var moduleName = require('../utils/module-name.js')
 
-exports.test = {} // used to hold functions for testing by unit tests
-
 // The export functions in this module mutate a dependency tree, adding
 // items to them.
 
@@ -133,12 +131,13 @@ function addRequiredDep (tree, child) {
   return true
 }
 
+exports._removeObsoleteDep = removeObsoleteDep
 function removeObsoleteDep (child) {
   if (child.removed) return
   child.removed = true
   var requires = child.requires || []
   requires.forEach(function (requirement) {
-    requirement.requiredBy = requirement.requiredBy.filter(function (reqBy) { reqBy !== child })
+    requirement.requiredBy = requirement.requiredBy.filter(function (reqBy) { return reqBy !== child })
     if (requirement.requiredBy.length === 0) removeObsoleteDep(requirement)
   })
 }
@@ -416,13 +415,13 @@ function flatNameFromTree (tree) {
   return flatName(path, tree)
 }
 
-exports.test.replaceModuleName = replaceModuleName
+exports._replaceModuleName = replaceModuleName
 function replaceModuleName (obj, key, name) {
   validate('OSS', arguments)
   obj[key] = union(obj[key] || [], [name])
 }
 
-exports.test.replaceModule = replaceModule
+exports._replaceModule = replaceModule
 function replaceModule (obj, key, child) {
   validate('OSO', arguments)
   if (!obj[key]) obj[key] = []

test/tap/unit-deps-removeObsoleteDep.js

@@ -0,0 +1,41 @@
+'use strict'
+var test = require('tap').test
+var requireInject = require('require-inject')
+
+// we're just mocking to avoid having to call `npm.load`
+var deps = requireInject('../../lib/install/deps.js', {
+  '../../lib/npm.js': {
+    config: {
+      get: function () { return 'mock' }
+    }
+  }
+})
+
+var removeObsoleteDep = deps._removeObsoleteDep
+
+test('removeObsoleteDep', function (t) {
+  var child1 = {requiredBy: []}
+  var test1 = {
+    removed: true,
+    requires: [ child1 ]
+  }
+  removeObsoleteDep(test1)
+  t.is(child1.removed, undefined, 'no recursion on deps flagged as removed already')
+
+  var child2 = {requiredBy: []}
+  var test2 = {
+    requires: [ child2 ]
+  }
+  child2.requiredBy.push(test2)
+  removeObsoleteDep(test2)
+  t.is(child2.removed, true, 'required by no other modules, removing')
+
+  var child3 = {requiredBy: ['NOTEMPTY']}
+  var test3 = {
+    requires: [ child3 ]
+  }
+  child3.requiredBy.push(test3)
+  removeObsoleteDep(test3)
+  t.is(child3.removed, undefined, 'required by other modules, keeping')
+  t.done()
+})

test/tap/unit-deps-replaceModule.js

@@ -7,7 +7,7 @@ test('setup', function (t) {
 })
 
 test('replaceModule', function (t) {
-  var replaceModule = require('../../lib/install/deps').test.replaceModule
+  var replaceModule = require('../../lib/install/deps')._replaceModule
   var mods = []
   for (var ii = 0; ii < 10; ++ii) {
     mods.push({package: {name: ii}})
@@ -31,7 +31,7 @@ test('replaceModule', function (t) {
 })
 
 test('replaceModuleName', function (t) {
-  var replaceModuleName = require('../../lib/install/deps').test.replaceModuleName
+  var replaceModuleName = require('../../lib/install/deps')._replaceModuleName
   var mods = []
   for (var ii = 0; ii < 10; ++ii) {
     mods.push('pkg' + ii)