test: clean up and re-organize `validate-commit-message` tests (#32023)

Mainly making the tests more closely follow the order of checks in the
function implementation, so that it is easier to follow.

PR Close #32023

tools/validate-commit-message/validate-commit-message.spec.js

@@ -6,32 +6,35 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-describe('validate-commit-message.js', function() {
+// Imports
-  var validateMessage = require('./validate-commit-message');
+const validateMessage = require('./validate-commit-message');
-  var SCOPES = validateMessage.config.scopes.join(', ');
-  var TYPES = validateMessage.config.types.join(', ');
-  var errors = [];
-  var logs = [];
 
-  var VALID = true;
+// Constants
-  var INVALID = false;
+const TYPES = validateMessage.config.types.join(', ');
+const SCOPES = validateMessage.config.scopes.join(', ');
 
-  beforeEach(function() {
+const INVALID = false;
-    errors.length = 0;
+const VALID = true;
-    logs.length = 0;
 
-    spyOn(console, 'error').and.callFake(function(msg) {
-      errors.push(msg.replace(/\x1B\[\d+m/g, ''));  // uncolor
-    });
 
-    spyOn(console, 'log').and.callFake(function(msg) {
+describe('validate-commit-message.js', () => {
-      logs.push(msg.replace(/\x1B\[\d+m/g, ''));  // uncolor
+  let errors = [];
-    });
+  let logs = [];
+
+  // Helpers
+  const stripColor = msg => msg.replace(/\x1B\[\d+m/g, '');
+
+  beforeEach(() => {
+    errors = [];
+    logs = [];
+
+    spyOn(console, 'error').and.callFake(msg => errors.push(stripColor(msg)));
+    spyOn(console, 'log').and.callFake(msg => logs.push(stripColor(msg)));
   });
 
-  describe('validateMessage', function() {
+  describe('validateMessage()', () => {
 
-    it('should be valid', function() {
+    it('should be valid', () => {
       expect(validateMessage('fix(core): something')).toBe(VALID);
       expect(validateMessage('feat(common): something')).toBe(VALID);
       expect(validateMessage('docs(compiler): something')).toBe(VALID);
@@ -49,91 +52,97 @@ describe('validate-commit-message.js', function() {
       expect(errors).toEqual([]);
     });
 
-    it('should fail when scope is invalid', function() {
+    it('should validate max length', () => {
+      var msg =
+          'fix(compiler): something super mega extra giga tera long, maybe even longer and longer and longer and longer and longer and longer...';
+
+      expect(validateMessage(msg)).toBe(INVALID);
+      expect(errors).toEqual([
+        `INVALID COMMIT MSG: "${msg}"\n => ERROR: The commit message is longer than 120 characters`
+      ]);
+    });
+
+    it('should validate "<type>(<scope>): <subject>" format', () => {
+      const msg = 'not correct format';
+
+      expect(validateMessage(msg)).toBe(INVALID);
+      expect(errors).toEqual([
+        `INVALID COMMIT MSG: "${msg}"\n => ERROR: The commit message does not match the format of '<type>(<scope>): <subject>' OR 'Revert: "type(<scope>): <subject>"'`,
+      ]);
+    });
+
+    it('should fail when type is invalid', () => {
+      const msg = 'weird(common): something';
+
+      expect(validateMessage(msg)).toBe(INVALID);
+      expect(errors).toEqual([
+        `INVALID COMMIT MSG: "${msg}"\n => ERROR: weird is not an allowed type.\n => TYPES: ${TYPES}`,
+      ]);
+    });
+
+    it('should fail when scope is invalid', () => {
+      const errorMessageFor = (scope, header) =>
+          `INVALID COMMIT MSG: "${header}"\n => ERROR: "${scope}" is not an allowed scope.\n => SCOPES: ${SCOPES}`;
+
       expect(validateMessage('fix(Compiler): something')).toBe(INVALID);
       expect(validateMessage('feat(bah): something')).toBe(INVALID);
       expect(validateMessage('style(webworker): something')).toBe(INVALID);
       expect(validateMessage('refactor(security): something')).toBe(INVALID);
       expect(validateMessage('refactor(docs): something')).toBe(INVALID);
-      ['INVALID COMMIT MSG: "fix(Compiler): something"\n' +
-           ' => ERROR: "Compiler" is not an allowed scope.\n' +
-           ' => SCOPES: ' + SCOPES,
-       'INVALID COMMIT MSG: "feat(bah): something"\n' +
-           ' => ERROR: "bah" is not an allowed scope.\n' +
-           ' => SCOPES: ' + SCOPES,
-       'INVALID COMMIT MSG: "style(webworker): something"\n' +
-           ' => ERROR: "webworker" is not an allowed scope.\n' +
-           ' => SCOPES: ' + SCOPES,
-       'INVALID COMMIT MSG: "refactor(security): something"\n' +
-           ' => ERROR: "security" is not an allowed scope.\n' +
-           ' => SCOPES: ' + SCOPES,
-       'INVALID COMMIT MSG: "refactor(docs): something"\n' +
-           ' => ERROR: "docs" is not an allowed scope.\n' +
-           ' => SCOPES: ' + SCOPES,
-      ].forEach((expectedErrorMessage, index) => {
-        expect(expectedErrorMessage).toEqual(errors[index]);
-      });
       expect(validateMessage('release(angular): something')).toBe(INVALID);
-    });
-
-
-    it('should validate 100 characters length', function() {
-      var msg =
-          'fix(compiler): something super mega extra giga tera long, maybe even longer and longer and longer and longer and longer and longer... ';
-
-      expect(validateMessage(msg)).toBe(INVALID);
       expect(errors).toEqual([
-        'INVALID COMMIT MSG: "fix(compiler): something super mega extra giga tera long, maybe even longer and longer and longer and longer and longer and longer... "\n => ERROR: The commit message is longer than 120 characters'
+        errorMessageFor('Compiler', 'fix(Compiler): something'),
+        errorMessageFor('bah', 'feat(bah): something'),
+        errorMessageFor('webworker', 'style(webworker): something'),
+        errorMessageFor('security', 'refactor(security): something'),
+        errorMessageFor('docs', 'refactor(docs): something'),
+        errorMessageFor('angular', 'release(angular): something'),
       ]);
     });
 
+    it('should allow empty scope', () => {
+      expect(validateMessage('fix: blablabla')).toBe(VALID);
+      expect(errors).toEqual([]);
+    });
 
-    it('should validate "<type>(<scope>): <subject>" format', function() {
+    // We do not want to allow WIP. It is OK to fail the PR build in this case to show that there is
-      var msg = 'not correct format';
+    // work still to be done (i.e. fixing the commit message).
+    it('should not allow "WIP: ..." syntax', () => {
+      const msg = 'WIP: fix: something';
 
       expect(validateMessage(msg)).toBe(INVALID);
       expect(errors).toEqual([
-        'INVALID COMMIT MSG: "not correct format"\n => ERROR: The commit message does not match the format of \'<type>(<scope>): <subject>\' OR \'Revert: "type(<scope>): <subject>"\''
+        `INVALID COMMIT MSG: "${msg}"\n => ERROR: WIP is not an allowed type.\n => TYPES: ${TYPES}`,
       ]);
     });
 
+    describe('(revert)', () => {
 
-    it('should support "revert: type(scope):" syntax', function() {
+      it('should allow valid "revert: ..." syntaxes', () => {
-      const correctMsg = 'revert: fix(compiler): reduce generated code payload size by 65%';
+        expect(validateMessage('revert: anything')).toBe(VALID);
-      expect(validateMessage(correctMsg)).toBe(VALID);
+        expect(validateMessage('Revert: "anything"')).toBe(VALID);
+        expect(validateMessage('revert anything')).toBe(VALID);
+        expect(validateMessage('rEvErT anything')).toBe(VALID);
+        expect(errors).toEqual([]);
+      });
+
+      it('should not allow "revert(scope): ..." syntax', () => {
+        const msg = 'revert(compiler): reduce generated code payload size by 65%';
+
+        expect(validateMessage(msg)).toBe(INVALID);
+        expect(errors).toEqual([
+          `INVALID COMMIT MSG: "${msg}"\n => ERROR: revert is not an allowed type.\n => TYPES: ${TYPES}`,
+        ]);
+      });
+
+      // https://github.com/angular/angular/issues/23479
+      it('should allow typical Angular messages generated by git', () => {
+        const msg =
+            'Revert "fix(compiler): Pretty print object instead of [Object object] (#22689)" (#23442)';
+
+        expect(validateMessage(msg)).toBe(VALID);
+        expect(errors).toEqual([]);
+      });
     });
-
-    it('should support reject "revert(scope):" syntax', function() {
-      const incorretMsg = 'revert(compiler): reduce generated code payload size by 65%';
-      expect(validateMessage(incorretMsg)).toBe(INVALID);
-      expect(errors).toEqual(
-          ['INVALID COMMIT MSG: "revert(compiler): reduce generated code payload size by 65%"\n' +
-           ' => ERROR: revert is not an allowed type.\n' +
-           ' => TYPES: ' + TYPES]);
-    });
-
-    // https://github.com/angular/angular/issues/23479
-    it('should support typical Angular messages generated by git', function() {
-      const correctMsg =
-          'Revert "fix(compiler): Pretty print object instead of [Object object] (#22689)" (#23442)';
-      expect(validateMessage(correctMsg)).toBe(VALID);
-    });
-
-    it('should validate type', function() {
-      expect(validateMessage('weird($filter): something')).toBe(INVALID);
-      expect(errors).toEqual(
-          ['INVALID COMMIT MSG: "weird($filter): something"\n' +
-           ' => ERROR: weird is not an allowed type.\n' +
-           ' => TYPES: ' + TYPES]);
-    });
-
-
-    it('should allow empty scope',
-       function() { expect(validateMessage('fix: blablabla')).toBe(VALID); });
-
-    // we don't want to allow WIP. it's ok to fail the PR build in this case to show that there is
-    // work still to be done.
-    it('should not ignore msg prefixed with "WIP: "',
-       function() { expect(validateMessage('WIP: bullshit')).toBe(INVALID); });
   });
 });