diff --git a/README.md b/README.md index e167fab..89af571 100644 --- a/README.md +++ b/README.md @@ -23,9 +23,9 @@ More information about `pattern` and `flags` can be found in the `flags` is optional and defaults to `gm`. -`excludeDescription`, `excludeTitle` and `checkAllCommitMessages` are optional. -Default behavior is to include the description and title and not check pull -request commit messages. +`excludeDescription`, `excludeTitle`, `excludeMergeCommits`, and +`checkAllCommitMessages` are optional. Default behavior is to include the +description, title, and merge commits, and not check pull request commit messages. ### Example Workflow @@ -67,8 +67,9 @@ jobs: error: 'The maximum line length of 74 characters is exceeded.' excludeDescription: 'true' # optional: this excludes the description body of a pull request excludeTitle: 'true' # optional: this excludes the title of a pull request + excludeMergeCommits: 'true' # optional: this excludes merge commits checkAllCommitMessages: 'true' # optional: this checks all commits associated with a pull request - accessToken: ${{ secrets.GITHUB_TOKEN }} # github access token is only required if checkAllCommitMessages is true + accessToken: ${{ secrets.GITHUB_TOKEN }} # github access token is only required if checkAllCommitMessages or excludeMergeCommits is true - name: Check for Resolves / Fixes uses: gsactions/commit-message-checker@v2 with: diff --git a/__tests__/commit-message-checker.test.ts b/__tests__/commit-message-checker.test.ts index b37d64a..cb1c53f 100644 --- a/__tests__/commit-message-checker.test.ts +++ b/__tests__/commit-message-checker.test.ts @@ -1,7 +1,7 @@ /* * This file is part of the "GS Commit Message Checker" Action for Github. * - * Copyright (C) 2019 by Gilbertsoft LLC (gilbertsoft.org) + * Copyright (C) 2019-2022 by Gilbertsoft LLC (gilbertsoft.org) * * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, diff --git a/__tests__/input-helper.test.ts b/__tests__/input-helper.test.ts index bb385c0..90c62b5 100644 --- a/__tests__/input-helper.test.ts +++ b/__tests__/input-helper.test.ts @@ -1,7 +1,7 @@ /* * This file is part of the "GS Commit Message Checker" Action for Github. * - * Copyright (C) 2019 by Gilbertsoft LLC (gilbertsoft.org) + * Copyright (C) 2019-2022 by Gilbertsoft LLC (gilbertsoft.org) * * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, @@ -40,10 +40,19 @@ const mockGitHub = jest.genMockFromModule('@actions/github') as any mockGitHub.context = {} // Mock @octokit/graphql +interface IGraphqlResponseGetter { + (parameters?: any): any +} let graphqlResponse = {} as any + +function defaultGraphqlResponseGetter(parameters?: any): any { + return graphqlResponse +} + +let getGraphqlResponse: IGraphqlResponseGetter = defaultGraphqlResponseGetter const mockGraphql = jest.genMockFromModule('@octokit/graphql') as any mockGraphql.graphql = (query: string, parameters?: any): any => { - return graphqlResponse + return getGraphqlResponse(parameters) } describe('input-helper tests', () => { @@ -62,6 +71,7 @@ describe('input-helper tests', () => { inputs = {} mockGitHub.context = {} graphqlResponse = {} + getGraphqlResponse = defaultGraphqlResponseGetter }) afterAll(() => { @@ -69,39 +79,20 @@ describe('input-helper tests', () => { jest.resetModules() }) - it('requires pattern', async () => { + it('inputs: requires pattern', async () => { await expect(inputHelper.getInputs()).rejects.toThrow( 'Input required and not supplied: pattern' ) }) - it('requires error message', async () => { + it('inputs: requires error message', async () => { inputs.pattern = 'some-pattern' await expect(inputHelper.getInputs()).rejects.toThrow( 'Input required and not supplied: error' ) }) - it('requires event', async () => { - inputs.pattern = 'some-pattern' - inputs.error = 'some-error' - await expect(inputHelper.getInputs()).rejects.toThrow( - 'Event "undefined" is not supported.' - ) - }) - - it('requires valid event', async () => { - mockGitHub.context = { - eventName: 'some-event' - } - inputs.pattern = 'some-pattern' - inputs.error = 'some-error' - await expect(inputHelper.getInputs()).rejects.toThrow( - 'Event "some-event" is not supported.' - ) - }) - - it('sets pattern', async () => { + it('inputs: sets pattern', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -118,7 +109,7 @@ describe('input-helper tests', () => { expect(checkerArguments.pattern).toBe('some-pattern') }) - it('sets flags', async () => { + it('inputs: sets flags', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -135,7 +126,7 @@ describe('input-helper tests', () => { expect(checkerArguments.flags).toBe('abcdefgh') }) - it('sets error', async () => { + it('inputs: sets error', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -152,7 +143,26 @@ describe('input-helper tests', () => { expect(checkerArguments.error).toBe('some-error') }) - it('requires pull_request payload', async () => { + it('event: requires event', async () => { + inputs.pattern = 'some-pattern' + inputs.error = 'some-error' + await expect(inputHelper.getInputs()).rejects.toThrow( + 'Event "undefined" is not supported.' + ) + }) + + it('event: requires valid event', async () => { + mockGitHub.context = { + eventName: 'some-event' + } + inputs.pattern = 'some-pattern' + inputs.error = 'some-error' + await expect(inputHelper.getInputs()).rejects.toThrow( + 'Event "some-event" is not supported.' + ) + }) + + it('pull_request: requires payload', async () => { mockGitHub.context = { eventName: 'pull_request' } @@ -163,7 +173,7 @@ describe('input-helper tests', () => { ) }) - it('requires pull_request', async () => { + it('pull_request: requires pull_request payload', async () => { mockGitHub.context = { eventName: 'pull_request', payload: {} @@ -175,7 +185,7 @@ describe('input-helper tests', () => { ) }) - it('requires pull_request title', async () => { + it('pull_request: requires title', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -192,7 +202,7 @@ describe('input-helper tests', () => { ) }) - it('sets pull_request title', async () => { + it('pull_request: sets title', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -210,7 +220,7 @@ describe('input-helper tests', () => { expect(checkerArguments.messages[0]).toBe('some-title') }) - it('sets pull_request title and body', async () => { + it('pull_request: sets title and body', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -228,7 +238,7 @@ describe('input-helper tests', () => { expect(checkerArguments.messages[0]).toBe('some-title\n\nsome-body') }) - it('excludes pull_request body', async () => { + it('pull_request: excludes body', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -247,7 +257,7 @@ describe('input-helper tests', () => { expect(checkerArguments.messages[0]).toBe('some-title') }) - it('excludes pull_request title', async () => { + it('pull_request: excludes title', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -266,7 +276,7 @@ describe('input-helper tests', () => { expect(checkerArguments.messages[0]).toBe('some-body') }) - it('excludes pull_request title and body', async () => { + it('pull_request: excludes title and body', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -286,7 +296,7 @@ describe('input-helper tests', () => { expect(checkerArguments.messages.length).toBe(0) }) - it('requires accessToken', async () => { + it('pull_request: requires accessToken', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -304,7 +314,7 @@ describe('input-helper tests', () => { ) }) - it('requires pull_request number', async () => { + it('pull_request: requires number', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -323,7 +333,7 @@ describe('input-helper tests', () => { ) }) - it('requires repository', async () => { + it('pull_request: requires repository', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -343,7 +353,7 @@ describe('input-helper tests', () => { ) }) - it('requires repository name', async () => { + it('pull_request: requires repository name', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -364,7 +374,7 @@ describe('input-helper tests', () => { ) }) - it('requires repository owner (1)', async () => { + it('pull_request: requires repository owner (1)', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -387,7 +397,7 @@ describe('input-helper tests', () => { ) }) - it('requires repository owner (2)', async () => { + it('pull_request: requires repository owner (2)', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -411,7 +421,7 @@ describe('input-helper tests', () => { ) }) - it('sets pull_request commits', async () => { + it('pull_request: sets commits', async () => { mockGitHub.context = { eventName: 'pull_request', payload: { @@ -445,7 +455,10 @@ describe('input-helper tests', () => { node: { commit: { message: - 'input: make input-helper functions async\n\nIn order to work with asynchronous call like an async http request\nin an easier way, the functions getInput and getMessages were\nconverted to async.' + 'input: make input-helper functions async\n\nIn order to work with asynchronous call like an async http request\nin an easier way, the functions getInput and getMessages were\nconverted to async.', + parents: { + totalCount: 1 + } } } }, @@ -453,7 +466,10 @@ describe('input-helper tests', () => { node: { commit: { message: - "input: PR options ignore title and check PR commits\n\nthis make it possible to igore partially or completely the PR payload.\nThe commits associated with the pull request can be checked instead of\nchecking the pull request payload. The parameter are:\n\n- excludeTitle: 'true | false'\n- excludeDescription: 'true | false'\n- checkAllCommitMessages: 'true | false'\n\nby default, all options comes false." + "input: PR options ignore title and check PR commits\n\nthis make it possible to igore partially or completely the PR payload.\nThe commits associated with the pull request can be checked instead of\nchecking the pull request payload. The parameter are:\n\n- excludeTitle: 'true | false'\n- excludeDescription: 'true | false'\n- checkAllCommitMessages: 'true | false'\n\nby default, all options comes false.", + parents: { + totalCount: 1 + } } } }, @@ -461,7 +477,20 @@ describe('input-helper tests', () => { node: { commit: { message: - 'docs: include parameters excludeTitle, checkAllCommitMessages and accessToken\n\nCo-authored-by: Gilbertsoft <25326036+gilbertsoft@users.noreply.github.com>' + 'docs: include parameters excludeTitle, checkAllCommitMessages and accessToken\n\nCo-authored-by: Gilbertsoft <25326036+gilbertsoft@users.noreply.github.com>', + parents: { + totalCount: 1 + } + } + } + }, + { + node: { + commit: { + message: 'merge: merge commit to be ignored', + parents: { + totalCount: 2 + } } } } @@ -478,10 +507,98 @@ describe('input-helper tests', () => { expect(checkerArguments.pattern).toBe('some-pattern') expect(checkerArguments.error).toBe('some-error') expect(checkerArguments.messages).toBeTruthy() + expect(checkerArguments.messages.length).toBe(4) + }) + + it('pull_request: excludes merge commits', async () => { + mockGitHub.context = { + eventName: 'pull_request', + payload: { + pull_request: { + title: 'some-title', + body: 'some-body', + number: 1 + }, + repository: { + owner: { + name: 'some-owner' + }, + name: 'some-repo' + } + } + } + + inputs.pattern = 'some-pattern' + inputs.error = 'some-error' + inputs.excludeDescription = 'true' + inputs.excludeTitle = 'true' + inputs.excludeMergeCommits = 'true' + inputs.checkAllCommitMessages = 'true' + inputs.accessToken = 'some-token' + + const response = { + repository: { + pullRequest: { + commits: { + edges: [ + { + node: { + commit: { + message: + 'input: make input-helper functions async\n\nIn order to work with asynchronous call like an async http request\nin an easier way, the functions getInput and getMessages were\nconverted to async.', + parents: { + totalCount: 1 + } + } + } + }, + { + node: { + commit: { + message: + "input: PR options ignore title and check PR commits\n\nthis make it possible to igore partially or completely the PR payload.\nThe commits associated with the pull request can be checked instead of\nchecking the pull request payload. The parameter are:\n\n- excludeTitle: 'true | false'\n- excludeDescription: 'true | false'\n- checkAllCommitMessages: 'true | false'\n\nby default, all options comes false.", + parents: { + totalCount: 1 + } + } + } + }, + { + node: { + commit: { + message: + 'docs: include parameters excludeTitle, checkAllCommitMessages and accessToken\n\nCo-authored-by: Gilbertsoft <25326036+gilbertsoft@users.noreply.github.com>', + parents: { + totalCount: 1 + } + } + } + }, + { + node: { + commit: { + message: 'merge: merge commit to be ignored', + parents: { + totalCount: 2 + } + } + } + } + ] + } + } + } + } + + graphqlResponse = response + + const checkerArguments: ICheckerArguments = await inputHelper.getInputs() + expect(checkerArguments).toBeTruthy() + expect(checkerArguments.messages).toBeTruthy() expect(checkerArguments.messages.length).toBe(3) }) - it('require push payload', async () => { + it('push: requires payload property', async () => { mockGitHub.context = { eventName: 'push' } @@ -492,7 +609,7 @@ describe('input-helper tests', () => { ) }) - it('push payload is optional', async () => { + it('push: payload content is optional', async () => { mockGitHub.context = { eventName: 'push', payload: {} @@ -503,7 +620,7 @@ describe('input-helper tests', () => { expect(checkerArguments.messages).toHaveLength(0) }) - it('push payload commits is optional', async () => { + it('push: payload commits is optional', async () => { mockGitHub.context = { eventName: 'push', payload: { @@ -516,12 +633,13 @@ describe('input-helper tests', () => { expect(checkerArguments.messages).toHaveLength(0) }) - it('sets correct single push payload', async () => { + it('push: requires repository', async () => { mockGitHub.context = { eventName: 'push', payload: { commits: [ { + id: '1', message: 'some-message' } ] @@ -529,36 +647,281 @@ describe('input-helper tests', () => { } inputs.pattern = 'some-pattern' inputs.error = 'some-error' + await expect(inputHelper.getInputs()).rejects.toThrow( + 'No repository found in the payload.' + ) + }) + + it('push: requires repository name', async () => { + mockGitHub.context = { + eventName: 'push', + payload: { + commits: [ + { + id: '1', + message: 'some-message' + } + ], + repository: {} + } + } + inputs.pattern = 'some-pattern' + inputs.error = 'some-error' + inputs.checkAllCommitMessages = 'true' + inputs.accessToken = 'dummy-token' + await expect(inputHelper.getInputs()).rejects.toThrow( + 'No name found in the repository.' + ) + }) + + it('push: requires repository owner (1)', async () => { + mockGitHub.context = { + eventName: 'push', + payload: { + commits: [ + { + id: '1', + message: 'some-message' + } + ], + repository: { + name: 'repository-name' + } + } + } + inputs.pattern = 'some-pattern' + inputs.error = 'some-error' + inputs.checkAllCommitMessages = 'true' + inputs.accessToken = 'dummy-token' + await expect(inputHelper.getInputs()).rejects.toThrow( + 'No owner found in the repository.' + ) + }) + + it('push: requires repository owner (2)', async () => { + mockGitHub.context = { + eventName: 'push', + payload: { + commits: [ + { + id: '1', + message: 'some-message' + } + ], + repository: { + name: 'repository-name', + owner: {} + } + } + } + inputs.pattern = 'some-pattern' + inputs.error = 'some-error' + inputs.checkAllCommitMessages = 'true' + inputs.accessToken = 'dummy-token' + await expect(inputHelper.getInputs()).rejects.toThrow( + 'No owner found in the repository.' + ) + }) + + it('push: sets single commit correctly', async () => { + mockGitHub.context = { + eventName: 'push', + payload: { + commits: [ + { + id: '1', + message: 'some-message' + } + ], + repository: { + owner: { + name: 'some-owner' + }, + name: 'some-repo' + } + } + } + + inputs.pattern = 'some-pattern' + inputs.error = 'some-error' + + const response = { + repository: { + object: { + message: 'some-message', + parents: { + totalCount: 1 + } + } + } + } + + graphqlResponse = response + const checkerArguments: ICheckerArguments = await inputHelper.getInputs() expect(checkerArguments).toBeTruthy() expect(checkerArguments.pattern).toBe('some-pattern') expect(checkerArguments.error).toBe('some-error') expect(checkerArguments.messages).toBeTruthy() + expect(checkerArguments.messages.length).toBe(1) expect(checkerArguments.messages[0]).toBe('some-message') }) - it('sets correct multiple push payload', async () => { + it('push: sets multiple commits correctly', async () => { mockGitHub.context = { eventName: 'push', payload: { commits: [ { + id: '1', message: 'some-message' }, { + id: '2', message: 'other-message' + }, + { + id: '3', + message: 'merge-commit-message' } - ] + ], + repository: { + owner: { + name: 'some-owner' + }, + name: 'some-repo' + } } } + inputs.pattern = 'some-pattern' inputs.error = 'some-error' + + getGraphqlResponse = (parameters?: any): any => { + if (parameters.commitSha === '3') { + return { + repository: { + object: { + message: 'merge-commit-message', + parents: { + totalCount: 2 + } + } + } + } + } + + return { + repository: { + object: { + parents: { + totalCount: 1 + } + } + } + } + } + const checkerArguments: ICheckerArguments = await inputHelper.getInputs() expect(checkerArguments).toBeTruthy() expect(checkerArguments.pattern).toBe('some-pattern') expect(checkerArguments.error).toBe('some-error') expect(checkerArguments.messages).toBeTruthy() + expect(checkerArguments.messages.length).toBe(3) expect(checkerArguments.messages[0]).toBe('some-message') expect(checkerArguments.messages[1]).toBe('other-message') }) + + it('push: requires accessToken to exclude merge commits', async () => { + mockGitHub.context = { + eventName: 'push', + payload: { + commits: [ + { + id: '1', + message: 'some-message' + } + ], + repository: { + owner: { + name: 'some-owner' + }, + name: 'some-repo' + } + } + } + + inputs.pattern = 'some-pattern' + inputs.error = 'some-error' + inputs.excludeMergeCommits = 'true' + + await expect(inputHelper.getInputs()).rejects.toThrow( + 'The `excludeMergeCommits` option requires a github access token.' + ) + }) + + it('push: excludes merge commits', async () => { + mockGitHub.context = { + eventName: 'push', + payload: { + commits: [ + { + id: '1', + message: 'merge-commit-message-1' + }, + { + id: '2', + message: 'some-message' + }, + { + id: '3', + message: 'merge-commit-message-2' + } + ], + repository: { + owner: { + name: 'some-owner' + }, + name: 'some-repo' + } + } + } + + inputs.pattern = 'some-pattern' + inputs.error = 'some-error' + inputs.excludeMergeCommits = 'true' + inputs.accessToken = 'some-token' + + getGraphqlResponse = (parameters?: any): any => { + if (parameters.commitSha === '2') { + return { + repository: { + object: { + message: 'some-message', + parents: { + totalCount: 1 + } + } + } + } + } + + return { + repository: { + object: { + parents: { + totalCount: 2 + } + } + } + } + } + + const checkerArguments: ICheckerArguments = await inputHelper.getInputs() + expect(checkerArguments).toBeTruthy() + expect(checkerArguments.pattern).toBe('some-pattern') + expect(checkerArguments.error).toBe('some-error') + expect(checkerArguments.messages).toBeTruthy() + expect(checkerArguments.messages.length).toBe(1) + }) }) diff --git a/action.yml b/action.yml index cbce124..9b7f5a7 100644 --- a/action.yml +++ b/action.yml @@ -20,6 +20,10 @@ inputs: description: 'Setting this input to true will exclude the Pull Request description from the check.' required: false default: 'false' + excludeMergeCommits: + description: 'Setting this input to true will exclude merge commits from the check.' + required: false + default: 'false' checkAllCommitMessages: description: 'Setting this input to true will check all Pull Request commits' required: false diff --git a/dist/index.js b/dist/index.js index e59aa2b..0ce76f4 100644 --- a/dist/index.js +++ b/dist/index.js @@ -206,6 +206,9 @@ function getInputs() { // Get excludeDescription const excludeDescriptionStr = core.getInput('excludeDescription'); core.debug(`excludeDescription: ${excludeDescriptionStr}`); + // Get excludeMergeCommits + const excludeMergeCommitsStr = core.getInput('excludeMergeCommits'); + core.debug(`excludeDescription: ${excludeMergeCommitsStr}`); // Get checkAllCommitMessages const checkAllCommitMessagesStr = core.getInput('checkAllCommitMessages'); core.debug(`checkAllCommitMessages: ${checkAllCommitMessagesStr}`); @@ -217,6 +220,9 @@ function getInputs() { ignoreDescription: excludeDescriptionStr ? excludeDescriptionStr === 'true' : /* default */ false, + ignoreMergeCommits: excludeMergeCommitsStr + ? excludeMergeCommitsStr === 'true' + : /* default */ false, checkAllCommitMessages: checkAllCommitMessagesStr ? checkAllCommitMessagesStr === 'true' : /* default */ false, @@ -236,7 +242,7 @@ exports.getInputs = getInputs; * @returns string[] */ function getMessages(pullRequestOptions) { - var _a; + var _a, _b; return __awaiter(this, void 0, void 0, function* () { core.debug('Get messages...'); core.debug(` - pullRequestOptions: ${JSON.stringify(pullRequestOptions, null, 2)}`); @@ -292,7 +298,7 @@ function getMessages(pullRequestOptions) { !github.context.payload.repository.owner.name)) { throw new Error('No owner found in the repository.'); } - const commitMessages = yield getCommitMessagesFromPullRequest(pullRequestOptions.accessToken, (_a = github.context.payload.repository.owner.name) !== null && _a !== void 0 ? _a : github.context.payload.repository.owner.login, github.context.payload.repository.name, github.context.payload.pull_request.number); + const commitMessages = yield getCommitMessagesFromPullRequest(pullRequestOptions.accessToken, (_a = github.context.payload.repository.owner.name) !== null && _a !== void 0 ? _a : github.context.payload.repository.owner.login, github.context.payload.repository.name, github.context.payload.pull_request.number, pullRequestOptions.ignoreMergeCommits); for (message of commitMessages) { if (message) { messages.push(message); @@ -310,8 +316,30 @@ function getMessages(pullRequestOptions) { core.debug(' - skipping commits'); break; } + if (!github.context.payload.repository) { + throw new Error('No repository found in the payload.'); + } + if (!github.context.payload.repository.name) { + throw new Error('No name found in the repository.'); + } + if (!github.context.payload.repository.owner || + (!github.context.payload.repository.owner.login && + !github.context.payload.repository.owner.name)) { + throw new Error('No owner found in the repository.'); + } + if (pullRequestOptions.ignoreMergeCommits) { + if (!pullRequestOptions.accessToken) { + throw new Error('The `excludeMergeCommits` option requires a github access token.'); + } + } for (const i in github.context.payload.commits) { if (github.context.payload.commits[i].message) { + // ignore merge commits if requested + if (pullRequestOptions.ignoreMergeCommits && + (yield isMergeCommit(pullRequestOptions.accessToken, (_b = github.context.payload.repository.owner.name) !== null && _b !== void 0 ? _b : github.context.payload.repository.owner.login, github.context.payload.repository.name, github.context.payload.commits[i].id))) { + core.debug(` - skipping merge commit ${github.context.payload.commits[i].id}`); + continue; + } messages.push(github.context.payload.commits[i].message); } } @@ -324,7 +352,7 @@ function getMessages(pullRequestOptions) { return messages; }); } -function getCommitMessagesFromPullRequest(accessToken, repositoryOwner, repositoryName, pullRequestNumber) { +function getCommitMessagesFromPullRequest(accessToken, repositoryOwner, repositoryName, pullRequestNumber, ignoreMergeCommits) { return __awaiter(this, void 0, void 0, function* () { core.debug('Get messages from pull request...'); core.debug(` - accessToken: ${accessToken}`); @@ -344,7 +372,10 @@ function getCommitMessagesFromPullRequest(accessToken, repositoryOwner, reposito edges { node { commit { - message + message, + parents(last: 1) { + totalCount + } } } } @@ -369,13 +400,59 @@ function getCommitMessagesFromPullRequest(accessToken, repositoryOwner, reposito core.debug(` - response: ${JSON.stringify(repository, null, 2)}`); let messages = []; if (repository.pullRequest) { - messages = repository.pullRequest.commits.edges.map(function (edge) { + messages = repository.pullRequest.commits.edges + .filter(function (edge) { + // Skip merge commits (which have more than 1 parent commit) + return !ignoreMergeCommits || edge.node.commit.parents.totalCount === 1; + }) + .map(function (edge) { return edge.node.commit.message; }); } return messages; }); } +function isMergeCommit(accessToken, repositoryOwner, repositoryName, commitSha) { + return __awaiter(this, void 0, void 0, function* () { + core.debug('Get messages from pull request...'); + core.debug(` - accessToken: ${accessToken}`); + core.debug(` - repositoryOwner: ${repositoryOwner}`); + core.debug(` - repositoryName: ${repositoryName}`); + core.debug(` - commitSha: ${commitSha}`); + const query = ` + query commit( + $repositoryOwner: String!, + $repositoryName: String!, + $commitSha: GitObjectID! + ) { + repository(owner: $repositoryOwner, name: $repositoryName) { + object(oid: $commitSha) { + ... on Commit { + message + parents(last: 1) { + totalCount + } + } + } + } + } +`; + const variables = { + baseUrl: process.env['GITHUB_API_URL'] || 'https://api.github.com', + repositoryOwner, + repositoryName, + commitSha, + headers: { + authorization: `token ${accessToken}` + } + }; + core.debug(` - query: ${query}`); + core.debug(` - variables: ${JSON.stringify(variables, null, 2)}`); + const response = yield (0, graphql_1.graphql)(query, variables); + core.debug(` - response: ${JSON.stringify(response, null, 2)}`); + return response.repository.object.parents.totalCount > 1; + }); +} /***/ }), diff --git a/package.json b/package.json index 68967c8..b6f2562 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ "format-check": "prettier --check '**/*.ts'", "lint": "eslint src/**/*.ts", "package": "ncc build", - "test": "jest", + "test": "jest --coverage", "all": "npm run build && npm run format && npm run lint && npm run package && npm test" }, "dependencies": { diff --git a/src/input-helper.ts b/src/input-helper.ts index eff4be3..158ed4a 100644 --- a/src/input-helper.ts +++ b/src/input-helper.ts @@ -26,6 +26,7 @@ import {ICheckerArguments} from './commit-message-checker' export interface PullRequestOptions { ignoreTitle: boolean ignoreDescription: boolean + ignoreMergeCommits: boolean // requires github token checkAllCommitMessages: boolean // requires github token accessToken: string } @@ -60,6 +61,10 @@ export async function getInputs(): Promise { const excludeDescriptionStr = core.getInput('excludeDescription') core.debug(`excludeDescription: ${excludeDescriptionStr}`) + // Get excludeMergeCommits + const excludeMergeCommitsStr = core.getInput('excludeMergeCommits') + core.debug(`excludeDescription: ${excludeMergeCommitsStr}`) + // Get checkAllCommitMessages const checkAllCommitMessagesStr = core.getInput('checkAllCommitMessages') core.debug(`checkAllCommitMessages: ${checkAllCommitMessagesStr}`) @@ -72,6 +77,9 @@ export async function getInputs(): Promise { ignoreDescription: excludeDescriptionStr ? excludeDescriptionStr === 'true' : /* default */ false, + ignoreMergeCommits: excludeMergeCommitsStr + ? excludeMergeCommitsStr === 'true' + : /* default */ false, checkAllCommitMessages: checkAllCommitMessagesStr ? checkAllCommitMessagesStr === 'true' : /* default */ false, @@ -175,7 +183,8 @@ async function getMessages( github.context.payload.repository.owner.name ?? github.context.payload.repository.owner.login, github.context.payload.repository.name, - github.context.payload.pull_request.number + github.context.payload.pull_request.number, + pullRequestOptions.ignoreMergeCommits ) for (message of commitMessages) { @@ -200,8 +209,49 @@ async function getMessages( break } + if (!github.context.payload.repository) { + throw new Error('No repository found in the payload.') + } + + if (!github.context.payload.repository.name) { + throw new Error('No name found in the repository.') + } + + if ( + !github.context.payload.repository.owner || + (!github.context.payload.repository.owner.login && + !github.context.payload.repository.owner.name) + ) { + throw new Error('No owner found in the repository.') + } + + if (pullRequestOptions.ignoreMergeCommits) { + if (!pullRequestOptions.accessToken) { + throw new Error( + 'The `excludeMergeCommits` option requires a github access token.' + ) + } + } + for (const i in github.context.payload.commits) { if (github.context.payload.commits[i].message) { + // ignore merge commits if requested + if ( + pullRequestOptions.ignoreMergeCommits && + (await isMergeCommit( + pullRequestOptions.accessToken, + github.context.payload.repository.owner.name ?? + github.context.payload.repository.owner.login, + github.context.payload.repository.name, + github.context.payload.commits[i].id + )) + ) { + core.debug( + ` - skipping merge commit ${github.context.payload.commits[i].id}` + ) + continue + } + messages.push(github.context.payload.commits[i].message) } } @@ -220,7 +270,8 @@ async function getCommitMessagesFromPullRequest( accessToken: string, repositoryOwner: string, repositoryName: string, - pullRequestNumber: number + pullRequestNumber: number, + ignoreMergeCommits: boolean ): Promise { core.debug('Get messages from pull request...') core.debug(` - accessToken: ${accessToken}`) @@ -241,7 +292,10 @@ async function getCommitMessagesFromPullRequest( edges { node { commit { - message + message, + parents(last: 1) { + totalCount + } } } } @@ -267,6 +321,9 @@ async function getCommitMessagesFromPullRequest( node: { commit: { message: string + parents: { + totalCount: number + } } } } @@ -289,12 +346,76 @@ async function getCommitMessagesFromPullRequest( let messages: string[] = [] if (repository.pullRequest) { - messages = repository.pullRequest.commits.edges.map(function ( - edge: CommitEdgeItem - ): string { - return edge.node.commit.message - }) + messages = repository.pullRequest.commits.edges + .filter(function (edge: CommitEdgeItem): boolean { + // Skip merge commits (which have more than 1 parent commit) + return !ignoreMergeCommits || edge.node.commit.parents.totalCount === 1 + }) + .map(function (edge: CommitEdgeItem): string { + return edge.node.commit.message + }) } return messages } + +async function isMergeCommit( + accessToken: string, + repositoryOwner: string, + repositoryName: string, + commitSha: string +): Promise { + core.debug('Get messages from pull request...') + core.debug(` - accessToken: ${accessToken}`) + core.debug(` - repositoryOwner: ${repositoryOwner}`) + core.debug(` - repositoryName: ${repositoryName}`) + core.debug(` - commitSha: ${commitSha}`) + + const query = ` + query commit( + $repositoryOwner: String!, + $repositoryName: String!, + $commitSha: GitObjectID! + ) { + repository(owner: $repositoryOwner, name: $repositoryName) { + object(oid: $commitSha) { + ... on Commit { + message + parents(last: 1) { + totalCount + } + } + } + } + } +` + const variables = { + baseUrl: process.env['GITHUB_API_URL'] || 'https://api.github.com', + repositoryOwner, + repositoryName, + commitSha, + headers: { + authorization: `token ${accessToken}` + } + } + + core.debug(` - query: ${query}`) + core.debug(` - variables: ${JSON.stringify(variables, null, 2)}`) + + interface CommitResponseData { + repository: { + object: { + message: string + parents: { + totalCount: number + } + } + } + } + + const response = await graphql(query, variables) + + core.debug(` - response: ${JSON.stringify(response, null, 2)}`) + + return response.repository.object.parents.totalCount > 1 +}