Skip to content

Commit

Permalink
updatenotification: only notify mm-repo for master if tag present (#3004
Browse files Browse the repository at this point in the history 
)

see discussion here:
#2991 (comment)

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>
  • Loading branch information
@khassel @rejas
khassel and rejas authored Jan 26, 2023
1 parent 7198ae5 commit 58b9ddc
Show file tree
Hide file tree
Showing 4 changed files with 229 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 24 additions & 1 deletion modules/default/updatenotification/git_helper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ class GitHelper {
return;
}

if (gitInfo.isBehindInStatus) {
if (gitInfo.isBehindInStatus && (gitInfo.module !== "MagicMirror" || gitInfo.current !== "master")) {
return gitInfo;
}

Expand All @@ -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}`);
Expand Down
70 changes: 68 additions & 2 deletions tests/unit/functions/__snapshots__/updatenotification_spec.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
}
`;
151 changes: 136 additions & 15 deletions tests/unit/functions/updatenotification_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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 };
}
});
});
Expand All @@ -71,15 +71,15 @@ describe("Updatenotification", () => {
jest.clearAllMocks();
});

describe("MagicMirror", () => {
describe("MagicMirror on develop", () => {
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 = "## 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);
});
Expand Down Expand Up @@ -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";

Expand All @@ -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);
});
Expand Down

0 comments on commit 58b9ddc

Please sign in to comment.