From 6e0fcb1270630772518ceb183d388776739701da Mon Sep 17 00:00:00 2001 From: Wagner Santos Date: Sat, 1 Feb 2020 08:43:41 -0300 Subject: [PATCH] fix: don't fail on warnings This follows the commitlint CLI behavior, which exits with success when there are only warnings. This behavior can be changed by passing 'true' to the parameter `failOnWarnings`. --- README.md | 12 +++++- action.js | 55 +++++++++++++++++++------- action.test.js | 102 ++++++++++++++++++++++++++++++++++++++++++------- action.yml | 4 ++ 4 files changed, 144 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 4e8fae2..195faa4 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,9 @@ Alternatively, you can run on other event types such as `on: [push]`. In that ca ### `configFile` -The path to your commitlint config file. Default `commitlint.config.js`. +The path to your commitlint config file. + +Default: `commitlint.config.js` ### `firstParent` @@ -39,7 +41,13 @@ When set to true, we follow only the first parent commit when seeing a merge com This helps to ignore errors in commits that were already present in your default branch (e.g. `master`) before adding conventional commit checks. More info in [git-log docs](https://git-scm.com/docs/git-log#Documentation/git-log.txt---first-parent). -Default `true` +Default: `true` + +### `failOnWarnings` + +Whether you want to fail on warnings or not. + +Default: `false` ## About `extends` in your config file diff --git a/action.js b/action.js index b16f317..ce2f1eb 100644 --- a/action.js +++ b/action.js @@ -101,16 +101,8 @@ function getOptsFromConfig(config) { } } -const showLintResults = async ([from, to]) => { - const commits = await getHistoryCommits(from, to) - const config = existsSync(configPath) - ? await load({}, { file: configPath }) - : {} - const opts = getOptsFromConfig(config) - const results = await Promise.all( - commits.map(commit => lint(commit, config.rules, opts)), - ) - const formattedResults = format( +const formatErrors = results => + format( { results }, { color: true, @@ -119,10 +111,45 @@ const showLintResults = async ([from, to]) => { }, ) - if (formattedResults) { - core.setFailed( - `You have commit messages with errors\n\n${formattedResults}`, - ) +const hasOnlyWarnings = results => { + const resultsWithOnlyWarnings = results.filter( + result => result.valid && result.warnings.length, + ) + + return ( + resultsWithOnlyWarnings.length && + resultsWithOnlyWarnings.length === results.length + ) +} + +const setFailed = formattedResults => { + core.setFailed(`You have commit messages with errors\n\n${formattedResults}`) +} + +const handleOnlyWarnings = formattedResults => { + if (core.getInput('failOnWarnings') === 'true') { + setFailed(formattedResults) + } else { + console.log(`You have commit messages with warnings\n\n${formattedResults}`) + } +} + +const showLintResults = async ([from, to]) => { + const failOnWarnings = core.getInput('failOnWarnings') + const commits = await getHistoryCommits(from, to) + const config = existsSync(configPath) + ? await load({}, { file: configPath }) + : {} + const opts = getOptsFromConfig(config) + const results = await Promise.all( + commits.map(commit => lint(commit, config.rules, opts)), + ) + const formattedResults = formatErrors(results) + + if (hasOnlyWarnings(results)) { + handleOnlyWarnings(formattedResults) + } else if (formattedResults) { + setFailed(formattedResults) } else { console.log('Lint free! 🎉') } diff --git a/action.test.js b/action.test.js index 6921cda..b49caba 100644 --- a/action.test.js +++ b/action.test.js @@ -37,6 +37,7 @@ const runAction = () => { describe('Commit Linter action', () => { let core + let cwd beforeEach(() => { core = require('@actions/core') @@ -44,6 +45,7 @@ describe('Commit Linter action', () => { td.replace(core, 'setFailed') td.when(core.getInput('configFile')).thenReturn('./commitlint.config.js') td.when(core.getInput('firstParent')).thenReturn('true') + td.when(core.getInput('failOnWarnings')).thenReturn('false') }) afterEach(() => { @@ -53,7 +55,7 @@ describe('Commit Linter action', () => { }) it('should fail for single push with incorrect message', async () => { - const cwd = await git.bootstrap('fixtures/conventional') + cwd = await git.bootstrap('fixtures/conventional') await gitEmptyCommit(cwd, 'wrong message') const [to] = await getCommitHashes(cwd) await createPushEventPayload(cwd, { to }) @@ -66,7 +68,7 @@ describe('Commit Linter action', () => { }) it('should pass for single push with correct message', async () => { - const cwd = await git.bootstrap('fixtures/conventional') + cwd = await git.bootstrap('fixtures/conventional') await gitEmptyCommit(cwd, 'chore: correct message') const [to] = await getCommitHashes(cwd) await createPushEventPayload(cwd, { to }) @@ -81,7 +83,7 @@ describe('Commit Linter action', () => { }) it('should fail for push range with wrong messages', async () => { - const cwd = await git.bootstrap('fixtures/conventional') + cwd = await git.bootstrap('fixtures/conventional') await gitEmptyCommit(cwd, 'message from before push') await gitEmptyCommit(cwd, 'wrong message 1') await gitEmptyCommit(cwd, 'wrong message 2') @@ -97,7 +99,7 @@ describe('Commit Linter action', () => { }) it('should pass for push range with correct messages', async () => { - const cwd = await git.bootstrap('fixtures/conventional') + cwd = await git.bootstrap('fixtures/conventional') await gitEmptyCommit(cwd, 'message from before push') await gitEmptyCommit(cwd, 'chore: correct message 1') await gitEmptyCommit(cwd, 'chore: correct message 2') @@ -114,7 +116,7 @@ describe('Commit Linter action', () => { }) it('should lint only last commit for forced push', async () => { - const cwd = await git.bootstrap('fixtures/conventional') + cwd = await git.bootstrap('fixtures/conventional') await gitEmptyCommit(cwd, 'message from before push') await gitEmptyCommit(cwd, 'wrong message 1') await gitEmptyCommit(cwd, 'wrong message 2') @@ -137,7 +139,7 @@ describe('Commit Linter action', () => { it('should lint only last commit when "before" field is an empty sha', async () => { const gitEmptySha = '0000000000000000000000000000000000000000' - const cwd = await git.bootstrap('fixtures/conventional') + cwd = await git.bootstrap('fixtures/conventional') await gitEmptyCommit(cwd, 'message from before push') await gitEmptyCommit(cwd, 'wrong message 1') await gitEmptyCommit(cwd, 'chore(WRONG): message 2') @@ -153,7 +155,7 @@ describe('Commit Linter action', () => { }) it('should fail for commit with scope that is not a lerna package', async () => { - const cwd = await git.bootstrap('fixtures/lerna-scopes') + cwd = await git.bootstrap('fixtures/lerna-scopes') td.when(core.getInput('configFile')).thenReturn('./commitlint.config.yml') await gitEmptyCommit(cwd, 'chore(wrong): not including package scope') const [to] = await getCommitHashes(cwd) @@ -169,7 +171,7 @@ describe('Commit Linter action', () => { }) it('should pass for scope that is a lerna package', async () => { - const cwd = await git.bootstrap('fixtures/lerna-scopes') + cwd = await git.bootstrap('fixtures/lerna-scopes') td.when(core.getInput('configFile')).thenReturn('./commitlint.config.yml') await gitEmptyCommit(cwd, 'chore(second-package): this works') const [to] = await getCommitHashes(cwd) @@ -184,7 +186,7 @@ describe('Commit Linter action', () => { }) it("should fail for commit that doesn't comply with jira rules", async () => { - const cwd = await git.bootstrap('fixtures/jira') + cwd = await git.bootstrap('fixtures/jira') td.when(core.getInput('configFile')).thenReturn('./commitlint.config.js') await gitEmptyCommit(cwd, 'ib-21212121212121: without jira ticket') const [to] = await getCommitHashes(cwd) @@ -217,7 +219,7 @@ describe('Commit Linter action', () => { }) it('should NOT consider commits from another branch', async () => { - const cwd = await git.bootstrap('fixtures/conventional') + cwd = await git.bootstrap('fixtures/conventional') await gitEmptyCommit(cwd, 'chore: commit before') await gitEmptyCommit(cwd, 'chore: correct message') await execa.command('git checkout -b another-branch', { cwd }) @@ -236,7 +238,7 @@ describe('Commit Linter action', () => { }) it('should consider commits from another branch when firstParent is false', async () => { - const cwd = await git.bootstrap('fixtures/conventional') + cwd = await git.bootstrap('fixtures/conventional') await gitEmptyCommit(cwd, 'chore: commit before') await gitEmptyCommit(cwd, 'chore: correct message') await execa.command('git checkout -b another-branch', { cwd }) @@ -255,7 +257,7 @@ describe('Commit Linter action', () => { }) it('should lint all commits from a pull request', async () => { - const cwd = await git.bootstrap('fixtures/conventional') + cwd = await git.bootstrap('fixtures/conventional') td.when(core.getInput('configFile')).thenReturn('./commitlint.config.js') await gitEmptyCommit(cwd, 'message from before push') await gitEmptyCommit(cwd, 'wrong message 1') @@ -286,7 +288,7 @@ describe('Commit Linter action', () => { }) it('should show an error message when failing to fetch commits', async () => { - const cwd = await git.bootstrap('fixtures/conventional') + cwd = await git.bootstrap('fixtures/conventional') td.when(core.getInput('configFile')).thenReturn('./commitlint.config.js') await gitEmptyCommit(cwd, 'commit message') await createPullRequestEventPayload(cwd) @@ -310,4 +312,78 @@ describe('Commit Linter action', () => { ) td.verify(core.setFailed(contains('HttpError: Bad credentials'))) }) + + describe('when all errors are just warnings', () => { + beforeEach(async () => { + cwd = await git.bootstrap('fixtures/conventional') + await gitEmptyCommit( + cwd, + 'chore: correct message\nsome context without leading blank line', + ) + const [to] = await getCommitHashes(cwd) + await createPushEventPayload(cwd, { to }) + updatePushEnvVars(cwd, to) + td.replace(process, 'cwd', () => cwd) + td.replace(console, 'log') + }) + + it('should pass and show that warnings exist', async () => { + await runAction() + + td.verify(core.setFailed(), { times: 0, ignoreExtraArgs: true }) + td.verify(console.log(contains('You have commit messages with warnings'))) + }) + + describe('and failOnWarnings is set to true', () => { + beforeEach(() => { + td.when(core.getInput('failOnWarnings')).thenReturn('true') + }) + + it('should fail', async () => { + await runAction() + + td.verify( + core.setFailed(contains('You have commit messages with errors')), + ) + }) + }) + }) + + describe('when a subset of errors are just warnings', () => { + beforeEach(async () => { + cwd = await git.bootstrap('fixtures/conventional') + await gitEmptyCommit( + cwd, + 'chore: correct message\nsome context without leading blank line', + ) + await gitEmptyCommit(cwd, 'wrong message') + const [before, to] = await getCommitHashes(cwd) + await createPushEventPayload(cwd, { before, to }) + updatePushEnvVars(cwd, to) + td.replace(process, 'cwd', () => cwd) + td.replace(console, 'log') + }) + + it('should fail', async () => { + await runAction() + + td.verify( + core.setFailed(contains('You have commit messages with errors')), + ) + }) + + describe('and failOnWarnings is set to true', () => { + beforeEach(() => { + td.when(core.getInput('failOnWarnings')).thenReturn('true') + }) + + it('should fail', async () => { + await runAction() + + td.verify( + core.setFailed(contains('You have commit messages with errors')), + ) + }) + }) + }) }) diff --git a/action.yml b/action.yml index 77832d2..2050a81 100644 --- a/action.yml +++ b/action.yml @@ -10,6 +10,10 @@ inputs: description: 'when set to true, we follow only the first parent commit when seeing a merge commit. More info in git-log docs https://git-scm.com/docs/git-log#Documentation/git-log.txt---first-parent' default: 'true' required: false + failOnWarnings: + description: 'whether you want to fail on warnings or not' + default: 'false' + required: false runs: using: 'docker' image: 'docker://wagoid/commitlint-github-action:1.3.2'