updatenotification: only notify mm-repo for master if tag present (#3004)

see discussion here:
https://github.com/MichMich/MagicMirror/pull/2991#issuecomment-1376372720

I still see a need for updating `master` in special cases (e.g. correct
errors in README.md which would otherwise be present up to 3 month until
next release) so with this PR **only for MagicMirror repo** and **only
for `master` branch** updatenotifications are only triggered if at least
one of the new commits has a tag.

May @MichMich must decide if this is wanted.

Co-authored-by: Veeck <github@veeck.de>

CHANGELOG.md

@@ -27,6 +27,7 @@ _This release is scheduled to be released on 2023-04-01._
 
 - Use develop as target branch for dependabot
 - Update issue template and contributing doc
+- Changed updatenotification module for MagicMirror repo only: Send only notifications for `master` if there is a tag on a newer commit
 - Update dates in Calendar widgets every minute
 
 ### Fixed

modules/default/updatenotification/git_helper.js

@@ -117,7 +117,7 @@ class GitHelper {
 			return;
 		}
 
-		if (gitInfo.isBehindInStatus) {
+		if (gitInfo.isBehindInStatus && (gitInfo.module !== "MagicMirror" || gitInfo.current !== "master")) {
 			return gitInfo;
 		}
 
@@ -141,6 +141,29 @@ class GitHelper {
 			const { stdout } = await this.execShell(`cd ${repo.folder} && git rev-list --ancestry-path --count ${refDiff}`);
 			gitInfo.behind = parseInt(stdout);
 
+			// for MagicMirror-Repo and "master" branch avoid getting notified when no tag is in refDiff
+			// so only releases are reported and we can change e.g. the README.md without sending notifications
+			if (gitInfo.behind > 0 && gitInfo.module === "MagicMirror" && gitInfo.current === "master") {
+				let tagList = "";
+				try {
+					const { stdout } = await this.execShell(`cd ${repo.folder} && git ls-remote -q --tags --refs`);
+					tagList = stdout.trim();
+				} catch (err) {
+					Log.error(`Failed to get tag list for ${repo.module}: ${err}`);
+				}
+				// check if tag is between commits and only report behind > 0 if so
+				try {
+					const { stdout } = await this.execShell(`cd ${repo.folder} && git rev-list --ancestry-path ${refDiff}`);
+					let cnt = 0;
+					for (const ref of stdout.trim().split("\n")) {
+						if (tagList.includes(ref)) cnt++; // tag found
+					}
+					if (cnt === 0) gitInfo.behind = 0;
+				} catch (err) {
+					Log.error(`Failed to get git revisions for ${repo.module}: ${err}`);
+				}
+			}
+
 			return gitInfo;
 		} catch (err) {
 			Log.error(`Failed to get git revisions for ${repo.module}: ${err}`);

tests/unit/functions/__snapshots__/updatenotification_spec.js.snap

@@ -11,7 +11,7 @@ exports[`Updatenotification custom module returns status information without has
 }
 `;
 
-exports[`Updatenotification MagicMirror returns status information 1`] = `
+exports[`Updatenotification MagicMirror on develop returns status information 1`] = `
 {
   "behind": 5,
   "current": "develop",
@@ -22,7 +22,7 @@ exports[`Updatenotification MagicMirror returns status information 1`] = `
 }
 `;
 
-exports[`Updatenotification MagicMirror returns status information early if isBehindInStatus 1`] = `
+exports[`Updatenotification MagicMirror on develop returns status information early if isBehindInStatus 1`] = `
 {
   "behind": 5,
   "current": "develop",
@@ -32,3 +32,69 @@ exports[`Updatenotification MagicMirror returns status information early if isBe
   "tracking": "origin/develop",
 }
 `;
+
+exports[`Updatenotification MagicMirror on master (empty taglist) returns status information 1`] = `
+{
+  "behind": 5,
+  "current": "master",
+  "hash": "332e429a41f1a2339afd4f0ae96dd125da6beada",
+  "isBehindInStatus": false,
+  "module": "MagicMirror",
+  "tracking": "origin/master",
+}
+`;
+
+exports[`Updatenotification MagicMirror on master (empty taglist) returns status information early if isBehindInStatus 1`] = `
+{
+  "behind": 5,
+  "current": "master",
+  "hash": "332e429a41f1a2339afd4f0ae96dd125da6beada",
+  "isBehindInStatus": true,
+  "module": "MagicMirror",
+  "tracking": "origin/master",
+}
+`;
+
+exports[`Updatenotification MagicMirror on master with match in taglist returns status information 1`] = `
+{
+  "behind": 5,
+  "current": "master",
+  "hash": "332e429a41f1a2339afd4f0ae96dd125da6beada",
+  "isBehindInStatus": false,
+  "module": "MagicMirror",
+  "tracking": "origin/master",
+}
+`;
+
+exports[`Updatenotification MagicMirror on master with match in taglist returns status information early if isBehindInStatus 1`] = `
+{
+  "behind": 5,
+  "current": "master",
+  "hash": "332e429a41f1a2339afd4f0ae96dd125da6beada",
+  "isBehindInStatus": true,
+  "module": "MagicMirror",
+  "tracking": "origin/master",
+}
+`;
+
+exports[`Updatenotification MagicMirror on master without match in taglist returns status information 1`] = `
+{
+  "behind": 0,
+  "current": "master",
+  "hash": "332e429a41f1a2339afd4f0ae96dd125da6beada",
+  "isBehindInStatus": false,
+  "module": "MagicMirror",
+  "tracking": "origin/master",
+}
+`;
+
+exports[`Updatenotification MagicMirror on master without match in taglist returns status information early if isBehindInStatus 1`] = `
+{
+  "behind": 0,
+  "current": "master",
+  "hash": "332e429a41f1a2339afd4f0ae96dd125da6beada",
+  "isBehindInStatus": true,
+  "module": "MagicMirror",
+  "tracking": "origin/master",
+}
+`;

tests/unit/functions/updatenotification_spec.js

@@ -23,12 +23,10 @@ describe("Updatenotification", () => {
 	let gitRevParseOut;
 	let gitStatusOut;
 	let gitFetchOut;
+	let gitRevListCountOut;
 	let gitRevListOut;
-	let gitRemoteErr;
-	let gitRevParseErr;
-	let gitStatusErr;
 	let gitFetchErr;
-	let gitRevListErr;
+	let gitTagListOut;
 
 	beforeAll(async () => {
 		const { promisify } = require("util");
@@ -43,24 +41,26 @@ describe("Updatenotification", () => {
 		gitRevParseOut = "";
 		gitStatusOut = "";
 		gitFetchOut = "";
+		gitRevListCountOut = "";
 		gitRevListOut = "";
-		gitRemoteErr = "";
-		gitRevParseErr = "";
-		gitStatusErr = "";
 		gitFetchErr = "";
-		gitRevListErr = "";
+		gitTagListOut = "";
 
 		execMock.mockImplementation((command) => {
 			if (command.includes("git remote -v")) {
-				return { stdout: gitRemoteOut, stderr: gitRemoteErr };
+				return { stdout: gitRemoteOut };
 			} else if (command.includes("git rev-parse HEAD")) {
-				return { stdout: gitRevParseOut, stderr: gitRevParseErr };
+				return { stdout: gitRevParseOut };
 			} else if (command.includes("git status -sb")) {
-				return { stdout: gitStatusOut, stderr: gitStatusErr };
+				return { stdout: gitStatusOut };
 			} else if (command.includes("git fetch -n --dry-run")) {
 				return { stdout: gitFetchOut, stderr: gitFetchErr };
 			} else if (command.includes("git rev-list --ancestry-path --count")) {
-				return { stdout: gitRevListOut, stderr: gitRevListErr };
+				return { stdout: gitRevListCountOut };
+			} else if (command.includes("git rev-list --ancestry-path")) {
+				return { stdout: gitRevListOut };
+			} else if (command.includes("git ls-remote -q --tags --refs")) {
+				return { stdout: gitTagListOut };
 			}
 		});
 	});
@@ -71,7 +71,7 @@ describe("Updatenotification", () => {
 		jest.clearAllMocks();
 	});
 
-	describe("MagicMirror", () => {
+	describe("MagicMirror on develop", () => {
 		const moduleName = "MagicMirror";
 
 		beforeEach(async () => {
@@ -79,7 +79,7 @@ describe("Updatenotification", () => {
 			gitRevParseOut = "332e429a41f1a2339afd4f0ae96dd125da6beada";
 			gitStatusOut = "## develop...origin/develop\n M tests/unit/functions/updatenotification_spec.js\n";
 			gitFetchErr = "From github.com:MichMich/MagicMirror\n60e0377..332e429  develop          -> origin/develop\n";
-			gitRevListOut = "5";
+			gitRevListCountOut = "5";
 
 			await gitHelper.add(moduleName);
 		});
@@ -110,6 +110,127 @@ describe("Updatenotification", () => {
 		});
 	});
 
+	describe("MagicMirror on master (empty taglist)", () => {
+		const moduleName = "MagicMirror";
+
+		beforeEach(async () => {
+			gitRemoteOut = "origin\tgit@github.com:MichMich/MagicMirror.git (fetch)\norigin\tgit@github.com:MichMich/MagicMirror.git (push)\n";
+			gitRevParseOut = "332e429a41f1a2339afd4f0ae96dd125da6beada";
+			gitStatusOut = "## master...origin/master\n M tests/unit/functions/updatenotification_spec.js\n";
+			gitFetchErr = "From github.com:MichMich/MagicMirror\n60e0377..332e429  master          -> origin/master\n";
+			gitRevListCountOut = "5";
+
+			await gitHelper.add(moduleName);
+		});
+
+		it("returns status information", async () => {
+			const repos = await gitHelper.getRepos();
+			expect(repos[0]).toMatchSnapshot();
+			expect(execMock).toHaveBeenCalledTimes(7);
+		});
+
+		it("returns status information early if isBehindInStatus", async () => {
+			gitStatusOut = "## master...origin/master [behind 5]";
+
+			const repos = await gitHelper.getRepos();
+			expect(repos[0]).toMatchSnapshot();
+			expect(execMock).toHaveBeenCalledTimes(7);
+		});
+
+		it("excludes repo if status can't be retrieved", async () => {
+			const errorMessage = "Failed to retrieve status";
+			execMock.mockRejectedValueOnce(errorMessage);
+
+			const repos = await gitHelper.getRepos();
+			expect(repos.length).toBe(0);
+
+			const { error } = require("logger");
+			expect(error).toHaveBeenCalledWith(`Failed to retrieve repo info for ${moduleName}: Failed to retrieve status`);
+		});
+	});
+
+	describe("MagicMirror on master with match in taglist", () => {
+		const moduleName = "MagicMirror";
+
+		beforeEach(async () => {
+			gitRemoteOut = "origin\tgit@github.com:MichMich/MagicMirror.git (fetch)\norigin\tgit@github.com:MichMich/MagicMirror.git (push)\n";
+			gitRevParseOut = "332e429a41f1a2339afd4f0ae96dd125da6beada";
+			gitStatusOut = "## master...origin/master\n M tests/unit/functions/updatenotification_spec.js\n";
+			gitFetchErr = "From github.com:MichMich/MagicMirror\n60e0377..332e429  master          -> origin/master\n";
+			gitRevListCountOut = "5";
+			gitTagListOut = "332e429a41f1a2339afd4f0ae96dd125da6beada...tag...\n";
+			gitRevListOut = "332e429a41f1a2339afd4f0ae96dd125da6beada\n";
+
+			await gitHelper.add(moduleName);
+		});
+
+		it("returns status information", async () => {
+			const repos = await gitHelper.getRepos();
+			expect(repos[0]).toMatchSnapshot();
+			expect(execMock).toHaveBeenCalledTimes(7);
+		});
+
+		it("returns status information early if isBehindInStatus", async () => {
+			gitStatusOut = "## master...origin/master [behind 5]";
+
+			const repos = await gitHelper.getRepos();
+			expect(repos[0]).toMatchSnapshot();
+			expect(execMock).toHaveBeenCalledTimes(7);
+		});
+
+		it("excludes repo if status can't be retrieved", async () => {
+			const errorMessage = "Failed to retrieve status";
+			execMock.mockRejectedValueOnce(errorMessage);
+
+			const repos = await gitHelper.getRepos();
+			expect(repos.length).toBe(0);
+
+			const { error } = require("logger");
+			expect(error).toHaveBeenCalledWith(`Failed to retrieve repo info for ${moduleName}: Failed to retrieve status`);
+		});
+	});
+
+	describe("MagicMirror on master without match in taglist", () => {
+		const moduleName = "MagicMirror";
+
+		beforeEach(async () => {
+			gitRemoteOut = "origin\tgit@github.com:MichMich/MagicMirror.git (fetch)\norigin\tgit@github.com:MichMich/MagicMirror.git (push)\n";
+			gitRevParseOut = "332e429a41f1a2339afd4f0ae96dd125da6beada";
+			gitStatusOut = "## master...origin/master\n M tests/unit/functions/updatenotification_spec.js\n";
+			gitFetchErr = "From github.com:MichMich/MagicMirror\n60e0377..332e429  master          -> origin/master\n";
+			gitRevListCountOut = "5";
+			gitTagListOut = "xxxe429a41f1a2339afd4f0ae96dd125da6beada...tag...\n";
+			gitRevListOut = "332e429a41f1a2339afd4f0ae96dd125da6beada\n";
+
+			await gitHelper.add(moduleName);
+		});
+
+		it("returns status information", async () => {
+			const repos = await gitHelper.getRepos();
+			expect(repos[0]).toMatchSnapshot();
+			expect(execMock).toHaveBeenCalledTimes(7);
+		});
+
+		it("returns status information early if isBehindInStatus", async () => {
+			gitStatusOut = "## master...origin/master [behind 5]";
+
+			const repos = await gitHelper.getRepos();
+			expect(repos[0]).toMatchSnapshot();
+			expect(execMock).toHaveBeenCalledTimes(7);
+		});
+
+		it("excludes repo if status can't be retrieved", async () => {
+			const errorMessage = "Failed to retrieve status";
+			execMock.mockRejectedValueOnce(errorMessage);
+
+			const repos = await gitHelper.getRepos();
+			expect(repos.length).toBe(0);
+
+			const { error } = require("logger");
+			expect(error).toHaveBeenCalledWith(`Failed to retrieve repo info for ${moduleName}: Failed to retrieve status`);
+		});
+	});
+
 	describe("custom module", () => {
 		const moduleName = "MMM-Fuel";
 
@@ -118,7 +239,7 @@ describe("Updatenotification", () => {
 			gitRevParseOut = "9d8310163da94441073a93cead711ba43e8888d0";
 			gitStatusOut = "## master...origin/master";
 			gitFetchErr = `From https://github.com/fewieden/${moduleName}\n19f7faf..9d83101  master      -> origin/master`;
-			gitRevListOut = "7";
+			gitRevListCountOut = "7";
 
 			await gitHelper.add(moduleName);
 		});