Merge pull request #206 from garden-io/fix-tree-version-string

fix(versioning): version string should include dirty timestamp

src/plugins/generic.ts

@@ -43,7 +43,7 @@ import {
   baseTestSpecSchema,
 } from "../types/test"
 import { spawn } from "../util/util"
-import { writeVersionFile, readVersionFile } from "../vcs/base"
+import { writeVersionFile, readVersionFile, getVersionString } from "../vcs/base"
 import execa = require("execa")
 
 export const name = "generic"
@@ -95,6 +95,24 @@ export async function parseGenericModule(
   }
 }
 
+export async function getGenericModuleBuildStatus({ module }: GetModuleBuildStatusParams): Promise<BuildStatus> {
+  if (!module.config.build.command.length) {
+    return { ready: true }
+  }
+
+  const buildVersionFilePath = join(await module.getBuildPath(), buildVersionFilename)
+  const builtVersion = await readVersionFile(buildVersionFilePath)
+  const moduleVersion = await module.getVersion()
+
+  const builtVersionString = builtVersion && getVersionString(builtVersion)
+
+  if (builtVersionString && builtVersionString === moduleVersion.versionString) {
+    return { ready: true }
+  }
+
+  return { ready: false }
+}
+
 export async function buildGenericModule({ module }: BuildModuleParams<GenericModule>): Promise<BuildResult> {
   const config: ModuleConfig = module.config
 
@@ -155,24 +173,9 @@ export const genericPlugin: GardenPlugin = {
   moduleActions: {
     generic: {
       parseModule: parseGenericModule,
+      getModuleBuildStatus: getGenericModuleBuildStatus,
       buildModule: buildGenericModule,
       testModule: testGenericModule,
-
-      async getModuleBuildStatus({ module }: GetModuleBuildStatusParams): Promise<BuildStatus> {
-        if (!module.config.build.command.length) {
-          return { ready: true }
-        }
-
-        const buildVersionFilePath = join(await module.getBuildPath(), buildVersionFilename)
-        const builtVersion = await readVersionFile(buildVersionFilePath)
-        const moduleVersion = await module.getVersion()
-
-        if (builtVersion && builtVersion.latestCommit === moduleVersion.versionString) {
-          return { ready: true }
-        }
-
-        return { ready: false }
-      },
     },
   },
 }

src/vcs/base.ts

@@ -85,7 +85,7 @@ export abstract class VcsHandler {
 
     if (dependencies.length === 0) {
       return {
-        versionString: treeVersion.latestCommit,
+        versionString: getVersionString(treeVersion),
         dirtyTimestamp: treeVersion.dirtyTimestamp,
         dependencyVersions: {},
       }
@@ -119,7 +119,7 @@ export abstract class VcsHandler {
 
       if (latestDirty.length > 1) {
         // if the last modified timestamp is common across multiple modules, hash their versions
-        const versionString = hashVersions(latestDirty)
+        const versionString = `${hashVersions(latestDirty)}-${dirtyTimestamp}`
 
         return {
           versionString,
@@ -129,7 +129,7 @@ export abstract class VcsHandler {
       } else {
         // if there's just one module that was most recently modified, return that version
         return {
-          versionString: `${latestDirty[0].latestCommit}-${dirtyTimestamp}`,
+          versionString: getVersionString(latestDirty[0]),
           dirtyTimestamp,
           dependencyVersions,
         }
@@ -183,3 +183,9 @@ export async function readVersionFile(path: string): Promise<TreeVersion | null>
 export async function writeVersionFile(path: string, version: TreeVersion) {
   await writeFile(path, JSON.stringify(version))
 }
+
+export function getVersionString(treeVersion: TreeVersion) {
+  return treeVersion.dirtyTimestamp
+    ? `${treeVersion.latestCommit}-${treeVersion.dirtyTimestamp}`
+    : treeVersion.latestCommit
+}

test/src/vcs/base.ts

@@ -73,7 +73,26 @@ describe("VcsHandler", () => {
       })
     })
 
-    it("should return the latest dirty version if any", async () => {
+    it("should return module version if there are no dependencies and properly handle a dirty timestamp", async () => {
+      const module = await ctx.getModule("module-a")
+      const latestCommit = "abcdef"
+      const version = {
+        latestCommit,
+        dirtyTimestamp: 1234,
+      }
+
+      handler.setTestVersion(module.path, version)
+
+      const result = await handler.resolveVersion(module, [])
+
+      expect(result).to.eql({
+        versionString: "abcdef-1234",
+        dirtyTimestamp: 1234,
+        dependencyVersions: {},
+      })
+    })
+
+    it("should return the dirty version if there is a single one", async () => {
       const [moduleA, moduleB, moduleC] = await ctx.getModules(["module-a", "module-b", "module-c"])
 
       // note: module-a has a version file
@@ -84,7 +103,7 @@ describe("VcsHandler", () => {
 
       const versionB = {
         latestCommit: "qwerty",
-        dirtyTimestamp: 456,
+        dirtyTimestamp: null,
       }
       handler.setTestVersion(moduleB.path, versionB)
 
@@ -96,48 +115,48 @@ describe("VcsHandler", () => {
       handler.setTestVersion(moduleC.path, versionC)
 
       expect(await handler.resolveVersion(moduleC, [moduleA, moduleB])).to.eql({
-        versionString: "qwerty-456",
-        dirtyTimestamp: 456,
+        versionString: "asdfgh-123",
+        dirtyTimestamp: 123,
         dependencyVersions: {
           "module-a": versionA,
           "module-b": versionB,
         },
       })
     })
 
-    it("should hash together the version of the module and all dependencies if none are dirty", async () => {
+    it("should return the latest dirty version if there are multiple", async () => {
       const [moduleA, moduleB, moduleC] = await ctx.getModules(["module-a", "module-b", "module-c"])
 
+      // note: module-a has a version file
       const versionA = {
         latestCommit: "1234567890",
         dirtyTimestamp: null,
       }
 
-      const versionStringB = "qwerty"
       const versionB = {
-        latestCommit: versionStringB,
-        dirtyTimestamp: null,
+        latestCommit: "qwerty",
+        dirtyTimestamp: 456,
       }
       handler.setTestVersion(moduleB.path, versionB)
 
       const versionStringC = "asdfgh"
       const versionC = {
         latestCommit: versionStringC,
-        dirtyTimestamp: null,
+        dirtyTimestamp: 123,
       }
       handler.setTestVersion(moduleC.path, versionC)
 
       expect(await handler.resolveVersion(moduleC, [moduleA, moduleB])).to.eql({
-        versionString: "v5ff3a146d9",
-        dirtyTimestamp: null,
+        versionString: "qwerty-456",
+        dirtyTimestamp: 456,
         dependencyVersions: {
           "module-a": versionA,
           "module-b": versionB,
         },
       })
     })
 
-    it("should hash together the dirty versions if there are multiple with same timestamp", async () => {
+    it("should hash together the version of the module and all dependencies if none are dirty", async () => {
       const [moduleA, moduleB, moduleC] = await ctx.getModules(["module-a", "module-b", "module-c"])
 
       const versionA = {
@@ -148,25 +167,60 @@ describe("VcsHandler", () => {
       const versionStringB = "qwerty"
       const versionB = {
         latestCommit: versionStringB,
-        dirtyTimestamp: 1234,
+        dirtyTimestamp: null,
       }
       handler.setTestVersion(moduleB.path, versionB)
 
       const versionStringC = "asdfgh"
       const versionC = {
         latestCommit: versionStringC,
-        dirtyTimestamp: 1234,
+        dirtyTimestamp: null,
       }
       handler.setTestVersion(moduleC.path, versionC)
 
       expect(await handler.resolveVersion(moduleC, [moduleA, moduleB])).to.eql({
-        versionString: "vcfa6d28ec5",
-        dirtyTimestamp: 1234,
+        versionString: "v5ff3a146d9",
+        dirtyTimestamp: null,
         dependencyVersions: {
           "module-a": versionA,
           "module-b": versionB,
         },
       })
     })
+
+    it(
+      "should hash together the dirty versions and add the timestamp if there are multiple with same timestamp",
+      async () => {
+
+        const [moduleA, moduleB, moduleC] = await ctx.getModules(["module-a", "module-b", "module-c"])
+
+        const versionA = {
+          latestCommit: "1234567890",
+          dirtyTimestamp: null,
+        }
+
+        const versionStringB = "qwerty"
+        const versionB = {
+          latestCommit: versionStringB,
+          dirtyTimestamp: 1234,
+        }
+        handler.setTestVersion(moduleB.path, versionB)
+
+        const versionStringC = "asdfgh"
+        const versionC = {
+          latestCommit: versionStringC,
+          dirtyTimestamp: 1234,
+        }
+        handler.setTestVersion(moduleC.path, versionC)
+
+        expect(await handler.resolveVersion(moduleC, [moduleA, moduleB])).to.eql({
+          versionString: "vcfa6d28ec5-1234",
+          dirtyTimestamp: 1234,
+          dependencyVersions: {
+            "module-a": versionA,
+            "module-b": versionB,
+          },
+        })
+      })
   })
 })