fix: don't assume submodule is on previous tag when strategy "tag" (#22)

Author: sgoudham

README.md

@@ -77,7 +77,7 @@ variables for each submodule that was updated:
   after it was updated.
 - `${prefix}--latestCommitSha`: The commit SHA of the submodule after it was
   updated.
-- `${prefix}--previousTag`: The tag of the submodule before it was updated. **May not exist if the submodule does not have any tags.**
+- `${prefix}--previousTag`: The latest tag of the submodule before it was updated. **May not exist if the submodule does not have any tags.**
 - `${prefix}--latestTag`: The tag that the submodule was updated to. **Only available when the strategy is set to 'tag'.**
 - `${prefix}--prBody`: A multi-line Markdown string intended for use in a pull request body.
 

dist/index.js

@@ -1,6 +1,6 @@
 import { expect, test, vi } from "vitest";
 import { getExecOutput } from "@actions/exec";
-import { getCommit, getPreviousTag, getTag } from "../git";
+import { getCommit, getPreviousTag, getTag, hasTag } from "../git";
 
 vi.mock("@actions/exec", async () => {
   return {
@@ -64,3 +64,33 @@ test("fail to get previous tag and continue", async () => {
   const previousTag = await getPreviousTag(input);
   expect(previousTag).toEqual(undefined);
 });
+
+test("previous commit has a tag pointing to it", async () => {
+  const input = "a19a19bd14f26c3bba311bbffc5a74710add5ac2";
+
+  vi.mocked(getExecOutput).mockReturnValueOnce(
+    Promise.resolve({
+      exitCode: 0,
+      stdout: "v1.0.0\n",
+      stderr: "",
+    })
+  );
+
+  const previousCommitHasTag = await hasTag("", input);
+  expect(previousCommitHasTag).true;
+});
+
+test("previous commit has no tag and continue", async () => {
+  const input = "a19a19bd14f26c3bba311bbffc5a74710add5ac2";
+
+  vi.mocked(getExecOutput).mockReturnValueOnce(
+    Promise.resolve({
+      exitCode: 0,
+      stdout: "\n\n",
+      stderr: "",
+    })
+  );
+
+  const previousCommitHasTag = await hasTag("", input);
+  expect(previousCommitHasTag).false;
+});

dist/index.js.map

@@ -74,6 +74,13 @@ test("extract single submodule from .gitmodules", async () => {
         stderr: "",
       })
     )
+    .mockReturnValueOnce(
+      Promise.resolve({
+        exitCode: 0,
+        stdout: `\n${expected[0].previousTag}`,
+        stderr: "",
+      })
+    )
     .mockReturnValueOnce(
       Promise.resolve({
         exitCode: 0,
@@ -90,6 +97,7 @@ test("extract single submodule from .gitmodules that has no tags", async () => {
   const input = await readFile("src/__tests__/fixtures/single-gitmodules.ini");
   const expected = [mdBookSubmodule()];
   expected[0].previousTag = undefined;
+  expected[0].previousCommitShaHasTag = false;
 
   vi.mocked(getExecOutput)
     .mockReturnValueOnce(
@@ -99,6 +107,13 @@ test("extract single submodule from .gitmodules that has no tags", async () => {
         stderr: "",
       })
     )
+    .mockReturnValueOnce(
+      Promise.resolve({
+        exitCode: 0,
+        stdout: `\n\n`,
+        stderr: "",
+      })
+    )
     .mockRejectedValueOnce(new Error());
 
   const actual = await parseGitmodules(input);
@@ -152,6 +167,27 @@ test("extract multiple git submodules from .gitmodules", async () => {
         stderr: "",
       })
     )
+    .mockReturnValueOnce(
+      Promise.resolve({
+        exitCode: 0,
+        stdout: `\n${vscodeIcons.previousTag}`,
+        stderr: "",
+      })
+    )
+    .mockReturnValueOnce(
+      Promise.resolve({
+        exitCode: 0,
+        stdout: `\n${nvim.previousTag}`,
+        stderr: "",
+      })
+    )
+    .mockReturnValueOnce(
+      Promise.resolve({
+        exitCode: 0,
+        stdout: `\n${mdBook.previousTag}`,
+        stderr: "",
+      })
+    )
     .mockReturnValueOnce(
       Promise.resolve({
         exitCode: 0,

src/__tests__/git.test.ts

@@ -75,3 +75,27 @@ test("single pr body for single submodule using tag strategy", async () => {
 
   expect(actual).toContain(expected);
 });
+
+test("markdown pr body for multiple submodules using tag strategy and previous commit has no tag", async () => {
+  const [mdBook, nvim, vscodeIcons] = [
+    mdBookSubmodule(),
+    nvimSubmodule(),
+    vscodeIconsSubmodule(),
+  ];
+  mdBook.previousCommitShaHasTag = false;
+  nvim.previousCommitShaHasTag = false;
+  mdBook.latestTag = "v2.2.0";
+  nvim.latestTag = "v1.9.0";
+  vscodeIcons.latestTag = "v1.15.0";
+  const submodules = [mdBook, nvim, vscodeIcons];
+  const expected = `
+| --- | --- | --- |
+| [catppuccin/mdBook](https://github.com/catppuccin/mdBook.git) | ports/mdBook | [a19a19b...v2.2.0](https://github.com/catppuccin/mdBook/compare/a19a19b...v2.2.0) |
+| [catppuccin/nvim](https://github.com/catppuccin/nvim.git) | ports/nvim | [774a4ed...v1.9.0](https://github.com/catppuccin/nvim/compare/774a4ed...v1.9.0) |
+| [catppuccin/vscode-icons](https://github.com/catppuccin/vscode-icons.git) | ports/vscode-icons | [v1.14.0...v1.15.0](https://github.com/catppuccin/vscode-icons/compare/v1.14.0...v1.15.0) |
+`;
+
+  const actual = multiplePrBody(submodules);
+
+  expect(actual).toContain(expected);
+});

src/__tests__/main.test.ts

@@ -17,6 +17,7 @@ class SubmoduleBuilder {
       !this.submodule.remoteName ||
       !this.submodule.previousShortCommitSha ||
       !this.submodule.previousCommitSha ||
+      !this.submodule.previousCommitShaHasTag ||
       !this.submodule.latestShortCommitSha ||
       !this.submodule.latestCommitSha
     ) {
@@ -33,6 +34,7 @@ export const mdBookSubmodule = (
   remoteName: string = "catppuccin/mdBook",
   previousShortCommitSha: string = "a19a19b",
   previousCommitSha: string = "a19a19bd14f26c3bba311bbffc5a74710add5ac2",
+  previousCommitShaHasTag: boolean = true,
   previousTag: string = "v0.1.2",
   latestShortCommitSha: string = "a19a19b",
   latestCommitSha: string = "a19a19bd14f26c3bba311bbffc5a74710add5ac2"
@@ -44,6 +46,7 @@ export const mdBookSubmodule = (
     remoteName,
     previousShortCommitSha,
     previousCommitSha,
+    previousCommitShaHasTag,
     previousTag,
     latestShortCommitSha,
     latestCommitSha,
@@ -57,6 +60,7 @@ export const vscodeIconsSubmodule = (
   remoteName: string = "catppuccin/vscode-icons",
   previousShortCommitSha: string = "71d98b8",
   previousCommitSha: string = "71d98b81bfdb6b8d3527037c3017eb07e6ec0621",
+  previousCommitShaHasTag: boolean = true,
   previousTag: string = "v1.14.0",
   latestShortCommitSha: string = "71d98b8",
   latestCommitSha: string = "71d98b81bfdb6b8d3527037c3017eb07e6ec0621"
@@ -68,6 +72,7 @@ export const vscodeIconsSubmodule = (
     remoteName,
     previousShortCommitSha,
     previousCommitSha,
+    previousCommitShaHasTag,
     previousTag,
     latestShortCommitSha,
     latestCommitSha,
@@ -81,6 +86,7 @@ export const nvimSubmodule = (
   remoteName: string = "catppuccin/nvim",
   previousShortCommitSha: string = "774a4ed",
   previousCommitSha: string = "774a4ed9a69d0a2633da60f73aa63a8e23aacced",
+  previousCommitShaHasTag: boolean = true,
   previousTag: string = "v1.8.0",
   latestShortCommitSha: string = "774a4ed",
   latestCommitSha: string = "774a4ed9a69d0a2633da60f73aa63a8e23aacced"
@@ -92,6 +98,7 @@ export const nvimSubmodule = (
     remoteName,
     previousShortCommitSha,
     previousCommitSha,
+    previousCommitShaHasTag,
     previousTag,
     latestShortCommitSha,
     latestCommitSha,

src/__tests__/markdown.test.ts

@@ -27,3 +27,15 @@ export const getPreviousTag = async (
     return undefined;
   });
 };
+
+export const hasTag = async (
+  path: string,
+  commitSha: string
+): Promise<boolean> => {
+  core.info(`'${path}': Checking if tag exists at commit '${commitSha}'`);
+  const options = { cwd: path };
+  const tag = (
+    await getExecOutput("git tag --points-at HEAD", [], options)
+  ).stdout.trim();
+  return tag !== "";
+};

src/__tests__/utils.ts

@@ -4,7 +4,7 @@ import * as fs from "node:fs/promises";
 import { logInfoAndDebug, toJson, toJsonPretty } from "./logging";
 import { multiplePrBody, singlePrBody } from "./markdown";
 import { z } from "zod";
-import { getCommit, getPreviousTag, getTag } from "./git";
+import { getCommit, getPreviousTag, getTag, hasTag } from "./git";
 import { parse } from "ini";
 
 const updateStrategy = z.enum(["commit", "tag"]);
@@ -31,6 +31,7 @@ export type Submodule = {
   remoteName: string;
   previousShortCommitSha: string;
   previousCommitSha: string;
+  previousCommitShaHasTag: boolean;
   previousTag?: string;
   latestShortCommitSha: string;
   latestCommitSha: string;
@@ -96,6 +97,7 @@ export const parseGitmodules = async (
       const urlParts = url.replace(".git", "").split("/");
       const remoteName = `${urlParts[3]}/${urlParts[4]}`;
       const [previousCommitSha, previousShortCommitSha] = await getCommit(path);
+      const previousCommitShaHasTag = await hasTag(path, previousCommitSha);
       const previousTag = await getPreviousTag(path);
 
       return {
@@ -105,13 +107,13 @@ export const parseGitmodules = async (
         remoteName,
         previousShortCommitSha,
         previousCommitSha,
+        previousCommitShaHasTag,
+        previousTag,
 
         // The latest commit should be updated after the submodule is updated
         // If you think about it, the "previous" commit is the latest commit too
         latestShortCommitSha: previousShortCommitSha,
         latestCommitSha: previousCommitSha,
-
-        previousTag,
       };
     })
   );

src/git.ts

@@ -14,7 +14,11 @@ const tableRow = (submodule: Submodule) => {
   let changeDisplay = `${submodule.previousShortCommitSha}...${submodule.latestShortCommitSha}`;
   let changeUrl = `[${changeDisplay}](${cleanUrl}/compare/${submodule.previousCommitSha}...${submodule.latestCommitSha})`;
   if (submodule.latestTag) {
-    changeDisplay = `${submodule.previousTag}...${submodule.latestTag}`;
+    if (submodule.previousCommitShaHasTag) {
+      changeDisplay = `${submodule.previousTag}...${submodule.latestTag}`;
+    } else {
+      changeDisplay = `${submodule.previousShortCommitSha}...${submodule.latestTag}`;
+    }
     changeUrl = `[${changeDisplay}](${cleanUrl}/compare/${changeDisplay})`;
   }
   return `| ${name} | ${submodule.path} | ${changeUrl} |`;