Merge pull request #118 from TrueCar/cybrass/patch-update-version-logic

Make sure to perform a numeric comparison

src/lib/updateVersion.js

@@ -2,6 +2,7 @@ import fs from "fs";
 import path from "path";
 import logger from "./logger";
 import readFileSyncStrip from "./readFileSyncStrip";
+import { compareVersions } from "./utils";
 
 module.exports = function updateLastVersionUsed(gluestickVersion, withWarnings=true) {
 
@@ -13,7 +14,9 @@ module.exports = function updateLastVersionUsed(gluestickVersion, withWarnings=t
   if (!fileContents.length) { fileContents = "{}"; }
 
   const project = JSON.parse(fileContents);
-  if (withWarnings && gluestickVersion < project.version) {
+  const needsUpdate = compareVersions(gluestickVersion, project.version || "") === -1;
+
+  if (withWarnings && needsUpdate) {
     logger.warn("This project is configured to work with versions >= " + project.version + " Please upgrade your global `gluestick` module with `sudo npm install gluestick -g");
   }
 

src/lib/utils.js

@@ -9,6 +9,7 @@ export function quitUnlessGluestickProject(command) {
     process.exit();
   }
 }
+
 export function isGluestickProject(dir=process.cwd()) {
   try {
     fs.statSync(path.join(dir, ".gluestick"));
@@ -17,3 +18,17 @@ export function isGluestickProject(dir=process.cwd()) {
   }
   return true;
 }
+
+export function compareVersions(versionA, versionB) {
+  const numbersA = versionA.split(".");
+  const numbersB = versionB.split(".");
+  for (let i = 0; i < 3; i++) {
+    const numberA = Number(numbersA[i]);
+    const numberB = Number(numbersB[i]);
+    if (numberA > numberB) { return 1; }
+    if (numberB > numberA) { return -1; }
+    if (!Number.isNaN(numberA) && Number.isNaN(numberB)) { return 1; }
+    if (Number.isNaN(numberA) && !Number.isNaN(numberB)) { return -1; }
+  }
+  return 0;
+}

test/lib/utils.test.js

@@ -4,7 +4,7 @@ import fs from "fs";
 import temp from "temp";
 import rimraf from "rimraf";
 import logger from "../../src/lib/logger";
-import { isGluestickProject, quitUnlessGluestickProject } from "../../src/lib/utils";
+import { isGluestickProject, quitUnlessGluestickProject, compareVersions } from "../../src/lib/utils";
 
 describe("utils", function () {
 
@@ -79,5 +79,50 @@ describe("utils", function () {
 
   });
 
-});
+  describe("compareVersions", function () {
+
+    it("returns 1 if the major number of the first arg is greater than the major number of the second arg", () => {
+      const versionA = "1.0.0";
+      const versionB = "0.0.0";
+      expect(compareVersions(versionA, versionB)).to.equal(1);
+    });
+
+    it("returns -1 if the major number of the first arg is less than the major number of the second arg", () => {
+      const versionA = "0.0.0";
+      const versionB = "1.0.0";
+      expect(compareVersions(versionA, versionB)).to.equal(-1);
+    });
+
+    it("returns 0 if the major number of the first arg equals the major number of the second arg", () => {
+      const versionA = "1.0.0";
+      const versionB = "1.0.0";
+      expect(compareVersions(versionA, versionB)).to.equal(0);
+    });
+
+    it("returns -1 if the minor number of the first arg is numerically less than the minor number of the second arg", () => {
+      const versionA = "1.8.0";
+      const versionB = "1.10.0";
+      expect(compareVersions(versionA, versionB)).to.equal(-1);
+    });
 
+    it("returns 1 if the minor number of the first arg is numerically greater than the minor number of the second arg", () => {
+      const versionA = "1.10.0";
+      const versionB = "1.8.0";
+      expect(compareVersions(versionA, versionB)).to.equal(1);
+    });
+
+    it("returns 1 if the first arg consists of numbers but the second arg doesn't", () => {
+      const versionA = "1.0.0";
+      const versionB = "abc";
+      expect(compareVersions(versionA, versionB)).to.equal(1);
+    });
+
+    it("returns -1 if the first arg doesn't consist of numbers but the second arg does", () => {
+      const versionA = "abc";
+      const versionB = "1.0.0";
+      expect(compareVersions(versionA, versionB)).to.equal(-1);
+    });
+
+  });
+
+});