From 9c57061a726019c44cb20f3db5fbd65bba628e2c Mon Sep 17 00:00:00 2001 From: Steffen Diswal <11992328+spdiswal@users.noreply.github.com> Date: Sun, 5 Apr 2026 23:33:25 +0200 Subject: [PATCH 1/3] Move test subject out of `it` blocks in rule tests Clearly separating the test subjects from the assertions makes the unit tests slightly more readable. This approach is possible without using a `beforeEach` hook because all test subjects (i.e. rules) are pure functions. --- src/domains/rules/NoSquashMarkers.tests.ts | 30 ++++++++++------ .../rules/UseCapitalisedSubjectLines.tests.ts | 36 ++++++++++++------- 2 files changed, 44 insertions(+), 22 deletions(-) diff --git a/src/domains/rules/NoSquashMarkers.tests.ts b/src/domains/rules/NoSquashMarkers.tests.ts index ef81250e..5ce44b10 100644 --- a/src/domains/rules/NoSquashMarkers.tests.ts +++ b/src/domains/rules/NoSquashMarkers.tests.ts @@ -34,8 +34,9 @@ describe.each` const commit = fakeCommit({ message: props.subjectLine }) describe("and the rule is enabled", () => { + const actualConcerns = noSquashMarkers([commit], enabled.options) + it("raises a concern about the squash marker", () => { - const actualConcerns = noSquashMarkers([commit], enabled.options) expect(actualConcerns).toEqual([ subjectLineConcern(enabled, commit.sha, { range: props.expectedRange }), ]) @@ -43,8 +44,9 @@ describe.each` }) describe("and the rule is disabled", () => { + const actualConcerns = noSquashMarkers([commit], disabled.options) + it("does not raise any concerns", () => { - const actualConcerns = noSquashMarkers([commit], disabled.options) expect(actualConcerns).toEqual([]) }) }) @@ -62,8 +64,9 @@ describe.each` const commit = fakeCommit({ message: props.subjectLine }) describe("and the rule is enabled", () => { + const actualConcerns = noSquashMarkers([commit], enabled.options) + it("raises a concern about all squash markers", () => { - const actualConcerns = noSquashMarkers([commit], enabled.options) expect(actualConcerns).toEqual([ subjectLineConcern(enabled, commit.sha, { range: props.expectedRange }), ]) @@ -71,8 +74,9 @@ describe.each` }) describe("and the rule is disabled", () => { + const actualConcerns = noSquashMarkers([commit], disabled.options) + it("does not raise any concerns", () => { - const actualConcerns = noSquashMarkers([commit], disabled.options) expect(actualConcerns).toEqual([]) }) }) @@ -110,15 +114,17 @@ describe.each` const commit = fakeCommit({ message: props.subjectLine }) describe("and the rule is enabled", () => { + const actualConcerns = noSquashMarkers([commit], enabled.options) + it("does not raise any concerns", () => { - const actualConcerns = noSquashMarkers([commit], enabled.options) expect(actualConcerns).toEqual([]) }) }) describe("and the rule is disabled", () => { + const actualConcerns = noSquashMarkers([commit], disabled.options) + it("does not raise any concerns", () => { - const actualConcerns = noSquashMarkers([commit], disabled.options) expect(actualConcerns).toEqual([]) }) }) @@ -137,8 +143,9 @@ describe("when verifying a set of multiple commits and some commits have squash ] describe("and the rule is enabled", () => { + const actualConcerns = noSquashMarkers(commits, enabled.options) + it("raises concerns about the commits with squash markers", () => { - const actualConcerns = noSquashMarkers(commits, enabled.options) expect(actualConcerns).toEqual([ subjectLineConcern(enabled, commits[0].sha, { range: [0, 6] }), subjectLineConcern(enabled, commits[1].sha, { range: [0, 7] }), @@ -148,8 +155,9 @@ describe("when verifying a set of multiple commits and some commits have squash }) describe("and the rule is disabled", () => { + const actualConcerns = noSquashMarkers(commits, disabled.options) + it("does not raise any concerns", () => { - const actualConcerns = noSquashMarkers(commits, disabled.options) expect(actualConcerns).toEqual([]) }) }) @@ -165,15 +173,17 @@ describe("when verifying a set of multiple commits and no commits have squash ma ] describe("and the rule is enabled", () => { + const actualConcerns = noSquashMarkers(commits, enabled.options) + it("does not raise any concerns", () => { - const actualConcerns = noSquashMarkers(commits, enabled.options) expect(actualConcerns).toEqual([]) }) }) describe("and the rule is disabled", () => { + const actualConcerns = noSquashMarkers(commits, disabled.options) + it("does not raise any concerns", () => { - const actualConcerns = noSquashMarkers(commits, disabled.options) expect(actualConcerns).toEqual([]) }) }) diff --git a/src/domains/rules/UseCapitalisedSubjectLines.tests.ts b/src/domains/rules/UseCapitalisedSubjectLines.tests.ts index 3c00a573..294e4bc1 100644 --- a/src/domains/rules/UseCapitalisedSubjectLines.tests.ts +++ b/src/domains/rules/UseCapitalisedSubjectLines.tests.ts @@ -36,8 +36,9 @@ describe.each` const commit = fakeCommit({ message: props.subjectLine }) describe("and the rule is enabled", () => { + const actualConcerns = useCapitalisedSubjectLines([commit], enabled.options) + it("raises a concern about the first non-capitalised character", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], enabled.options) expect(actualConcerns).toEqual([ subjectLineConcern(enabled, commit.sha, { range: props.expectedRange }), ]) @@ -45,8 +46,9 @@ describe.each` }) describe("and the rule is disabled", () => { + const actualConcerns = useCapitalisedSubjectLines([commit], disabled.options) + it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], disabled.options) expect(actualConcerns).toEqual([]) }) }) @@ -75,15 +77,17 @@ describe.each` const commit = fakeCommit({ message: props.subjectLine }) describe("and the rule is enabled", () => { + const actualConcerns = useCapitalisedSubjectLines([commit], enabled.options) + it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], enabled.options) expect(actualConcerns).toEqual([]) }) }) describe("and the rule is disabled", () => { + const actualConcerns = useCapitalisedSubjectLines([commit], disabled.options) + it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], disabled.options) expect(actualConcerns).toEqual([]) }) }) @@ -104,15 +108,17 @@ describe.each` const commit = fakeCommit({ message: props.subjectLine }) describe("and the rule is enabled", () => { + const actualConcerns = useCapitalisedSubjectLines([commit], enabled.options) + it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], enabled.options) expect(actualConcerns).toEqual([]) }) }) describe("and the rule is disabled", () => { + const actualConcerns = useCapitalisedSubjectLines([commit], disabled.options) + it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], disabled.options) expect(actualConcerns).toEqual([]) }) }) @@ -130,15 +136,17 @@ describe.each` const commit = fakeCommit({ message: props.subjectLine }) describe("and the rule is enabled", () => { + const actualConcerns = useCapitalisedSubjectLines([commit], enabled.options) + it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], enabled.options) expect(actualConcerns).toEqual([]) }) }) describe("and the rule is disabled", () => { + const actualConcerns = useCapitalisedSubjectLines([commit], disabled.options) + it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines([commit], disabled.options) expect(actualConcerns).toEqual([]) }) }) @@ -158,8 +166,9 @@ describe("when verifying a set of multiple commits and some commits have non-cap ] describe("and the rule is enabled", () => { + const actualConcerns = useCapitalisedSubjectLines(commits, enabled.options) + it("raises concerns about the commits with non-capitalised subject lines", () => { - const actualConcerns = useCapitalisedSubjectLines(commits, enabled.options) expect(actualConcerns).toEqual([ subjectLineConcern(enabled, commits[0].sha, { range: [0, 1] }), subjectLineConcern(enabled, commits[3].sha, { range: [0, 1] }), @@ -169,8 +178,9 @@ describe("when verifying a set of multiple commits and some commits have non-cap }) describe("and the rule is disabled", () => { + const actualConcerns = useCapitalisedSubjectLines(commits, disabled.options) + it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines(commits, disabled.options) expect(actualConcerns).toEqual([]) }) }) @@ -185,15 +195,17 @@ describe("when verifying a set of multiple commits and all commits have capitali ] describe("and the rule is enabled", () => { + const actualConcerns = useCapitalisedSubjectLines(commits, enabled.options) + it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines(commits, enabled.options) expect(actualConcerns).toEqual([]) }) }) describe("and the rule is disabled", () => { + const actualConcerns = useCapitalisedSubjectLines(commits, disabled.options) + it("does not raise any concerns", () => { - const actualConcerns = useCapitalisedSubjectLines(commits, disabled.options) expect(actualConcerns).toEqual([]) }) }) From b309485d3f28ec7ac6273fb524ebeb007c21acd0 Mon Sep 17 00:00:00 2001 From: Steffen Diswal <11992328+spdiswal@users.noreply.github.com> Date: Mon, 6 Apr 2026 00:28:03 +0200 Subject: [PATCH 2/3] Test revert markers with inconsistent quote pairs --- .../commits/tokens/RevertMarkerToken.tests.ts | 25 +++++++++++++++++-- .../commits/tokens/RevertMarkerToken.ts | 3 ++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/domains/commits/tokens/RevertMarkerToken.tests.ts b/src/domains/commits/tokens/RevertMarkerToken.tests.ts index 8a05256f..9ff90d9e 100644 --- a/src/domains/commits/tokens/RevertMarkerToken.tests.ts +++ b/src/domains/commits/tokens/RevertMarkerToken.tests.ts @@ -18,6 +18,7 @@ describe.each` ${'revert "Fix a nasty bug"'} | ${[revertMarker('revert "'), 'Fix a nasty bug"']} ${'REVERT "Refactor the authentication module"'} | ${[revertMarker('REVERT "'), 'Refactor the authentication module"']} ${' Revert " Apply strawberry jam to make the code sweeter" '} | ${[revertMarker(' Revert "'), ' Apply strawberry jam to make the code sweeter" ']} + ${' revert " revert "Find a new court jester to blame " " '} | ${[revertMarker(' revert " revert "'), 'Find a new court jester to blame " " ']} ${'Revert "Make the program act like a clown"'} | ${[revertMarker('Revert "'), 'Make the program act like a clown"']} ${'Revert "Upgrade React to 19.2.0 (#42)"'} | ${[revertMarker('Revert "'), "Upgrade React to ", dependencyVersion("19.2.0"), issueLink(" (#42)"), '"']} ${'fixup! Revert "Add an amazing feature"'} | ${[squashMarker("fixup! "), revertMarker('Revert "'), 'Add an amazing feature"']} @@ -34,6 +35,25 @@ describe.each` }, ) +describe.each` + subjectLine | expectedTokens + ${'Revert "Repair the soft ice machine'} | ${[revertMarker('Revert "'), "Repair the soft ice machine"]} + ${'Revert "Revert "Repair the soft ice machine"'} | ${[revertMarker('Revert "Revert "'), 'Repair the soft ice machine"']} + ${'Revert "Revert "Revert "Repair the soft ice machine'} | ${[revertMarker('Revert "Revert "Revert "'), "Repair the soft ice machine"]} + ${' revert " revert "Find a new court jester to blame " '} | ${[revertMarker(' revert " revert "'), 'Find a new court jester to blame " ']} + ${'fixup! Revert "Add an amazing feature'} | ${[squashMarker("fixup! "), revertMarker('Revert "'), "Add an amazing feature"]} +`( + "when the subject line of $subjectLine contains revert markers with inconsistent pairs of quotes", + (props: { subjectLine: string; expectedTokens: TokenisedLine }) => { + const crudeCommit = fakeCrudeCommit({ message: props.subjectLine }) + + it("extracts revert marker tokens", () => { + const commit = mapCrudeCommitToCommit(crudeCommit, configuration) + expect(commit.subjectLine).toEqual(props.expectedTokens) + }) + }, +) + describe.each` subjectLine | expectedTokens ${'#1 Revert "Add an amazing feature"'} | ${[issueLink("#1 "), 'Revert "Add an amazing feature"']} @@ -58,8 +78,9 @@ describe.each` ${'Revert "'} ${'Revert ""'} ${"Revert 'the next big thing'"} - ${"Revert some more stuff"} - ${"Reverted the secret stuff"} + ${'revert more stuff"'} + ${"Reverted some secret stuff"} + ${'Revert ""weirdly quoted message'} ${'"Revert "Make the formatter happy again""'} ${'Revert"without-space"'} ${'fix: Revert "something"'} diff --git a/src/domains/commits/tokens/RevertMarkerToken.ts b/src/domains/commits/tokens/RevertMarkerToken.ts index c525e885..f4f75c6b 100644 --- a/src/domains/commits/tokens/RevertMarkerToken.ts +++ b/src/domains/commits/tokens/RevertMarkerToken.ts @@ -13,7 +13,8 @@ export function isRevertMarker(token: Token): token is RevertMarkerToken { return typeof token === "object" && token.type === "revert-marker" } -const regex = /^\s*(?:revert\s+")+(?=[^"])/iu +// Assume all revert markers to contain a double quote `"` followed by a non-quote character (quote pair consistency not enforced for simplicity). +const regex = /^(?:\s*revert\s+")+(?=[^"])/iu export function tokeniseRevertMarkers(initialTokens: TokenisedLine): TokenisedLine { const result: TokenisedLine = [] From c0e5cf11415ba208a72b9e16807c8508d848185b Mon Sep 17 00:00:00 2001 From: Steffen Diswal <11992328+spdiswal@users.noreply.github.com> Date: Mon, 6 Apr 2026 00:38:58 +0200 Subject: [PATCH 3/3] Implement `noRevertRevertCommits` --- .../rules/NoRevertRevertCommits.tests.ts | 210 ++++++++++++++++++ src/domains/rules/NoRevertRevertCommits.ts | 39 +++- .../rules/reports/CommitwiseReport.tests.ts | 80 +++++-- src/domains/rules/reports/CommitwiseReport.ts | 2 +- 4 files changed, 308 insertions(+), 23 deletions(-) create mode 100644 src/domains/rules/NoRevertRevertCommits.tests.ts diff --git a/src/domains/rules/NoRevertRevertCommits.tests.ts b/src/domains/rules/NoRevertRevertCommits.tests.ts new file mode 100644 index 00000000..9caad641 --- /dev/null +++ b/src/domains/rules/NoRevertRevertCommits.tests.ts @@ -0,0 +1,210 @@ +import { describe, expect, it } from "vitest" +import { fakeCommitFactory } from "#commits/Commit.fixtures.ts" +import type { Commit } from "#commits/Commit.ts" +import { fakeConfiguration } from "#configurations/Configuration.fixtures.ts" +import type { Concerns } from "#rules/concerns/Concern.ts" +import { subjectLineConcern } from "#rules/concerns/SubjectLineConcern.ts" +import { noRevertRevertCommits } from "#rules/NoRevertRevertCommits.ts" +import { ruleContext } from "#rules/Rule.ts" +import type { CharacterRange } from "#types/CharacterRange.ts" +import type { Vector } from "#types/Vector.ts" + +const enabled = ruleContext("noRevertRevertCommits", {}) +const disabled = ruleContext("noRevertRevertCommits", null) + +const fakeCommit = fakeCommitFactory(fakeConfiguration()) + +describe.each` + subjectLine | expectedRange + ${'Revert "Revert "Fix the bug""'} | ${[0, 16]} + ${'Revert "revert "Repair the soft ice machine""'} | ${[0, 17]} + ${' revert " revert "Apply strawberry jam to make the code sweeter " " '} | ${[1, 18]} + ${' rEvErT "REVERT "Refactor the authentication module""'} | ${[2, 20]} + ${'Revert "Revert "Revert "Fix the nasty bug"""'} | ${[0, 24]} + ${' revert "revert "revert "revert "Repair the soft ice machine """"'} | ${[1, 34]} +`( + "when the subject line of $subjectLine contains a revert marker with 2 or more 'revert' occurrences", + (props: { subjectLine: string; expectedRange: CharacterRange }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe("and the rule is enabled", () => { + const actualConcerns = noRevertRevertCommits([commit], enabled.options) + + it("raises a concern about the revert marker", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(enabled, commit.sha, { range: props.expectedRange }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = noRevertRevertCommits([commit], disabled.options) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine + ${'Revert "Repair the soft ice machine"'} + ${'Revert "Fix a nasty bug"'} + ${'revert "Fix a nasty bug"'} + ${'REVERT "Refactor the authentication module"'} + ${' Revert "Apply strawberry jam to make the code sweeter" '} + ${'Revert "Make the program act like a clown"'} + ${'fixup! Revert "Add an amazing feature"'} +`( + "when the subject line of $subjectLine contains a revert marker with 1 'revert' occurrence", + (props: { subjectLine: string }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe("and the rule is enabled", () => { + const actualConcerns = noRevertRevertCommits([commit], enabled.options) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = noRevertRevertCommits([commit], disabled.options) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine + ${'fixup! Revert "Revert "Add an amazing feature""'} + ${' squash!amend! revert "revert "retrieve data from the exclusive third-party service""'} +`( + "when the subject line of $subjectLine contains a squash marker and a revert marker with 2 or more 'revert' occurrences", + (props: { subjectLine: string }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe("and the rule is enabled", () => { + const actualConcerns = noRevertRevertCommits([commit], enabled.options) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = noRevertRevertCommits([commit], disabled.options) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe.each` + subjectLine + ${""} + ${" "} + ${"\t\t"} + ${"Refactor the taxi module"} + ${" Unsubscribe from the service"} + ${"Formatting."} + ${"WIP"} + ${"Fix the bug"} + ${" Hunt down the bugs "} + ${"fixup! Add an amazing feature"} + ${"squash! Make the program act like a clown"} + ${'#7044: Revert "Unsolve the problem"'} + ${"Time to revert it"} + ${'Not a Revert "thing"'} + ${"Revert the last change"} + ${"Reverted some secret stuff"} + ${'fix: Revert "something"'} +`( + "when the subject line of $subjectLine does not contain a revert marker", + (props: { subjectLine: string }) => { + const commit = fakeCommit({ message: props.subjectLine }) + + describe("and the rule is enabled", () => { + const actualConcerns = noRevertRevertCommits([commit], enabled.options) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = noRevertRevertCommits([commit], disabled.options) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + }, +) + +describe("when verifying a set of multiple commits and some commits have revert markers with 2 or more 'revert' occurrences", () => { + const commits: Vector = [ + fakeCommit({ message: 'Revert "Revert "Fix the bug""' }), + fakeCommit({ message: 'Revert "Repair the soft ice machine"' }), + fakeCommit({ message: "Refactor the taxi module" }), + fakeCommit({ message: 'squash! Revert "Revert "Fix the bug""' }), + fakeCommit({ message: "WIP" }), + fakeCommit({ message: ' revert "revert "WIP""' }), + fakeCommit({ message: 'Revert "Revert "Revert "Add an amazing feature"""' }), + fakeCommit({ message: " Unsubscribe from the service" }), + fakeCommit({ message: "Fix the bug" }), + ] + + describe("and the rule is enabled", () => { + const actualConcerns = noRevertRevertCommits(commits, enabled.options) + + it("raises concerns about the commits with double revert markers", () => { + expect(actualConcerns).toEqual([ + subjectLineConcern(enabled, commits[0].sha, { range: [0, 16] }), + subjectLineConcern(enabled, commits[5].sha, { range: [1, 17] }), + subjectLineConcern(enabled, commits[6].sha, { range: [0, 24] }), + ]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = noRevertRevertCommits(commits, disabled.options) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) +}) + +describe("when verifying a set of multiple commits and no commits have revert markers with 2 or more 'revert' occurrences", () => { + const commits: Vector = [ + fakeCommit({ message: 'Revert "Repair the soft ice machine"' }), + fakeCommit({ message: "Fix the bug" }), + fakeCommit({ message: 'fixup! Revert "Repair the soft ice machine"' }), + fakeCommit({ message: "Time to revert it" }), + fakeCommit({ message: 'Revert "Time to revert it"' }), + fakeCommit({ message: "1 2 3 this is a test" }), + ] + + describe("and the rule is enabled", () => { + const actualConcerns = noRevertRevertCommits(commits, enabled.options) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) + + describe("and the rule is disabled", () => { + const actualConcerns = noRevertRevertCommits(commits, disabled.options) + + it("does not raise any concerns", () => { + expect(actualConcerns).toEqual([]) + }) + }) +}) diff --git a/src/domains/rules/NoRevertRevertCommits.ts b/src/domains/rules/NoRevertRevertCommits.ts index ed74f101..e3ae3de3 100644 --- a/src/domains/rules/NoRevertRevertCommits.ts +++ b/src/domains/rules/NoRevertRevertCommits.ts @@ -1,7 +1,38 @@ -import type { Commits } from "#commits/Commit.ts" -import type { Concerns } from "#rules/concerns/Concern.ts" +import type { Commit, Commits } from "#commits/Commit.ts" +import { isRevertMarker } from "#commits/tokens/RevertMarkerToken.ts" +import { isSquashMarker } from "#commits/tokens/SquashMarkerToken.ts" +import type { Concern, Concerns } from "#rules/concerns/Concern.ts" +import { subjectLineConcern } from "#rules/concerns/SubjectLineConcern.ts" +import { type RuleContext, ruleContext } from "#rules/Rule.ts" import type { EmptyObject } from "#types/EmptyObject.ts" +import { notNullish } from "#utilities/Arrays.ts" -export function noRevertRevertCommits(_commits: Commits, _options: EmptyObject | null): Concerns { - throw new Error("The `noRevertRevertCommits` rule has not been implemented yet") // TODO: To be implemented. +export function noRevertRevertCommits(commits: Commits, options: EmptyObject | null): Concerns { + const rule = ruleContext("noRevertRevertCommits", options) + + return options !== null + ? commits.map((commit) => verifyCommit(commit, rule)).filter(notNullish) + : [] +} + +function verifyCommit(commit: Commit, rule: RuleContext): Concern | null { + for (const token of commit.subjectLine) { + if (isSquashMarker(token)) { + return null + } + if (isRevertMarker(token)) { + const revertOccurrences = (token.value.match(/revert/giu) ?? []).length + + if (revertOccurrences > 1) { + const leadingWhitespaceOffset = token.value.length - token.value.trimStart().length + const trimmedTokenLength = token.value.trim().length + + return subjectLineConcern(rule, commit.sha, { + range: [leadingWhitespaceOffset, leadingWhitespaceOffset + trimmedTokenLength], + }) + } + } + } + + return null } diff --git a/src/domains/rules/reports/CommitwiseReport.tests.ts b/src/domains/rules/reports/CommitwiseReport.tests.ts index 5d725a19..cd85dbe4 100644 --- a/src/domains/rules/reports/CommitwiseReport.tests.ts +++ b/src/domains/rules/reports/CommitwiseReport.tests.ts @@ -21,45 +21,45 @@ describe("when there are no concerns", () => { }) }) -describe("when 'useCapitalisedSubjectLines' has a concern about characters 0-1", () => { +describe("when 'noRevertRevertCommits' has a concern about characters 0-16", () => { const commit = fakeCommit({ - sha: "497de39943643a56f7a69d3d19723e3035318644", - message: "release the robot butler", + sha: "d4e7a978cea34b727ea52f90c928fd535e4aee", + message: 'Revert "Revert "Make the program act like a clown""', }) - const concern = subjectLineConcern(inRule("useCapitalisedSubjectLines"), commit.sha, { - range: [0, 1], + const concern = subjectLineConcern(inRule("noRevertRevertCommits"), commit.sha, { + range: [0, 16], }) it("describes the rule violation in the subject line", () => { const actualOutput = commitwiseReport([commit], [concern]) expect(actualOutput).toBe( ` -497de39 release the robot butler - ┬ - ╰─ The first letter in subject lines must be in uppercase. - (useCapitalisedSubjectLines) +d4e7a97 Revert "Revert "Make the program act like a clown"" + ───────┬──────── + ╰─ Cherry-pick the original commit instead of reverting it over. + (noRevertRevertCommits) `.trim(), ) }) }) -describe("when 'useCapitalisedSubjectLines' has a concern about characters 7-8", () => { +describe("when 'noRevertRevertCommits' has a concern about characters 1-26", () => { const commit = fakeCommit({ - sha: "92d6b11650c6b63d64fd77522241b7f50ff5b", - message: "fixup! resolve a bug that thought it was a feature", + sha: "34aa41b818c40682cabeecd5623dfe51df7a4a5", + message: ' revert "revert "revert "repair the soft ice machine """', }) - const concern = subjectLineConcern(inRule("useCapitalisedSubjectLines"), commit.sha, { - range: [7, 8], + const concern = subjectLineConcern(inRule("noRevertRevertCommits"), commit.sha, { + range: [1, 26], }) it("describes the rule violation in the subject line", () => { const actualOutput = commitwiseReport([commit], [concern]) expect(actualOutput).toBe( ` -92d6b11 fixup! resolve a bug that thought it was a feature - ┬ - ╰─ The first letter in subject lines must be in uppercase. - (useCapitalisedSubjectLines) +34aa41b revert "revert "revert "repair the soft ice machine """ + ────────────┬──────────── + ╰─ Cherry-pick the original commit instead of reverting it over. + (noRevertRevertCommits) `.trim(), ) }) @@ -105,6 +105,50 @@ describe("when 'noSquashMarkers' has a concern about characters 1-14", () => { }) }) +describe("when 'useCapitalisedSubjectLines' has a concern about characters 0-1", () => { + const commit = fakeCommit({ + sha: "497de39943643a56f7a69d3d19723e3035318644", + message: "release the robot butler", + }) + const concern = subjectLineConcern(inRule("useCapitalisedSubjectLines"), commit.sha, { + range: [0, 1], + }) + + it("describes the rule violation in the subject line", () => { + const actualOutput = commitwiseReport([commit], [concern]) + expect(actualOutput).toBe( + ` +497de39 release the robot butler + ┬ + ╰─ The first letter in subject lines must be in uppercase. + (useCapitalisedSubjectLines) +`.trim(), + ) + }) +}) + +describe("when 'useCapitalisedSubjectLines' has a concern about characters 7-8", () => { + const commit = fakeCommit({ + sha: "92d6b11650c6b63d64fd77522241b7f50ff5b", + message: "fixup! resolve a bug that thought it was a feature", + }) + const concern = subjectLineConcern(inRule("useCapitalisedSubjectLines"), commit.sha, { + range: [7, 8], + }) + + it("describes the rule violation in the subject line", () => { + const actualOutput = commitwiseReport([commit], [concern]) + expect(actualOutput).toBe( + ` +92d6b11 fixup! resolve a bug that thought it was a feature + ┬ + ╰─ The first letter in subject lines must be in uppercase. + (useCapitalisedSubjectLines) +`.trim(), + ) + }) +}) + describe.todo("when there are multiple concerns of different types", () => { const commits: Vector = [fakeCommit(), fakeCommit(), fakeCommit()] const concerns: Concerns = [] diff --git a/src/domains/rules/reports/CommitwiseReport.ts b/src/domains/rules/reports/CommitwiseReport.ts index fd21bedc..4e0cca25 100644 --- a/src/domains/rules/reports/CommitwiseReport.ts +++ b/src/domains/rules/reports/CommitwiseReport.ts @@ -71,7 +71,7 @@ function formatRule(rule: RuleContext): string { throw new Error(`Not implemented yet: ${rule.key}`) } case "noRevertRevertCommits": { - throw new Error(`Not implemented yet: ${rule.key}`) + return "Cherry-pick the original commit instead of reverting it over." } case "noSingleWordSubjectLines": { throw new Error(`Not implemented yet: ${rule.key}`)