chore: test consolidation

workspaces/arborist/lib/arborist/index.js

@@ -3,28 +3,25 @@
 // - virtual
 // - ideal
 //
-// The actual tree is what's present on disk in the node_modules tree
-// and elsewhere that links may extend.
+// The actual tree is what's present on disk in the node_modules tree and elsewhere that links may extend.
 //
 // The virtual tree is loaded from metadata (package.json and lock files).
 //
-// The ideal tree is what we WANT that actual tree to become.  This starts
-// with the virtual tree, and then applies the options requesting
-// add/remove/update actions.
+// The ideal tree is what we WANT that actual tree to become.  This starts with the virtual tree, and then applies the options requesting add/remove/update actions.
 //
-// To reify a tree, we calculate a diff between the ideal and actual trees,
-// and then turn the actual tree into the ideal tree by taking the actions
-// required.  At the end of the reification process, the actualTree is
-// updated to reflect the changes.
+// To reify a tree, we calculate a diff between the ideal and actual trees, and then turn the actual tree into the ideal tree by taking the actions required.
+// At the end of the reification process, the actualTree is updated to reflect the changes.
 //
-// Each tree has an Inventory at the root.  Shrinkwrap is tracked by Arborist
-// instance.  It always refers to the actual tree, but is updated (and written
-// to disk) on reification.
+// Each tree has an Inventory at the root.
+// Shrinkwrap is tracked by Arborist instance.
+// It always refers to the actual tree, but is updated (and written to disk) on reification.
 
-// Each of the mixin "classes" adds functionality, but are not dependent on
-// constructor call order.  So, we just load them in an array, and build up
-// the base class, so that the overall voltron class is easier to test and
-// cover, and separation of concerns can be maintained.
+// Each of the mixin "classes" adds functionality, but are not dependent on constructor call order.
+// So, we just load them in an array, and build up the base class, so that the overall voltron class is easier to test and cover, and separation of concerns can be maintained.
+// Eventually, each mixin shared enough `Symbol.for` declarations that separation of concerns was not actually happening very well.
+// Unit testing was also masking bugs that surfaced when the code was used as a whole, so tests all require Arborist in full now.
+// So, this is now moving towards a single large class so that it's easier to weed out duplication in logic and efforts.
+// Tests are still separated by main instance method.
 
 const { resolve } = require('node:path')
 const { homedir } = require('node:os')
@@ -39,9 +36,6 @@
 const mixins = [
   require('../tracker.js'),
   require('./build-ideal-tree.js'),
-  require('./load-actual.js'),
-  require('./load-virtual.js'),
-  require('./rebuild.js'),
   require('./reify.js'),
   require('./isolated-reifier.js'),
 ]
@@ -68,6 +62,20 @@
   constructor (options = {}) {
     const timeEnd = time.start('arborist:ctor')
     super(options)
+
+    if (options.workspaces?.length && options.global) {
+      throw new Error('Cannot operate on workspaces in global mode')
+    }
+    // normalize trailing slash
+    const registry = options.registry || 'https://registry.npmjs.org'
+    options.registry = this.registry = registry.replace(/\/+$/, '') + '/'
+
+    // the tree of nodes on disk
+    this.actualTree = options.actualTree
+    this.idealTree = 'idealTree' in options ? options.idealTree : null
+    this.installLinks = 'installLinks' in options ? options.installLinks : false
+    this.legacyPeerDeps = 'legacyPeerDeps' in options ? options.legacyPeerDeps : false
+
     this.options = {
       nodeVersion: process.version,
       ...options,
@@ -88,12 +96,12 @@
       replaceRegistryHost: options.replaceRegistryHost,
       savePrefix: 'savePrefix' in options ? options.savePrefix : '^',
       scriptShell: options.scriptShell,
+      usePackageLock: 'packageLock' in options ? options.packageLock : true,
       workspaces: options.workspaces || [],
       workspacesEnabled: options.workspacesEnabled !== false,
     }
-    // TODO we only ever look at this.options.replaceRegistryHost, not
-    // this.replaceRegistryHost.  Defaulting needs to be written back to
-    // this.options to work properly
+    // TODO we only ever look at this.options.replaceRegistryHost, not this.replaceRegistryHost.
+    // Defaulting needs to be written back to this.options to work properly
     this.replaceRegistryHost = this.options.replaceRegistryHost =
       (!this.options.replaceRegistryHost || this.options.replaceRegistryHost === 'npmjs') ?
         'registry.npmjs.org' : this.options.replaceRegistryHost
@@ -104,14 +112,13 @@
     this.cache = resolve(this.options.cache)
     this.diff = null
     this.path = resolve(this.options.path)
+    this.scriptsRun = new Set()
     timeEnd()
   }
 
-  // TODO: We should change these to static functions instead
-  //   of methods for the next major version
+  // TODO: We should change these to static functions instead of methods for the next major version
 
-  // Get the actual nodes corresponding to a root node's child workspaces,
-  // given a list of workspace names.
+  // Get the actual nodes corresponding to a root node's child workspaces, given a list of workspace names.
   workspaceNodes (tree, workspaces) {
     const wsMap = tree.workspaces
     if (!wsMap) {

workspaces/arborist/lib/arborist/isolated-reifier.js

@@ -1,6 +1,5 @@
 const _makeIdealGraph = Symbol('makeIdealGraph')
 const _createIsolatedTree = Symbol.for('createIsolatedTree')
-const _createBundledTree = Symbol('createBundledTree')
 const { mkdirSync } = require('node:fs')
 const pacote = require('pacote')
 const { join } = require('node:path')
@@ -162,7 +161,7 @@ module.exports = cls => class IsolatedReifier extends cls {
     result.hasInstallScript = node.hasInstallScript
   }
 
-  async [_createBundledTree] () {
+  async #createBundledTree () {
     // TODO: make sure that idealTree object exists
     const idealTree = this.idealTree
     // TODO: test workspaces having bundled deps
@@ -217,7 +216,7 @@ module.exports = cls => class IsolatedReifier extends cls {
 
     const proxiedIdealTree = this.idealGraph
 
-    const bundledTree = await this[_createBundledTree]()
+    const bundledTree = await this.#createBundledTree()
 
     const treeHash = (startNode) => {
       // generate short hash based on the dependency tree