refactor(dev-infra): update commit message validation to return validation result (#38703)
Previously, the validateCommitMessage function ran validation and logged the results. The validateCommitMessage function now returns an object containing the validation results and the cli action functions are instead responsible for logging the results. This is being done as a prefactor for a change which allows for commit message hook validation to be either a blocking error or a warning. PR Close #38703
This commit is contained in:
Joey Perrott
@ -8,7 +8,7 @@
|
||||
|
||||
// Imports
|
||||
import * as validateConfig from './config';
|
||||
import {validateCommitMessage} from './validate';
|
||||
import {validateCommitMessage, ValidateCommitMessageResult} from './validate';
|
||||
|
||||
type CommitMessageConfig = validateConfig.CommitMessageConfig;
|
||||
|
||||
@ -31,44 +31,35 @@ const SCOPES = config.commitMessage.scopes.join(', ');
|
||||
const INVALID = false;
|
||||
const VALID = true;
|
||||
|
||||
function expectValidationResult(
|
||||
validationResult: ValidateCommitMessageResult, valid: boolean, errors: string[] = []) {
|
||||
expect(validationResult).toEqual(jasmine.objectContaining({valid, errors}));
|
||||
}
|
||||
|
||||
// TODO(josephperrott): Clean up tests to test script rather than for
|
||||
// specific commit messages we want to use.
|
||||
describe('validate-commit-message.js', () => {
|
||||
let lastError: string = '';
|
||||
|
||||
beforeEach(() => {
|
||||
lastError = '';
|
||||
|
||||
spyOn(console, 'error').and.callFake((msg: string) => lastError = msg);
|
||||
spyOn(validateConfig, 'getCommitMessageConfig')
|
||||
.and.returnValue(config as ReturnType<typeof validateConfig.getCommitMessageConfig>);
|
||||
});
|
||||
|
||||
describe('validateMessage()', () => {
|
||||
it('should be valid', () => {
|
||||
expect(validateCommitMessage('feat(packaging): something')).toBe(VALID);
|
||||
expect(lastError).toBe('');
|
||||
|
||||
expect(validateCommitMessage('fix(packaging): something')).toBe(VALID);
|
||||
expect(lastError).toBe('');
|
||||
|
||||
expect(validateCommitMessage('fixup! fix(packaging): something')).toBe(VALID);
|
||||
expect(lastError).toBe('');
|
||||
|
||||
expect(validateCommitMessage('squash! fix(packaging): something')).toBe(VALID);
|
||||
expect(lastError).toBe('');
|
||||
|
||||
expect(validateCommitMessage('Revert: "fix(packaging): something"')).toBe(VALID);
|
||||
expect(lastError).toBe('');
|
||||
expectValidationResult(validateCommitMessage('feat(packaging): something'), VALID);
|
||||
expectValidationResult(validateCommitMessage('fix(packaging): something'), VALID);
|
||||
expectValidationResult(validateCommitMessage('fixup! fix(packaging): something'), VALID);
|
||||
expectValidationResult(validateCommitMessage('squash! fix(packaging): something'), VALID);
|
||||
expectValidationResult(validateCommitMessage('Revert: "fix(packaging): something"'), VALID);
|
||||
});
|
||||
|
||||
it('should validate max length', () => {
|
||||
const msg =
|
||||
'fix(compiler): something super mega extra giga tera long, maybe even longer and longer and longer and longer and longer and longer...';
|
||||
|
||||
expect(validateCommitMessage(msg)).toBe(INVALID);
|
||||
expect(lastError).toContain(`The commit message header is longer than ${
|
||||
config.commitMessage.maxLineLength} characters`);
|
||||
expectValidationResult(validateCommitMessage(msg), INVALID, [
|
||||
`The commit message header is longer than ${config.commitMessage.maxLineLength} characters`
|
||||
]);
|
||||
});
|
||||
|
||||
it('should skip max length limit for URLs', () => {
|
||||
@ -77,49 +68,56 @@ describe('validate-commit-message.js', () => {
|
||||
'limit. For more details see the following super long URL:\n\n' +
|
||||
'https://github.com/angular/components/commit/e2ace018ddfad10608e0e32932c43dcfef4095d7#diff-9879d6db96fd29134fc802214163b95a';
|
||||
|
||||
expect(validateCommitMessage(msg)).toBe(VALID);
|
||||
expectValidationResult(validateCommitMessage(msg), VALID);
|
||||
});
|
||||
|
||||
it('should validate "<type>(<scope>): <subject>" format', () => {
|
||||
const msg = 'not correct format';
|
||||
|
||||
expect(validateCommitMessage(msg)).toBe(INVALID);
|
||||
expect(lastError).toContain(`The commit message header does not match the expected format.`);
|
||||
expectValidationResult(
|
||||
validateCommitMessage(msg), INVALID,
|
||||
[`The commit message header does not match the expected format.`]);
|
||||
});
|
||||
|
||||
it('should fail when type is invalid', () => {
|
||||
const msg = 'weird(core): something';
|
||||
|
||||
expect(validateCommitMessage(msg)).toBe(INVALID);
|
||||
expect(lastError).toContain(`'weird' is not an allowed type.\n => TYPES: ${TYPES}`);
|
||||
expectValidationResult(
|
||||
validateCommitMessage(msg), INVALID,
|
||||
[`'weird' is not an allowed type.\n => TYPES: ${TYPES}`]);
|
||||
});
|
||||
|
||||
it('should fail when scope is invalid', () => {
|
||||
const errorMessageFor = (scope: string, header: string) =>
|
||||
`'${scope}' is not an allowed scope.\n => SCOPES: ${SCOPES}`;
|
||||
|
||||
expect(validateCommitMessage('fix(Compiler): something')).toBe(INVALID);
|
||||
expect(lastError).toContain(errorMessageFor('Compiler', 'fix(Compiler): something'));
|
||||
expectValidationResult(
|
||||
validateCommitMessage('fix(Compiler): something'), INVALID,
|
||||
[errorMessageFor('Compiler', 'fix(Compiler): something')]);
|
||||
|
||||
expect(validateCommitMessage('feat(bah): something')).toBe(INVALID);
|
||||
expect(lastError).toContain(errorMessageFor('bah', 'feat(bah): something'));
|
||||
expectValidationResult(
|
||||
validateCommitMessage('feat(bah): something'), INVALID,
|
||||
[errorMessageFor('bah', 'feat(bah): something')]);
|
||||
|
||||
expect(validateCommitMessage('fix(webworker): something')).toBe(INVALID);
|
||||
expect(lastError).toContain(errorMessageFor('webworker', 'fix(webworker): something'));
|
||||
expectValidationResult(
|
||||
validateCommitMessage('fix(webworker): something'), INVALID,
|
||||
[errorMessageFor('webworker', 'fix(webworker): something')]);
|
||||
|
||||
expect(validateCommitMessage('refactor(security): something')).toBe(INVALID);
|
||||
expect(lastError).toContain(errorMessageFor('security', 'refactor(security): something'));
|
||||
expectValidationResult(
|
||||
validateCommitMessage('refactor(security): something'), INVALID,
|
||||
[errorMessageFor('security', 'refactor(security): something')]);
|
||||
|
||||
expect(validateCommitMessage('refactor(docs): something')).toBe(INVALID);
|
||||
expect(lastError).toContain(errorMessageFor('docs', 'refactor(docs): something'));
|
||||
expectValidationResult(
|
||||
validateCommitMessage('refactor(docs): something'), INVALID,
|
||||
[errorMessageFor('docs', 'refactor(docs): something')]);
|
||||
|
||||
expect(validateCommitMessage('feat(angular): something')).toBe(INVALID);
|
||||
expect(lastError).toContain(errorMessageFor('angular', 'feat(angular): something'));
|
||||
expectValidationResult(
|
||||
validateCommitMessage('feat(angular): something'), INVALID,
|
||||
[errorMessageFor('angular', 'feat(angular): something')]);
|
||||
});
|
||||
|
||||
it('should allow empty scope', () => {
|
||||
expect(validateCommitMessage('build: blablabla')).toBe(VALID);
|
||||
expect(lastError).toBe('');
|
||||
expectValidationResult(validateCommitMessage('build: blablabla'), VALID);
|
||||
});
|
||||
|
||||
// We do not want to allow WIP. It is OK to fail the PR build in this case to show that there is
|
||||
@ -127,30 +125,25 @@ describe('validate-commit-message.js', () => {
|
||||
it('should not allow "WIP: ..." syntax', () => {
|
||||
const msg = 'WIP: fix: something';
|
||||
|
||||
expect(validateCommitMessage(msg)).toBe(INVALID);
|
||||
expect(lastError).toContain(`'WIP' is not an allowed type.\n => TYPES: ${TYPES}`);
|
||||
expectValidationResult(
|
||||
validateCommitMessage(msg), INVALID,
|
||||
[`'WIP' is not an allowed type.\n => TYPES: ${TYPES}`]);
|
||||
});
|
||||
|
||||
describe('(revert)', () => {
|
||||
it('should allow valid "revert: ..." syntaxes', () => {
|
||||
expect(validateCommitMessage('revert: anything')).toBe(VALID);
|
||||
expect(lastError).toBe('');
|
||||
|
||||
expect(validateCommitMessage('Revert: "anything"')).toBe(VALID);
|
||||
expect(lastError).toBe('');
|
||||
|
||||
expect(validateCommitMessage('revert anything')).toBe(VALID);
|
||||
expect(lastError).toBe('');
|
||||
|
||||
expect(validateCommitMessage('rEvErT anything')).toBe(VALID);
|
||||
expect(lastError).toBe('');
|
||||
expectValidationResult(validateCommitMessage('revert: anything'), VALID);
|
||||
expectValidationResult(validateCommitMessage('Revert: "anything"'), VALID);
|
||||
expectValidationResult(validateCommitMessage('revert anything'), VALID);
|
||||
expectValidationResult(validateCommitMessage('rEvErT anything'), VALID);
|
||||
});
|
||||
|
||||
it('should not allow "revert(scope): ..." syntax', () => {
|
||||
const msg = 'revert(compiler): reduce generated code payload size by 65%';
|
||||
|
||||
expect(validateCommitMessage(msg)).toBe(INVALID);
|
||||
expect(lastError).toContain(`'revert' is not an allowed type.\n => TYPES: ${TYPES}`);
|
||||
expectValidationResult(
|
||||
validateCommitMessage(msg), INVALID,
|
||||
[`'revert' is not an allowed type.\n => TYPES: ${TYPES}`]);
|
||||
});
|
||||
|
||||
// https://github.com/angular/angular/issues/23479
|
||||
@ -158,28 +151,26 @@ describe('validate-commit-message.js', () => {
|
||||
const msg =
|
||||
'Revert "fix(compiler): Pretty print object instead of [Object object] (#22689)" (#23442)';
|
||||
|
||||
expect(validateCommitMessage(msg)).toBe(VALID);
|
||||
expect(lastError).toBe('');
|
||||
expectValidationResult(validateCommitMessage(msg), VALID);
|
||||
});
|
||||
});
|
||||
|
||||
describe('(squash)', () => {
|
||||
describe('without `disallowSquash`', () => {
|
||||
it('should return commits as valid', () => {
|
||||
expect(validateCommitMessage('squash! feat(core): add feature')).toBe(VALID);
|
||||
expect(validateCommitMessage('squash! fix: a bug')).toBe(VALID);
|
||||
expect(validateCommitMessage('squash! fix a typo')).toBe(VALID);
|
||||
expectValidationResult(validateCommitMessage('squash! feat(core): add feature'), VALID);
|
||||
expectValidationResult(validateCommitMessage('squash! fix: a bug'), VALID);
|
||||
expectValidationResult(validateCommitMessage('squash! fix a typo'), VALID);
|
||||
});
|
||||
});
|
||||
|
||||
describe('with `disallowSquash`', () => {
|
||||
it('should fail', () => {
|
||||
expect(validateCommitMessage('fix(core): something', {disallowSquash: true})).toBe(VALID);
|
||||
expect(validateCommitMessage('squash! fix(core): something', {
|
||||
disallowSquash: true
|
||||
})).toBe(INVALID);
|
||||
expect(lastError).toContain(
|
||||
'The commit must be manually squashed into the target commit');
|
||||
expectValidationResult(
|
||||
validateCommitMessage('fix(core): something', {disallowSquash: true}), VALID);
|
||||
expectValidationResult(
|
||||
validateCommitMessage('squash! fix(core): something', {disallowSquash: true}),
|
||||
INVALID, ['The commit must be manually squashed into the target commit']);
|
||||
});
|
||||
});
|
||||
});
|
||||
@ -187,9 +178,9 @@ describe('validate-commit-message.js', () => {
|
||||
describe('(fixup)', () => {
|
||||
describe('without `nonFixupCommitHeaders`', () => {
|
||||
it('should return commits as valid', () => {
|
||||
expect(validateCommitMessage('fixup! feat(core): add feature')).toBe(VALID);
|
||||
expect(validateCommitMessage('fixup! fix: a bug')).toBe(VALID);
|
||||
expect(validateCommitMessage('fixup! fixup! fix: a bug')).toBe(VALID);
|
||||
expectValidationResult(validateCommitMessage('fixup! feat(core): add feature'), VALID);
|
||||
expectValidationResult(validateCommitMessage('fixup! fix: a bug'), VALID);
|
||||
expectValidationResult(validateCommitMessage('fixup! fixup! fix: a bug'), VALID);
|
||||
});
|
||||
});
|
||||
|
||||
@ -197,36 +188,39 @@ describe('validate-commit-message.js', () => {
|
||||
it('should check that the fixup commit matches a non-fixup one', () => {
|
||||
const msg = 'fixup! foo';
|
||||
|
||||
expect(validateCommitMessage(
|
||||
msg, {disallowSquash: false, nonFixupCommitHeaders: ['foo', 'bar', 'baz']}))
|
||||
.toBe(VALID);
|
||||
expect(validateCommitMessage(
|
||||
msg, {disallowSquash: false, nonFixupCommitHeaders: ['bar', 'baz', 'foo']}))
|
||||
.toBe(VALID);
|
||||
expect(validateCommitMessage(
|
||||
msg, {disallowSquash: false, nonFixupCommitHeaders: ['baz', 'foo', 'bar']}))
|
||||
.toBe(VALID);
|
||||
expectValidationResult(
|
||||
validateCommitMessage(
|
||||
msg, {disallowSquash: false, nonFixupCommitHeaders: ['foo', 'bar', 'baz']}),
|
||||
VALID);
|
||||
expectValidationResult(
|
||||
validateCommitMessage(
|
||||
msg, {disallowSquash: false, nonFixupCommitHeaders: ['bar', 'baz', 'foo']}),
|
||||
VALID);
|
||||
expectValidationResult(
|
||||
validateCommitMessage(
|
||||
msg, {disallowSquash: false, nonFixupCommitHeaders: ['baz', 'foo', 'bar']}),
|
||||
VALID);
|
||||
|
||||
expect(validateCommitMessage(
|
||||
msg, {disallowSquash: false, nonFixupCommitHeaders: ['qux', 'quux', 'quuux']}))
|
||||
.toBe(INVALID);
|
||||
expect(lastError).toContain(
|
||||
'Unable to find match for fixup commit among prior commits: \n' +
|
||||
' qux\n' +
|
||||
' quux\n' +
|
||||
' quuux');
|
||||
expectValidationResult(
|
||||
validateCommitMessage(
|
||||
msg, {disallowSquash: false, nonFixupCommitHeaders: ['qux', 'quux', 'quuux']}),
|
||||
INVALID,
|
||||
['Unable to find match for fixup commit among prior commits: \n' +
|
||||
' qux\n' +
|
||||
' quux\n' +
|
||||
' quuux']);
|
||||
});
|
||||
|
||||
it('should fail if `nonFixupCommitHeaders` is empty', () => {
|
||||
expect(validateCommitMessage('refactor(core): make reactive', {
|
||||
disallowSquash: false,
|
||||
nonFixupCommitHeaders: []
|
||||
})).toBe(VALID);
|
||||
expect(validateCommitMessage(
|
||||
'fixup! foo', {disallowSquash: false, nonFixupCommitHeaders: []}))
|
||||
.toBe(INVALID);
|
||||
expect(lastError).toContain(
|
||||
`Unable to find match for fixup commit among prior commits: -`);
|
||||
expectValidationResult(
|
||||
validateCommitMessage(
|
||||
'refactor(core): make reactive',
|
||||
{disallowSquash: false, nonFixupCommitHeaders: []}),
|
||||
VALID);
|
||||
expectValidationResult(
|
||||
validateCommitMessage(
|
||||
'fixup! foo', {disallowSquash: false, nonFixupCommitHeaders: []}),
|
||||
INVALID, [`Unable to find match for fixup commit among prior commits: -`]);
|
||||
});
|
||||
});
|
||||
});
|
||||
@ -246,24 +240,27 @@ describe('validate-commit-message.js', () => {
|
||||
});
|
||||
|
||||
it('should fail validation if the body is shorter than `minBodyLength`', () => {
|
||||
expect(validateCommitMessage(
|
||||
'fix(core): something\n\n Explanation of the motivation behind this change'))
|
||||
.toBe(VALID);
|
||||
expect(validateCommitMessage('fix(core): something\n\n too short')).toBe(INVALID);
|
||||
expect(lastError).toContain(
|
||||
'The commit message body does not meet the minimum length of 30 characters');
|
||||
expect(validateCommitMessage('fix(core): something')).toBe(INVALID);
|
||||
expect(lastError).toContain(
|
||||
'The commit message body does not meet the minimum length of 30 characters');
|
||||
expectValidationResult(
|
||||
validateCommitMessage(
|
||||
'fix(core): something\n\n Explanation of the motivation behind this change'),
|
||||
VALID);
|
||||
expectValidationResult(
|
||||
validateCommitMessage('fix(core): something\n\n too short'), INVALID,
|
||||
['The commit message body does not meet the minimum length of 30 characters']);
|
||||
expectValidationResult(validateCommitMessage('fix(core): something'), INVALID, [
|
||||
|
||||
'The commit message body does not meet the minimum length of 30 characters'
|
||||
]);
|
||||
});
|
||||
|
||||
it('should pass validation if the body is shorter than `minBodyLength` but the commit type is in the `minBodyLengthTypeExclusions` list',
|
||||
() => {
|
||||
expect(validateCommitMessage('docs: just fixing a typo')).toBe(VALID);
|
||||
expect(validateCommitMessage('docs(core): just fixing a typo')).toBe(VALID);
|
||||
expect(validateCommitMessage(
|
||||
'docs(core): just fixing a typo\n\nThis was just a silly typo.'))
|
||||
.toBe(VALID);
|
||||
expectValidationResult(validateCommitMessage('docs: just fixing a typo'), VALID);
|
||||
expectValidationResult(validateCommitMessage('docs(core): just fixing a typo'), VALID);
|
||||
expectValidationResult(
|
||||
validateCommitMessage(
|
||||
'docs(core): just fixing a typo\n\nThis was just a silly typo.'),
|
||||
VALID);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user