From 58b9ddcd9f74ffd34b99337a9ac567441d4bed3c Mon Sep 17 00:00:00 2001 From: Karsten Hassel Date: Thu, 26 Jan 2023 13:21:30 +0100 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + .../default/updatenotification/git_helper.js | 25 ++- .../updatenotification_spec.js.snap | 70 +++++++- .../unit/functions/updatenotification_spec.js | 151 ++++++++++++++++-- 4 files changed, 229 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea876fcd..ad541cd5 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/modules/default/updatenotification/git_helper.js b/modules/default/updatenotification/git_helper.js index e793ebc9..30b0fe72 100644 --- a/modules/default/updatenotification/git_helper.js +++ b/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}`); diff --git a/tests/unit/functions/__snapshots__/updatenotification_spec.js.snap b/tests/unit/functions/__snapshots__/updatenotification_spec.js.snap index efe2a5a5..adacd250 100644 --- a/tests/unit/functions/__snapshots__/updatenotification_spec.js.snap +++ b/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", +} +`; diff --git a/tests/unit/functions/updatenotification_spec.js b/tests/unit/functions/updatenotification_spec.js index c3369a54..426154ec 100644 --- a/tests/unit/functions/updatenotification_spec.js +++ b/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); });