Fix mistakes introduced in "Smarter binary module deduping". Fixes gh…

…-1302
tessel · Aug 8, 2017 · 356a4aa · 356a4aa
1 parent 8f6e048
commit 356a4aa
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 13 deletions.
diff --git a/lib/tessel/deployment/javascript.js b/lib/tessel/deployment/javascript.js
@@ -267,14 +267,14 @@ exportables.resolveBinaryModules = function(options) {
         //
         // In cases where the modules are the same version,
         // we'll shed the copy that's lower in the tree.
-
         if (binaryModulesUsed.has(releasename)) {
-
           let existing = binaryModulesUsed.get(releasename);
-          let a = existing.modulePath.match(/\//g).length;
-          let b = details.modulePath.match(/\//g).length;
+          let a = existing.globPath.split('/').length;
+          let b = details.globPath.split('/').length;
 
-          if (a > b) {
+          // The current `details` module is higher up than
+          // the matched `existing` module...
+          if (b < a) {
             // The module described in 'details' is higher up,
             // use this instead. Since the binary has already
             // been resolved and that has no directory location

diff --git a/test/unit/deployment/javascript.js b/test/unit/deployment/javascript.js
@@ -3085,13 +3085,16 @@ exports['deployment.js.injectBinaryModules'] = {
   },
 
   usesBinaryHighestInTreeWhenEncounteringDuplicates(test) {
-    // test.expect(1);
+    test.expect(6);
     this.target = path.normalize('test/unit/fixtures/project-binary-modules-duplicate-lower-deps');
     this.relative.restore();
     this.relative = sandbox.stub(path, 'relative', () => {
       return path.join(FIXTURE_PATH, '/project-binary-modules-duplicate-lower-deps/');
     });
 
+    // We WANT to glob the actual target directory
+    this.globSync.restore();
+
     this.mapHas = sandbox.spy(Map.prototype, 'has');
     this.mapGet = sandbox.spy(Map.prototype, 'get');
     this.mapSet = sandbox.spy(Map.prototype, 'set');
@@ -3116,16 +3119,14 @@ exports['deployment.js.injectBinaryModules'] = {
       test.equal(this.mapHas.getCall(0).args[0], 'release@1.1.1');
       test.equal(this.mapHas.getCall(1).args[0], 'release@1.1.1');
 
-      // Ensure that a swap has occurred
-      test.equal(this.mapGet.lastCall.returnValue, this.mapSet.lastCall.args[1]);
-      test.equal(
-        path.normalize(this.mapSet.lastCall.args[1].modulePath),
-        path.normalize('node_modules/release/')
-      );
-
       // Ensure that only one of the two were included in the
       // final list of binary modules to bundle
       test.equal(binaryModulesUsed.size, 1);
+      // Ensure that the swap has occurred
+      test.equal(
+        path.normalize(binaryModulesUsed.get('release@1.1.1').globPath),
+        path.normalize('node_modules/release/build/Release/release.node')
+      );
 
       test.done();
     });